Title: [273264] trunk
Revision
273264
Author
[email protected]
Date
2021-02-22 11:12:03 -0800 (Mon, 22 Feb 2021)

Log Message

REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not
https://bugs.webkit.org/show_bug.cgi?id=222202
<rdar://problem/74537782>

Reviewed by Simon Fraser.

PerformanceTests:

New performance test for nested column flexboxes with percentage heights.

* Layout/nested-column-flexboxes-relative-height.html: Added.

Source/WebCore:

The problem was that we were doing the initial layout for the children of the flex container twice in those cases where
the child inline axis was not the main axis (for example with column flex containers in horizontal writing modes).
Refactored the code (specially the way we clear overriding sizes) so that we only do it once. This saves tons of layouts
in pages with nested column flexboxes with relative heights.

No new tests as there is no change in functionality, we're removing duplicate extra layouts. We're however adding a new
performance test for column flexboxes with percentage heights. With this patch we go from 3.5 layout/s to 145 layout/s
which is ~4000% better.

Inspired by Blink's crrev.com/c/1614058 by <[email protected]>.

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild): Do not unconditionally clear overriding sizes. Also removed
relayoutChildren which is now unused. Do not layout the item, that should have been done in
computeInnerFlexBaseSizeForChild() before. Added ASSERTs to verify that child's intrinsic main size was cached as
a consequence of the previous layout.
(WebCore::RenderFlexibleBox::constructFlexItem): Do not pass relayoutChildren to computeInnerFlexBaseSizeForChild. Also no
need to update it after laying out the child.
* rendering/RenderFlexibleBox.h:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/TestExpectations (273263 => 273264)


--- trunk/LayoutTests/TestExpectations	2021-02-22 18:56:13 UTC (rev 273263)
+++ trunk/LayoutTests/TestExpectations	2021-02-22 19:12:03 UTC (rev 273264)
@@ -3915,7 +3915,7 @@
 
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-012.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-015.html [ ImageOnlyFailure ]
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-017.html [ ImageOnlyFailure ]
+webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-017.html [ Failure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html [ ImageOnlyFailure ]

Modified: trunk/PerformanceTests/ChangeLog (273263 => 273264)


--- trunk/PerformanceTests/ChangeLog	2021-02-22 18:56:13 UTC (rev 273263)
+++ trunk/PerformanceTests/ChangeLog	2021-02-22 19:12:03 UTC (rev 273264)
@@ -1,3 +1,15 @@
+2021-02-22  Sergio Villar Senin  <[email protected]>
+
+        REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not
+        https://bugs.webkit.org/show_bug.cgi?id=222202
+        <rdar://problem/74537782>
+
+        Reviewed by Simon Fraser.
+
+        New performance test for nested column flexboxes with percentage heights.
+
+        * Layout/nested-column-flexboxes-relative-height.html: Added.
+
 2021-02-18  Myles C. Maxfield  <[email protected]>
 
         MotionMark scores are super sensitive to a single long frame

Added: trunk/PerformanceTests/Layout/nested-column-flexboxes-relative-height.html (0 => 273264)


--- trunk/PerformanceTests/Layout/nested-column-flexboxes-relative-height.html	                        (rev 0)
+++ trunk/PerformanceTests/Layout/nested-column-flexboxes-relative-height.html	2021-02-22 19:12:03 UTC (rev 273264)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<head>
+<style>
+.flex {
+    display: flex;
+    flex-direction: column;
+    height: 100%;
+}
+</style>
+<script src=""
+<script>
+function startTest() {
+    document.body.offsetHeight;
+
+    var index = 0;
+    PerfTestRunner.measureRunsPerSecond({run: function() {
+        document.body.style.width = ++index % 2 ? "99%" : "98%";
+        document.body.offsetHeight;
+    }});
+}
+</script>
+</head>
+<body _onload_="startTest()">
+<div class="flex">
+    <div class="flex">
+        <div class="flex">
+            <div class="flex">
+                <div class="flex">
+                    <div class="flex">
+                        <div class="flex">
+                            <div class="flex">
+                                <div class="flex">
+                                    <div class="flex">
+                                        <div class="flex"></div>
+                                    </div>
+                                </div>
+                            </div>
+                        </div>
+                    </div>
+                </div>
+            </div>
+        </div>
+    </div>
+</div>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (273263 => 273264)


--- trunk/Source/WebCore/ChangeLog	2021-02-22 18:56:13 UTC (rev 273263)
+++ trunk/Source/WebCore/ChangeLog	2021-02-22 19:12:03 UTC (rev 273264)
@@ -1,3 +1,31 @@
+2021-02-22  Sergio Villar Senin  <[email protected]>
+
+        REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not
+        https://bugs.webkit.org/show_bug.cgi?id=222202
+        <rdar://problem/74537782>
+
+        Reviewed by Simon Fraser.
+
+        The problem was that we were doing the initial layout for the children of the flex container twice in those cases where
+        the child inline axis was not the main axis (for example with column flex containers in horizontal writing modes).
+        Refactored the code (specially the way we clear overriding sizes) so that we only do it once. This saves tons of layouts
+        in pages with nested column flexboxes with relative heights.
+
+        No new tests as there is no change in functionality, we're removing duplicate extra layouts. We're however adding a new
+        performance test for column flexboxes with percentage heights. With this patch we go from 3.5 layout/s to 145 layout/s
+        which is ~4000% better.
+
+        Inspired by Blink's crrev.com/c/1614058 by <[email protected]>.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild): Do not unconditionally clear overriding sizes. Also removed
+        relayoutChildren which is now unused. Do not layout the item, that should have been done in
+        computeInnerFlexBaseSizeForChild() before. Added ASSERTs to verify that child's intrinsic main size was cached as
+        a consequence of the previous layout.
+        (WebCore::RenderFlexibleBox::constructFlexItem): Do not pass relayoutChildren to computeInnerFlexBaseSizeForChild. Also no
+        need to update it after laying out the child.
+        * rendering/RenderFlexibleBox.h:
+
 2021-02-22  Alex Christensen  <[email protected]>
 
         Disable RangeResponseGenerator.

Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (273263 => 273264)


--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp	2021-02-22 18:56:13 UTC (rev 273263)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp	2021-02-22 19:12:03 UTC (rev 273264)
@@ -922,10 +922,8 @@
 }
 
     
