Summary: | REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Component: | Layout and Rendering | Assignee: | Sergio Villar Senin <svillar> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bfulgham, cdumez, changseok, esprehn+autocc, ews-watchlist, glenn, jbedard, jfernandez, kondapallykalyan, pdr, rbuis, rego, rniwa, simon.fraser, svillar, webkit-bug-importer, zalan | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | Safari Technology Preview | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=210089 https://bugs.webkit.org/show_bug.cgi?id=226764 |
||||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2021-02-19 14:32:58 PST
Looks like flexbox layout is O(n^2) or more on depth. One call to RenderFlexibleBox::layoutFlexItems() can layout a given child more than once, so as soon as you nest flexboxes, this multiplies down the tree. On twitch.tv, a layout can takes > 500ms can call RenderFlexibleBox::layoutFlexItems() more than 32,000 times on a single RenderFlexibleBox. Created attachment 421081 [details]
Some logging and render tree dump
I bet 99.9% of those layouts result in no size change. We need to find out how to optimize them away. Created attachment 421094 [details]
Inspector timeline from before the regression
Created attachment 421095 [details]
Inspector timeline from after the regression
I haven't inspected the page in detail but if it's using nested column flexboxes with percentages then it could indeed hit hard because in the calls to updateBlockChildDirtyBitsBeforeLayout() we have: if (relayoutChildren || (child.hasRelativeLogicalHeight() && !isRenderView())) That very same method has the following comment: // FIXME: Technically percentage height objects only need a relayout if their percentage isn't going to be turned into // an auto value. Add a method to determine this, so that we can avoid the relayout. However changing it seems risky as it might potentially affect everything not only flexbox. Created attachment 421181 [details]
Patch
(In reply to Sergio Villar Senin from comment #7) > I haven't inspected the page in detail but if it's using nested column > flexboxes with percentages then it could indeed hit hard because in the > calls to updateBlockChildDirtyBitsBeforeLayout() we have: > > if (relayoutChildren || (child.hasRelativeLogicalHeight() && > !isRenderView())) > > That very same method has the following comment: > // FIXME: Technically percentage height objects only need a relayout if > their percentage isn't going to be turned into > // an auto value. Add a method to determine this, so that we can avoid > the relayout. > > However changing it seems risky as it might potentially affect everything > not only flexbox. I managed to fix the issue without having to change that. Created attachment 421192 [details]
Patch
Fixing some tests flakiness
Committed r273264: <https://commits.webkit.org/r273264> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421192 [details]. |