Bug 128321 - Check selectors exactly when invalidating style
Summary: Check selectors exactly when invalidating style
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-06 11:59 PST by Antti Koivisto
Modified: 2014-02-06 17:57 PST (History)
8 users (show)

See Also:

Attachments
patch (16.45 KB, patch)
2014-02-06 12:08 PST, Antti Koivisto 		kling: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2014-02-06 11:59:06 PST 
instead of heuristics
Comment 1 Antti Koivisto 2014-02-06 12:08:22 PST 
Created attachment 223362 [details]
patch
Comment 2 Andreas Kling 2014-02-06 12:45:10 PST 
Comment on attachment 223362 [details]
patch

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

r=me with some tweaks.

Q: Can we piggyback on the already massaged data in the Document's StyleResolver here?

> Source/WebCore/ChangeLog:9
> +        With selectors are really fast to match with the JIT. Take advantage of it by invalidating
> +        document style exactly when new stylesheets arrive (instead of heuristics).

I don't think you need the "With" here. We should remove it to achieve 60fps on mobile in 2014.

> Source/WebCore/css/StyleInvalidationAnalysis.cpp:40
> +static bool shouldDirtyAllStyle(const Vector<RefPtr<StyleRuleBase>> rules)

We should pass the rules vector by (const) reference here.

> Source/WebCore/css/StyleInvalidationAnalysis.cpp:42
> +    for (auto rule : rules) {

Ref churn here. auto& would avoid it.

> Source/WebCore/css/StyleInvalidationAnalysis.cpp:57
> +    for (auto import : sheet.importRules()) {

Same here.

> Source/WebCore/css/StyleInvalidationAnalysis.cpp:70
> +    for (auto sheet : sheets) {

Sam has convinced me to start writing "auto*" to clarify that there's no by-value copy.

> Source/WebCore/css/StyleInvalidationAnalysis.cpp:84
> +    for (auto sheet : sheets)

Same comment here.
Comment 3 Antti Koivisto 2014-02-06 15:30:05 PST 
(In reply to comment #2)
> (From update of attachment 223362 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223362&action=review
> 
> r=me with some tweaks.
> 
> Q: Can we piggyback on the already massaged data in the Document's StyleResolver here?

Not really. The new sheet stuff is not in StyleResolver yet. Optimally we would transfer it over instead of reprocessing but everything here is pretty cheap in any case.
Comment 4 Antti Koivisto 2014-02-06 17:54:16 PST 
https://trac.webkit.org/r163592