On pages with many scrollable elements, we can end up spending a lot of time styling the anonymous scrollbar content on the main thread. For example, when loading the single page HTML spec, we spend ~840 ms out of the total ~8,950 ms load time (~9%) under nsCSSFrameConstructor::GetAnonymousContent's call to StyleNewSubtree.

This is what the distribution of frame types for GetAnonymousContent calls looks like in that document:

      1 GetAnonymousContent Canvas
    739 GetAnonymousContent GfxButtonControl
    912 GetAnonymousContent Details
   1229 GetAnonymousContent Scroll
   2234 GetAnonymousContent Scrollbar

So 66% of the anonymous content generating frames are scrollbar-related. Scrollframes generate four elements (two <scrollbar>s, a <scrollcorner>, and a <resizer>). Scrollbars themselves generate six elements (four <scrollbarbutton>s, some of which are display:none depending on the platform, the <slider>, and the <thumb>). So by number of elements these consist of the vast majority of anonymous content styling work.

If we ignore scrollbar-color, then the number of unique ComputedStyle objects we need for all the scrollbar parts is manageable, and mostly depends on a few specific attributes set on the elements. I think the only other inherited property values that matter for scrollbar parts are scrollbar-width and visibility. Apart from that, we should be able to cache the ComputedStyle for a given combination of these attributes and reuse them on subsequent scrollable elements.

The downside of this is increased coupling between the UA sheets that style the scrollbar parts and the code that checks the attributes that we'd key the cache off. But the UA sheet rules for scrollbar parts don't change very often, so maybe a comment reminding people to check whether the caching code needs updating is enough.

If we ignore scrollbar-color, then the number of unique ComputedStyle objects we need for all the scrollbar parts is manageable, and mostly depends on a few specific attributes set on the elements. I think the only other inherited property values that matter for scrollbar parts are scrollbar-width and visibility.

And color (to resolve scrollbar-color), right?

The downside of this is increased coupling between the UA sheets that style the scrollbar parts and the code that checks the attributes that we'd key the cache off. But the UA sheet rules for scrollbar parts don't change very often, so maybe a comment reminding people to check whether the caching code needs updating is enough.

Is the plan to cache styles in the whole subtree? Also, is this something that could be enforced by asserts?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)

And color (to resolve scrollbar-color), right?

Yes. I was thinking just to check if scrollbar-color is non-auto, and skip caching in that case, just to avoid issues like someone animating scrollbar-color and blowing out the cache.

The downside of this is increased coupling between the UA sheets that style the scrollbar parts and the code that checks the attributes that we'd key the cache off. But the UA sheet rules for scrollbar parts don't change very often, so maybe a comment reminding people to check whether the caching code needs updating is enough.

Is the plan to cache styles in the whole subtree?

Yeah, although the only NAC subtree that actually has a child of a NAC root is <slider>, which has the <thumb> as its child.

Also, is this something that could be enforced by asserts?

I think so, yes. We could resolve a new ComputedStyle, and assert that all reset properties, scrollbar-width, and visibility match.

I can't think of a good way to assert that we don't accidentally rely on some other inherited property.

(In reply to Cameron McCormack (:heycam) from comment #2)

I can't think of a good way to assert that we don't accidentally rely on some other inherited property.

We could maybe add:

all: initial;

At the first definition of the anon box, then in debug builds we can assert that the style is exactly the same as the cached one. Assuming it makes the style not too slow to compute, which I hope it won't...

Otherwise just hoping that we don't add code that depends on inherited properties seems very handwavey in my opinion... We probably also depend on a bunch of other properties accidentally (and non-accidentally) like visibility for sure.

Great idea, I'll try adding a scrollcorner, resizer, scrollbar, scrollbarbutton, slider, thumb { all: initial; } rule to the top of minimal-xul.css and see what the effect is.

I misremembered about scrollbar-width too; that's a reset property, and none of the UA rules set it explicitly on the scrollbar part elements, so we don't need to worry about it.

Not only that, but scrollbar-color, while inherited, is only ever looked at on the originating element of the NAC (see the various nsLayoutUtils::StyleForScrollbar calls). So we should be able to cache scrollbar styles with non-default colors after all.

Some added trickiness is that the initial values of font related properties and of direction depend on the language in the document, and so might change. Since the scrollbar parts don't rely on font or direction values coming from the scrollable elements, we can reset them to some known values rather than use initial.

I have a patch now that reduces time spent under nsCSSFrameConstructor::GetAnonymousContent from ~820 ms down to ~25 ms, for the HTML spec.

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&revision=0bbaa2248c1d72f33f4311869ec79378a8f77960

For nsGfxScrollFrame, the number of NAC elements we produce varies (depending on which scrollbars we have), but for nsScrollbarFrame, we always produce the same six, so if we wanted to we could change the setup so that we could use a single AnonymousContentKey to represent all of a frame's anonymous content, rather than separate values for each element.

On my machine this test takes ~5200 ms with the anonymous content style
caching pref disabled, and ~1000 ms with it enabled.

(In reply to Cameron McCormack (:heycam) from comment #13)

Created attachment 9074395 [details]
Bug 1554571 - Part 4: Add perf-reftest singleton for anonymous content style caching.

On my machine this test takes ~5200 ms with the anonymous content style
caching pref disabled, and ~1000 ms with it enabled.

Super cool!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4eb70d60891cf495f6705d7471360865f3a7d6e

Backed out 4 changesets for causing failures in minimal-xul.css

Backout link: https://hg.mozilla.org/integration/autoland/rev/0a8bb0b56daadd482efd43d00fea439eb0a39553

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=253641360&resultStatus=testfailed%2Cbusted%2Cexception&revision=1cc4e6374b8a993fbb688ea01708ce6c73cd6974&searchStr=%28bc

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253641360&repo=autoland&lineNumber=2286

2:36:27 INFO - Buffered messages finished
02:36:27 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | Got error message for chrome://global/content/minimal-xul.css: Unknown property ‘-moz-list-reversed’. Declaration dropped. -
02:36:27 INFO - Stack trace:
02:36:27 INFO - chrome://mochikit/content/browser-test.js:test_ok:1313
02:36:27 INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_parsable_css.js:messageIsCSSError:256
02:36:27 INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_parsable_css.js:checkAllTheCSS:455

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb6ce9a38da2a4b63afdc5c6dd94a232dc5c15bb

https://hg.mozilla.org/mozilla-central/rev/8c16d6d715bf
https://hg.mozilla.org/mozilla-central/rev/0e6bda67a31b
https://hg.mozilla.org/mozilla-central/rev/427a480c163a
https://hg.mozilla.org/mozilla-central/rev/94b01ff1b161