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)
Core
CSS Parsing and Computation
Tracking
()
NEW
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.
Reporter | ||
Comment 1•6 years ago
|
||
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
Reporter | ||
Comment 2•6 years ago
|
||
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
Reporter | ||
Comment 3•6 years ago
|
||
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.
Reporter | ||
Comment 4•6 years ago
|
||
(I didn't have time to analyze the third and fourth before heading out, but will do so tomorrow)
Reporter | ||
Comment 5•6 years ago
|
||
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.
Reporter | ||
Comment 7•5 years ago
|
||
This is fixed by [1], but no luck thus far getting a Modernizr release pushed out. :-(
Reporter | ||
Updated•5 years ago
|
Assignee: bobbyholley → nobody
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•