Added attachment showing the recalc count actually regressed in some cases with  issue 567021 . Raised priority.

Deleted: sheet-invalidation.html 643 bytes

sheet-invalidation.html 643 bytes View Download

I have a preliminary implementation and a local copy of a snapshot of my personal facebook stream. It contains 46 stylesheets. Running following snippet in the console:

for (var j=0; j<10; j++) {
  var start = new Date();
  for (var i=0; i<document.styleSheets.length; i++) {
    document.styleSheets[i].disabled = true;
    document.body.offsetTop;
    document.styleSheets[i].disabled = false;
    document.body.offsetTop;
  }
  console.log(new Date() - start);
}

Output from Chrome 55.0.2883.87:

5063
4879
4860
4879
4911
5175
4958
5001
4958
4914

content_shell master:

1995
1519
1526
1525
1523
1533
1558
1526
1534
1533

content_shell master + preliminary patch:

1116
954
958
951
957
956
967
979
953
961

With your approach what happens if there's something like:

.x div .y div

Does it invalidate all instances of .y?

One thing to keep in mind in the FB case is sometimes the best selector to use in the invalidation rule might not be the rightmost one. As an example, imagine that we're loading the chat portion of the website. We might have a css rule of the form:

.ChatTab .Button a { color: blue; }

At this point the page might have a number button elements, but would not have any ChatTab elements -- because we haven't loaded the chat CSS we don't put the chat tab in the DOM.

Something that could be worth considering is a heuristic like "If the selector contains any class names which do not exist in the document, ignore the rule". This may not show up as much in the benchmark where you add/remove stylesheets but in practical usage I suspect this will benefit the site.

> With your approach what happens if there's something like:
>
> .x div .y div
>
> Does it invalidate all instances of .y?

It will invalidate all div descendants of .x

Invalidation sets are always used to invalidate style for elements matching the rightmost compound. This is how it's done for dom mutations as well.

It's not given that .x or .y will be the more specific choice in general. If you want to complicate the heuristics, picking ids before class selectors regardless of which compound it belongs to could be an improvement. As an example opposite to your .ChatTab example, take:

.mytheme .searchField span {}

which invalidates every span in the document if ".mytheme" is a class set on body for themeing.

There's always a balance between the cost of creating the selector meta-data in terms of memory and cpu, the cost of more accurately finding the elements that need the style recalc, and the cost of the actual work of matching selectors and constructing the computed style.

If you insert a really small stylesheet into a document, the cheapest would probably be to, for every element, do an accurate match for all selectors in that stylesheet, until one matches, and mark elements which matches at least one selector from that stylesheet for style recalc.

> Something that could be worth considering is a heuristic like
> "If the selector contains any class names which do not exist
> in the document, ignore the rule". This may not show up as much
> in the benchmark where you add/remove stylesheets but in practical
> usage I suspect this will benefit the site.

With the WIP patch I have, If all selectors are prefixed with ".ChatTab", and no elements have the "ChatTab" class, no element will have its style recalculated. However, there will still be a traversal across the dom trying to figure out if any of the element classes, ids, or other attributes have such selectors in its leftmost compound.

I discovered yesterday that I need to take custom pseudo elements into account to avoid full document recalcs for some of the facebook stylesheets as they contain rules like:

