Bug 222202 – REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

View a twitch livestream in fullscreen, and open web inspector on another screen. Take a timeline, and look at the layout time.

WebKit continually does very long layouts (longer than 350ms). Firefox and Chrome show no long layouts.

[Radar WebKit Bug Importer]

<rdar://problem/74537782>

[Simon Fraser (smfr)]

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.

[Simon Fraser (smfr)]

Created attachment 421081 [details]
Some logging and render tree dump

[Simon Fraser (smfr)]

I bet 99.9% of those layouts result in no size change. We need to find out how to optimize them away.

[Simon Fraser (smfr)]

Created attachment 421094 [details]
Inspector timeline from before the regression

[Simon Fraser (smfr)]

Created attachment 421095 [details]
Inspector timeline from after the regression

[Sergio Villar Senin]

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.

[Sergio Villar Senin]

Created attachment 421181 [details]
Patch

[Sergio Villar Senin]

(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.

[Sergio Villar Senin]

Created attachment 421192 [details]
Patch

Fixing some tests flakiness

[EWS]

Committed r273264: <https://commits.webkit.org/r273264>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421192 [details].