New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 823999 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 770343



Sign in to add a comment

Blink.PrePaint.UpdateTime regression with root layer scrolling

Project Member Reported by pdr@chromium.org, Mar 21 2018

Issue description

Finch hints at a regression:
https://uma.googleplex.com/p/chrome/variations/?sid=72d7cbd66172f2e400563ad3da1498f9

50th %ile ~30% regression which is ~1.6us
75th %ile ~74% regression which is ~40us
99th %ile ~36% regression which is ~400us
These numbers aren't statistically significant.

There's something strange with the MacOS numbers that skews the data. Excluding MacOS, the regression mostly goes away: https://uma.googleplex.com/p/chrome/variations/?sid=cfca6fcf5fa4d877a25075d77cb135a2

I don't know of anything MacOS-specific with root layer scrolling that Android wouldn't also be hitting. Anyone have ideas?
 

Comment 1 by pdr@chromium.org, Mar 21 2018

Labels: NextAction3282018
Lets wait a week and see what the numbers look like.

Comment 2 by skobes@chromium.org, Mar 28 2018

Blocking: -417782 770343

Comment 3 by pdr@chromium.org, Apr 4 2018

I tried to reproduce this locally and could not reproduce it when visiting top sites or running performance tests.

Poking more at the data, this appears to only be a regression in Canary:
Dev (no regression): https://uma.googleplex.com/p/chrome/variations/?sid=df69f9d4d06200a1f1e7f31e1cd9ba24
Canary (regression): https://uma.googleplex.com/p/chrome/variations/?sid=14260741000a3fbc5496762305cf5e24
I can't explain that yet.

I have a clue though, based on UKM data. On pages with expensive PrePaint steps, scrollbar hiding seems to cause a more expensive PrePaint step with RLS on. I am not sure if that's a major cause of this, but am investigating.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86d1cc1b33260b95367afdedb6195c842dc8e5c0

commit 86d1cc1b33260b95367afdedb6195c842dc8e5c0
Author: Philip Rogers <pdr@chromium.org>
Date: Fri Apr 06 23:14:43 2018

[RLS] Do not use scrollbar-excluded overflow clip at the root

Clip paint property nodes store a rect for the clip excluding overlay
scrollbars. When overlay scrollbars appear or disappear, the associated
clip node needs to be updated. Changing a clip node is expensive because
a full treewalk is needed to ensure descendant clip caches are cleared.

This patch special-cases the topmost clip node to not store a clip
excluding overlay scrollbars. With this patch, the common operation of
showing/hiding overlay scrollbars does not cause a full treewalk.

Bug:  823999 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I605fa3dea5c33e0cbd0de314b915fd6526d64f1a
Reviewed-on: https://chromium-review.googlesource.com/999053
Commit-Queue: Philip Rogers <pdr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548960}
[modify] https://crrev.com/86d1cc1b33260b95367afdedb6195c842dc8e5c0/third_party/WebKit/Source/core/layout/ScrollbarsTest.cpp
[modify] https://crrev.com/86d1cc1b33260b95367afdedb6195c842dc8e5c0/third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
[modify] https://crrev.com/86d1cc1b33260b95367afdedb6195c842dc8e5c0/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26e2572987b0e3a147911c1aa26f7f747968cc01

commit 26e2572987b0e3a147911c1aa26f7f747968cc01
Author: Philip Rogers <pdr@chromium.org>
Date: Wed Apr 25 18:03:59 2018

[RLS] Avoid full tree rebuilds on hit testing clip changes

This is a revert of [1] which caused performance regressions. This patch
reimplements a solution that avoids full tree rebuilds when only hit
testing clip values change in the clip tree.

Typically, when property nodes are updated, we force a full tree
rebuild if nodes are created or destroyed, but not if node values are
simply updated. Clip nodes are an exception to this because changing
their values ends up causing a full tree rebuild. The hit testing clip
rect (aka clip_rect_excluding_overlay_scrollbars) is different though,
and does not need to force a full tree rebuild when it changes. This
patch special-cases the hit testing clip rects to avoid a full tree
rebuild. When overlay scrollbars hide or appear, the hit testing
clip values change but a full tree rebuild is not needed.

[1] https://crrev.com/86d1cc1b33260b95367afdedb6195c842dc8e5c0

Bug:  823999 ,  830746 ,  830771 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I7ab426aaead0c87a1aedf0980648505d82d40215
Reviewed-on: https://chromium-review.googlesource.com/1026941
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553652}
[modify] https://crrev.com/26e2572987b0e3a147911c1aa26f7f747968cc01/third_party/blink/renderer/core/paint/object_paint_properties.h
[modify] https://crrev.com/26e2572987b0e3a147911c1aa26f7f747968cc01/third_party/blink/renderer/core/paint/paint_layer_clipper.cc
[modify] https://crrev.com/26e2572987b0e3a147911c1aa26f7f747968cc01/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
[modify] https://crrev.com/26e2572987b0e3a147911c1aa26f7f747968cc01/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc
[modify] https://crrev.com/26e2572987b0e3a147911c1aa26f7f747968cc01/third_party/blink/renderer/platform/graphics/paint/clip_paint_property_node.h

Comment 6 by pdr@chromium.org, Apr 27 2018

Status: Fixed (was: Assigned)
This has been fixed. The MacOS-specific regressions are now gone.

Sign in to add a comment