Issue metadata
Sign in to add a comment
|
3%-11.4% regression in smoothness.sync_scroll.key_mobile_sites_smooth at 540732:540754 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Mar 5 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/1288b712440000
,
Mar 5 2018
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/1288b712440000 [RLS] Re-land: skip compositing steps on frame scroll. by chrishtr@chromium.org https://chromium.googlesource.com/chromium/src/+/9732521599b291a0703830c0d9d94c381916b7d8 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Mar 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc commit 3341c5a4883f580df6e4ae0138ca4b514b8ff2bc Author: Chris Harrelson <chrishtr@chromium.org> Date: Tue Mar 13 00:23:04 2018 Devirtualize ScrollableArea::UsesCompositedScrolling. This method is called in various hot code paths, in particular in ObjectPaintInvalidator. Bug: 818709 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I6eaf1acadc5ac5721c7e42c900964bcc31fee769 Reviewed-on: https://chromium-review.googlesource.com/959155 Reviewed-by: vmpstr <vmpstr@chromium.org> Commit-Queue: vmpstr <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/master@{#542665} [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/core/frame/LocalFrameView.cpp [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/core/frame/LocalFrameView.h [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/core/paint/compositing/PaintLayerCompositor.cpp [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp [modify] https://crrev.com/3341c5a4883f580df6e4ae0138ca4b514b8ff2bc/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
,
Mar 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e54f71ef43dee5f77d71aa53a626d1292ee637b commit 1e54f71ef43dee5f77d71aa53a626d1292ee637b Author: Chris Harrelson <chrishtr@chromium.org> Date: Tue Mar 13 00:24:54 2018 [PE] Don't call CompositedScrollsWithRespectTo during raster invalidation It is redundant with the call to UsesCompositedScrolling just below, and is equivalent to UsesCompositedScrolling + object != paint_invalidation_container. UsesCompositedScrolling calls two methods plus a virtual, so may be slow. Bug: 818709 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Iab617e3941563ac1994e5732359cf691442a2b7e Reviewed-on: https://chromium-review.googlesource.com/959444 Commit-Queue: vmpstr <vmpstr@chromium.org> Reviewed-by: vmpstr <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/master@{#542667} [modify] https://crrev.com/1e54f71ef43dee5f77d71aa53a626d1292ee637b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
,
Mar 15 2018
On this page: https://www.google.co.uk/search?hl=en&q=barack+obama&cad=h It looks like there is a 33% slowdown in PrePaint, when testing on a mid-range Android phone. This appears to account for the bulk of the regression. Note that the profile for this page is quite different on mobile and on desktop, I think due to URL bar hiding.
,
Mar 15 2018
I have profiled locally on desktop. Two problems: 1. Increased cost of ::UpdateForChildren in PaintPropertyTreeBuilder.cpp, mostly due to expensive calls to LayoutBox::ScrolledContentOffset, but also a bit of other stuff. 2. Increased cost of PaintInvalidator::InvalidatePaint. Haven't dug into that yet.
,
Mar 16 2018
BTW it appears that, despite r542665, ScrollableArea::UsesCompositedScrolling is still virtual.
,
Mar 16 2018
Oops. Will fix now.
,
Mar 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0499ebf8339aea9b4c86b4a374b18c836f7b7121 commit 0499ebf8339aea9b4c86b4a374b18c836f7b7121 Author: Chris Harrelson <chrishtr@chromium.org> Date: Fri Mar 16 19:24:10 2018 [RLS] Micro-optimize ScrolledContentOffset and IsFixedPositionObjectInPagedMedia. LayoutBox::ScrolledContentOffset can be much simpler for LayoutViews, and also for LayoutBoxes that are not LayoutFlexibleBox. To tease this apart, this patch makes ScrolledContentOffset virtual to provide the more expensive implementation only when needed. Also, check for fixed-position in IsFixedPositionObjectInPagedMedia before finding the View(), which may be expensive because it involves ping- ponging through Document. Bug:818709 Change-Id: I8470b7c1487b3fbf4f227aad21e8f9da81c1e9bb Reviewed-on: https://chromium-review.googlesource.com/964933 Reviewed-by: vmpstr <vmpstr@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#543781} [modify] https://crrev.com/0499ebf8339aea9b4c86b4a374b18c836f7b7121/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/0499ebf8339aea9b4c86b4a374b18c836f7b7121/third_party/WebKit/Source/core/layout/LayoutBox.h [modify] https://crrev.com/0499ebf8339aea9b4c86b4a374b18c836f7b7121/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp [modify] https://crrev.com/0499ebf8339aea9b4c86b4a374b18c836f7b7121/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h [modify] https://crrev.com/0499ebf8339aea9b4c86b4a374b18c836f7b7121/third_party/WebKit/Source/core/layout/LayoutObject.cpp [modify] https://crrev.com/0499ebf8339aea9b4c86b4a374b18c836f7b7121/third_party/WebKit/Source/core/layout/LayoutView.cpp [modify] https://crrev.com/0499ebf8339aea9b4c86b4a374b18c836f7b7121/third_party/WebKit/Source/core/layout/LayoutView.h
,
Mar 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/def2da31e92ed904c370231eacce9710707b95f3 commit def2da31e92ed904c370231eacce9710707b95f3 Author: Chris Harrelson <chrishtr@chromium.org> Date: Fri Mar 16 19:58:48 2018 [RLS] Actually make SA::UsesCompositedScrolling non-virtual. This was an oversight in a previous CL. Bug: 818709 Change-Id: I2697d136be7af56e00f342f77451b765a37c8eaa Reviewed-on: https://chromium-review.googlesource.com/966796 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#543795} [modify] https://crrev.com/def2da31e92ed904c370231eacce9710707b95f3/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82812cf386713a67d3d1360be9b48b104b20d93f commit 82812cf386713a67d3d1360be9b48b104b20d93f Author: Chris Harrelson <chrishtr@chromium.org> Date: Wed May 02 03:47:11 2018 [RLS] Use faster fast-path when updating composited root scroller offset. Previously, we would update all of the composited layers plus reach into the LayerTreeHost (via some virtuals) to update even more things. All of these will be updated anyway if the composited layer configurations actually change, and it's pure overhead here. PaintLayerCompositor::FrameViewDidScroll does the same thing (and this is where I cribbed from in my original patch), but it's overkill. I didn't change that call site, however, since it is non-RLS specific. Bug:818709 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I3643e42bc4b68bafcfd06dfe787a18b09ee2c7da Reviewed-on: https://chromium-review.googlesource.com/1038439 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Cr-Commit-Position: refs/heads/master@{#555302} [modify] https://crrev.com/82812cf386713a67d3d1360be9b48b104b20d93f/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc [modify] https://crrev.com/82812cf386713a67d3d1360be9b48b104b20d93f/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h [modify] https://crrev.com/82812cf386713a67d3d1360be9b48b104b20d93f/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
,
May 2 2018
I was finally able to reproduce the regression. On a Nexus 5, I was able to observe a difference in frame_times of 20ms before patch and 24ms after patch. When looking at the resulting traces, I think I understand what is happening. Before the patch, there are 3 long updates at the beginning of the scroll, each of which is 14ms. After, there is only 1 and it is 29ms. Subsequent main frame computations are about the same cost in both cases. Therefore this is not a real regression. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Mar 5 2018