-LayoutUnit RenderFlexibleBox::computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding, bool relayoutChildren)
+LayoutUnit RenderFlexibleBox::computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding)
 {
-    child.clearOverridingContentSize();
-    
     Length flexBasis = flexBasisForChild(child);
     if (childMainSizeIsDefinite(child, flexBasis))
         return std::max(0_lu, computeMainAxisExtentForChild(child, MainOrPreferredSize, flexBasis).value());
@@ -935,18 +933,11 @@
         return adjustChildSizeForAspectRatioCrossAxisMinAndMax(child, computeMainSizeFromAspectRatioUsing(child, crossSizeLength));
     }
 
-    // The flex basis is indefinite (=auto), so we need to compute the actual
-    // width of the child. For the logical width axis we just use the preferred
-    // width; for the height we need to lay out the child.
+    // The flex basis is indefinite (=auto), so we need to compute the actual width of the child.
     LayoutUnit mainAxisExtent;
     if (!mainAxisIsChildInlineAxis(child)) {
-        updateBlockChildDirtyBitsBeforeLayout(relayoutChildren, child);
-        if (child.needsLayout() || relayoutChildren || !m_intrinsicSizeAlongMainAxis.contains(&child)) {
-            if (!child.needsLayout())
-                child.setChildNeedsLayout(MarkOnlyThis);
-            child.layoutIfNeeded();
-            cacheChildMainSize(child);
-        }
+        ASSERT(!child.needsLayout());
+        ASSERT(m_intrinsicSizeAlongMainAxis.contains(&child));
         mainAxisExtent = m_intrinsicSizeAlongMainAxis.get(&child);
     } else {
         // We don't need to add scrollbarLogicalWidth here because the preferred
@@ -1299,6 +1290,7 @@
 
 FlexItem RenderFlexibleBox::constructFlexItem(RenderBox& child, bool relayoutChildren)
 {
+    child.clearOverridingContentSize();
     if (childHasIntrinsicMainAxisSize(child)) {
         // If this condition is true, then computeMainAxisExtentForChild will call
         // child.intrinsicContentLogicalHeight() and child.scrollbarLogicalHeight(),
@@ -1317,13 +1309,12 @@
             child.setChildNeedsLayout(MarkOnlyThis);
             child.layoutIfNeeded();
             cacheChildMainSize(child);
-            relayoutChildren = false;
             child.clearOverridingContainingBlockContentSize();
         }
     }
     
     LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
-    LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding, relayoutChildren);
+    LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
     LayoutUnit childMinMaxAppliedMainAxisExtent = adjustChildSizeForMinAndMax(child, childInnerFlexBaseSize);
     LayoutUnit margin = isHorizontalFlow() ? child.horizontalMarginExtent() : child.verticalMarginExtent();
     return FlexItem(child, childInnerFlexBaseSize, childMinMaxAppliedMainAxisExtent, borderAndPadding, margin);

Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.h (273263 => 273264)


--- trunk/Source/WebCore/rendering/RenderFlexibleBox.h	2021-02-22 18:56:13 UTC (rev 273263)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.h	2021-02-22 19:12:03 UTC (rev 273264)
@@ -146,7 +146,7 @@
     bool childCrossSizeShouldUseContainerCrossSize(const RenderBox& child) const;
     LayoutUnit computeMainSizeFromAspectRatioUsing(const RenderBox& child, Length crossSizeLength) const;
     void setFlowAwareLocationForChild(RenderBox& child, const LayoutPoint&);
-    LayoutUnit computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding, bool relayoutChildren);
+    LayoutUnit computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding);
     void adjustAlignmentForChild(RenderBox& child, LayoutUnit);
     ItemPosition alignmentForChild(const RenderBox& child) const;
     bool childMainSizeIsDefinite(const RenderBox&, const Length& flexBasis) const;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to