RenderLayer::collectFragments costs around 20% time in RenderLayer::collectFragments for this test.
Created attachment 437140 [details] screen-shot-collectFragments-cost
Created attachment 437142 [details] test-change-first-child-height.html
Created attachment 437162 [details] Patch
Comment on attachment 437162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437162&action=review > Source/WebCore/rendering/RenderLayer.cpp:4591 > + if (!paintDirtyRect.intersects(m_repaintRects.outlineBoundsRect)) This needs to check m_repaintRectsValid
Comment on attachment 437162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437162&action=review >> Source/WebCore/rendering/RenderLayer.cpp:4591 >> + if (!paintDirtyRect.intersects(m_repaintRects.outlineBoundsRect)) > > This needs to check m_repaintRectsValid Done, thanks!
Created attachment 437376 [details] Patch
Created attachment 437606 [details] Patch
<rdar://problem/82921321>
Created attachment 437754 [details] Patch
Created attachment 437822 [details] Patch
Comment on attachment 437822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437822&action=review > Source/WebCore/rendering/RenderLayer.cpp:3553 > + return true; With a transform in place there could still be no intersection I think? If so it is probably best to keep this check out of hasIntersections. Or rename to something like hasSimpleIntersections.
Comment on attachment 437822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437822&action=review >> Source/WebCore/rendering/RenderLayer.cpp:3553 >> + return true; > > With a transform in place there could still be no intersection I think? If so it is probably best to keep this check out of hasIntersections. Or rename to something like hasSimpleIntersections. Thanks, looks like this can be removed.
Created attachment 437952 [details] Patch
Created attachment 437954 [details] screenshot-of-optimized-collectFragments
Created attachment 437955 [details] screenshot-of-original-collectFragments
Hi, I think the patch is ready for review. According to the tests on my local Mac. The function RenderLayer::collectFragments costs about 20% less (from 27% to 6.2%) while loading the test page. A performance test has been added, which is based on requestAnimationFrame. The result is about 10% better. Please take a look. Thanks:)
Created attachment 438231 [details] Patch
Comment on attachment 438231 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438231&action=review > Source/WebCore/rendering/RenderLayer.cpp:3552 > + if (auto* box = renderBox(); box && box->hasRenderOverflow()) { If a box has overflow, why can't we just check against m_repaintRects.clippedOverflowRect?
Comment on attachment 438231 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438231&action=review >> Source/WebCore/rendering/RenderLayer.cpp:3552 >> + if (auto* box = renderBox(); box && box->hasRenderOverflow()) { > > If a box has overflow, why can't we just check against m_repaintRects.clippedOverflowRect? Yes, we can remove this. I was trying to collect all the rects, including the descendants' which is not needed. And it seems this approach will change the return value of collectFragments when there is backgroundClipRect. I will try to upload a new patch that should not change the return value.
Created attachment 439162 [details] Patch
The time is reduced from 27% to 15.2% with the new optimization.
Created attachment 439328 [details] screenshot-of-optimized-collectFragments
Created attachment 439330 [details] Patch
Comment on attachment 439330 [details] Patch Hi, I think the patch is ready for review now. Please take a look, thanks!
The intersection check and the parentClipRects part are not very relevant, so I've split the parentClipRects changes to a new bug. (230885)
Created attachment 439451 [details] Patch-for-review
Created attachment 440179 [details] Patch
Comment on attachment 439451 [details] Patch-for-review View in context: https://bugs.webkit.org/attachment.cgi?id=439451&action=review > Source/WebCore/rendering/RenderLayer.cpp:4625 > + foregroundRect = LayoutRect(); It seems at this point we can just return? Also is it possible to add a single block to handle the !intersectsBackgroundRect, rather than in three areas?
Comment on attachment 439451 [details] Patch-for-review View in context: https://bugs.webkit.org/attachment.cgi?id=439451&action=review >> Source/WebCore/rendering/RenderLayer.cpp:4625 >> + foregroundRect = LayoutRect(); > > It seems at this point we can just return? Also is it possible to add a single block to handle the !intersectsBackgroundRect, rather than in three areas? Yeah, indeed, we can return here. BTW, we moved this fix to bug: 231169