Bug 54360 - Enable fast path selector checking for child and subselector combinators
Summary: Enable fast path selector checking for child and subselector combinators
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-13 11:01 PST by Antti Koivisto
Modified: 2011-02-15 07:14 PST (History)
2 users (show)

See Also:


Attachments
patch (6.96 KB, patch)
2011-02-13 11:08 PST, Antti Koivisto
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2011-02-13 11:01:39 PST
Currently the fast path(CSSStyleSelector::SelectorChecker::fastCheckSelector) is enabled for descendant combinators only. It should be enabled for child and subselector combinators too.
Comment 1 Antti Koivisto 2011-02-13 11:08:47 PST
Created attachment 82269 [details]
patch
Comment 2 Darin Adler 2011-02-14 10:05:32 PST
Comment on attachment 82269 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=82269&action=review

Are enough of these cases covered by existing tests?

> Source/WebCore/css/CSSStyleSelector.cpp:2146
> +template <class SelectorChecker>
> +inline bool fastCheckSingleSelector(const CSSSelector*& selector, const Element*& element, const CSSSelector*& topChildOrSubselector, const Element*& topChildOrSubselectorMatchElement)

We can pass functions as template arguments. Wrapping each function in a class is unnecessary.

    template<bool selectorChecker(const Element*, AtomicStringImpl*)>

> Source/WebCore/css/CSSStyleSelector.cpp:2149
> +    while (element) {

I think this would read slightly better as a for loop.

> Source/WebCore/css/CSSStyleSelector.cpp:2157
> +            };

Excess semicolon here.
Comment 3 Antti Koivisto 2011-02-14 10:23:52 PST
(In reply to comment #2)
> (From update of attachment 82269 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=82269&action=review
> 
> Are enough of these cases covered by existing tests?

I think we have decent coverage, based on the tests I broke at different points.

> We can pass functions as template arguments. Wrapping each function in a class is unnecessary.

Very true, will fix.
Comment 4 Antti Koivisto 2011-02-15 07:14:25 PST
http://trac.webkit.org/changeset/78556