*::-webkit-input-placeholder{color:#90949c}
*:focus::-webkit-input-placeholder{color:#bec2c9}

> It's not given that .x or .y will be the more specific choice in general. If you want to complicate the heuristics, picking ids before class selectors regardless of which compound it belongs to could be an improvement. As an example opposite to your .ChatTab example, take:
> .mytheme .searchField span {}

Yeah, very fair point -- this is also a really common pattern within FB. We have a number of things like ".chrome .chat .button span {}".

> There's always a balance between the cost of creating the selector meta-data in terms of memory and cpu, the cost of more accurately finding the elements that need the style recalc, and the cost of the actual work of matching selectors and constructing the computed style.

Yeah, fair point here. I guess my intuition here is that as a developer you generally avoid inserting a stylesheet which will match any elements currently in the DOM. Doing so means that that stylesheet insertion will cause a FOUC which is an anti-pattern. At least the way we write stylesheets this generally means that while a complex rule might have a number of parts to it, at least one part of the rule will reference a class that won't be in the document.

Is there any way to quickly determine the size of an invalidation? For example, lets say that a rule starts with .chrome. That rule will require the traversal of a large part of the DOM. It could be worth putting in the effort to figure out if the rule potentially matches any element in the DOM by walking through the set of ID/class selectors and seeing if there are any such instances.

Anyways I think it's really good that we're looking htrough the kind of rules that FB has and figuring out which ones the browser can quickly evaluate. We should probably also try to come up with a more practical test case showing how we actually use CSS (ie not disabling stylesheets which are already present which would cause a FOUC and we would never do but focusing on the types of situations where we actually add styles)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/47ac662b73ab95e33844fe6e6a9ce06104708ebb

commit 47ac662b73ab95e33844fe6e6a9ce06104708ebb
Author: rune <rune@opera.com>
Date: Fri Feb 17 01:55:01 2017

Add type selector invalidation set for ruleset invalidations.

We currently look at RuleSet::tagRules() to figure out if an element
needs a style recalc when adding a stylesheet. This recalculates too
much for rules like "#id span" which ends up in the tagRules bucket,
causing style recalcs for every span. The plan is to use an
m_typeRuleInvalidationSet which contains the tag names for rules which
don't contain other simple selectors which have associated invalidation
sets.

For instance, "#id span" will not add span to m_typeRuleInvalidationSet
since we can rely on the invalidation set for #id to invalidate spans.
However, "span" or "div span" will add span to that set.

This CL prepares for this by introducing the set and a way to collect
it. This new set will be scheduled on the root node of the TreeScope
when adding/removing a stylesheet. We did not support scheduling
invalidation sets on the document node, so this CL adds that
possibility as well.

BUG= 680549 

Review-Url: https://codereview.chromium.org/2699883002
Cr-Commit-Position: refs/heads/master@{#451170}

[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/css/RuleFeature.h
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.cpp
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.h
[add] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/css/invalidation/StyleInvalidatorTest.cpp
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/inspector/InspectorTraceEvents.cpp
[modify] https://crrev.com/47ac662b73ab95e33844fe6e6a9ce06104708ebb/third_party/WebKit/Source/core/inspector/InspectorTraceEvents.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a04a994fb4146b480a2fb4c0505679fc1e1d857a

commit a04a994fb4146b480a2fb4c0505679fc1e1d857a
Author: rune <rune@opera.com>
Date: Sat Feb 18 19:53:20 2017

Schedule a type selector invalidation set for RuleSet invalidations.

We marked all elements which had a selector in the tagRules bucket for
style recalc for RuleSet invalidations. That means we would recalculate
style for all spans if we added a stylesheet containing a rule with an
"#id span" selector (but not for "#id span.class" as that ends up in
the classRules bucket).

Instead, use an invalidation set containing only tag names for the
selectors where there are no ids, classes, or attribute selectors, and
which have a type selector in the rightmost compound. This means that
"#id span" will not add "span" to that set, but "span" and "div span"
will. "div span" will not add "div", and "div *" will cause a full
scope recalc. In order to support invalidation for those, we would have
had to have one invalidation set for each tag name instead of a single
descendant invalidation set for all.

RuleSet invalidations schedule this typeRuleInvalidationSet on the root
of the TreeScope When doing ruleset invalidations.

BUG= 680549 

Review-Url: https://codereview.chromium.org/2703643003
Cr-Commit-Position: refs/heads/master@{#451488}

[modify] https://crrev.com/a04a994fb4146b480a2fb4c0505679fc1e1d857a/third_party/WebKit/LayoutTests/fast/css/invalidation/sheet-ruleset-invalidation.html
[modify] https://crrev.com/a04a994fb4146b480a2fb4c0505679fc1e1d857a/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/a04a994fb4146b480a2fb4c0505679fc1e1d857a/third_party/WebKit/Source/core/css/RuleFeature.h
[modify] https://crrev.com/a04a994fb4146b480a2fb4c0505679fc1e1d857a/third_party/WebKit/Source/core/dom/StyleEngine.cpp
[modify] https://crrev.com/a04a994fb4146b480a2fb4c0505679fc1e1d857a/third_party/WebKit/Source/core/dom/StyleEngine.h
[modify] https://crrev.com/a04a994fb4146b480a2fb4c0505679fc1e1d857a/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/248934089f7a250a94842e1c3059167d704e72fd

commit 248934089f7a250a94842e1c3059167d704e72fd
Author: rune <rune@opera.com>
Date: Mon Feb 20 08:04:01 2017

Invalidate custom pseudo elements for RuleSet invalidations.

When we have selectors containing custom pseudo elements matching
elements inside the UA shadow, and no id, class, or attribute selectors
present, do like we do for type selectors and invalidate as part of an
invalidation set scheduled on the root node of the tree scope for
RuleSet invalidations.

We utilize the same invalidation set as for type invalidations and mark
it as invalidating custom pseudo elements as well as marking it as
tree-boundary-crossing to allow drilling into the UA shadow.

This means we will traverse into all shadow sub-trees, but it should at
least be better than the existing recalc all behavior.

The full recalc for custom pseudo elements caused a full recalc for one
of the stylesheets on facebook.com.

R=ericwilligers@chromium.org
BUG= 680549 

Review-Url: https://codereview.chromium.org/2700943003
Cr-Commit-Position: refs/heads/master@{#451578}

[modify] https://crrev.com/248934089f7a250a94842e1c3059167d704e72fd/third_party/WebKit/LayoutTests/fast/css/invalidation/sheet-ruleset-invalidation.html
[modify] https://crrev.com/248934089f7a250a94842e1c3059167d704e72fd/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/248934089f7a250a94842e1c3059167d704e72fd/third_party/WebKit/Source/core/css/RuleFeature.h
[modify] https://crrev.com/248934089f7a250a94842e1c3059167d704e72fd/third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp
[modify] https://crrev.com/248934089f7a250a94842e1c3059167d704e72fd/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp

Adjusting priority back since a fix for the regression in https://bugs.chromium.org/p/chromium/issues/detail?id=680549#c2 landed.

The problem in the issue title/description has been fixed. Further optimizations tracked in  issue 706758 .