Open Bug 1434145 Opened 6 years ago Updated 2 years ago

Four large stylist flushes in quick succession on cnn.com

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: bholley, Unassigned)

References

Details

Attachments

(2 files)

I was profiling CNN today, and saw this us spending 55ms on my MBP doing stylist flushes: https://perfht.ml/2FrUWxt . This also happens when loading the page locally, so this is unrelated to network congestion.

It seems likely that we could do better here.
Enabling logging in stylesheet_set along with some custom logging in ServoStyleSet.cpp to log flush times, I get:

DEBUG:style::stylesheet_set: StylesheetSet::append_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::append_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::flush
BHDebug: Did a flush that took 172.228644 milliseconds
DEBUG:style::stylesheet_set: StylesheetSet::append_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::remove_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::flush
BHDebug: Did a flush that took 160.610066 milliseconds
DEBUG:style::stylesheet_set: StylesheetSet::append_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::flush
BHDebug: Did a flush that took 0.033784 milliseconds
DEBUG:style::stylesheet_set: StylesheetSet::remove_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::flush
BHDebug: Did a flush that took 158.512177 milliseconds
DEBUG:style::stylesheet_set: StylesheetSet::append_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::flush
BHDebug: Did a flush that took 0.084973 milliseconds
DEBUG:style::stylesheet_set: StylesheetSet::remove_stylesheet
DEBUG:style::stylesheet_set: StylesheetSet::flush
BHDebug: Did a flush that took 157.866285 milliseconds
This is the stack responsible for the first flush, which happens before layout has started on the presshell. I'm quite skeptical that we really need to be triggering a flush here.

A very naive solution would be to move the code at [1] below the mDidInitialize bailout. I suspect that would break things, but I can't immediately think of a reason why.

[1] https://searchfox.org/mozilla-central/rev/97cb0aa64ae51adcabff76fb3b5eb18368f5f8ab/layout/base/PresShell.cpp#4590
This is the stack responsible for the second flush. This appears to be normal layout start, triggered by a script query of offsetHeight. We have to rebuild the stylist here because a document stylesheet was added in the interim.
(I didn't have time to analyze the third and fourth before heading out, but will do so tomorrow)
Depends on: 1434474
Bug 1434474 should eliminate one of the flushes. We can eliminate a second flush using a simple optimization, which I'll attach.

The last one is trickier though, and basically boils down to Modernizr doing slow stuff. I've emailed the Modernizr folks to see if they're open to improving things.
Depends on: 1434756
[ Triage 2017/02/20: P3 ]
Priority: -- → P3
Depends on: 1443536

This is fixed by [1], but no luck thus far getting a Modernizr release pushed out. :-(

[1] https://github.com/Modernizr/Modernizr/pull/2309

Assignee: bobbyholley → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: