New issue
Advanced search Search tips

Issue 895109 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

UpdateLayerTree is very slow on Google Drive and makes animations janky

Reported by espr...@gmail.com, Oct 13

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS x86_64 10895.56.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.95 Safari/537.36

Steps to reproduce the problem:
1. Load Google Drive: https://drive.google.com/drive/my-drive
2. Scroll up and down so the header shows and hides.

What is the expected behavior?
Should be fast.

What went wrong?
UpdateLayerTree takes up 40ms per frame on the main thread. The animation is very slow.

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 69.0.3497.95  Channel: n/a
OS Version: 10895.56.0
Flash Version:
 
Profile-20181013T143058.zip
705 KB Download
Screenshot 2018-10-13 at 2.36.48 PM.png
66.8 KB View Download
Screenshot 2018-10-13 at 2.37.10 PM.png
88.1 KB View Download
Btw this is on a high end Chromebook Pixelbook device.
Can someone on the team with a Pixelbook take a look? Mine is ordered but not yet here.
Cc: masonfreed@chromium.org chrishtr@chromium.org pdr@chromium.org vmp...@chromium.org
Bump. Anyone with a Pixelbook to confirm this?
Labels: Needs-Bisect
Double-bump. Maybe someone could recommend someone else that might have a pixelbook, and I'll cc them here?
This should be reproducible on probably any chromebook (certainly any high dpi one). Try a Samus or a Caroline?
chrishtr@, I marked this with Needs-Bisect yesterday - that's the correct tag to get someone to bisect it overnight, correct?
Labels: -Needs-Bisect
I can reproduce the slowness on Linux. Removing needs-bisect.

For future reference:
1. Needs-Bisect is not supported on ChromeOS, due to lack of a script to do per-revision bisects.
2. You must add a milestone label for Needs-Bisect to be noticed by the testers.
The reason for the compositing update step in this case is because we currently
have a slow path that invalidates sticky constraints and re-computes compositing
inputs on scroll. The reason the compositing inputs are slow to compute is that
there are a lot of PaintLayers on the page.
Owner: chrishtr@chromium.org
Status: Assigned (was: Unconfirmed)
Second, even for composited layers we do a GraphicsLayerUpdate steps
if it's not the root composited layer.

Neither of these reasons should be required over time and we are continuously
optimizing.

I think the first can actually be removed now. It was added in https://chromium-review.googlesource.com/c/chromium/src/+/994462/ because sticky offset was at the
time baked into paint offset. That has sinced been fixed as part of the BGPT milestone
in https://chromium-review.googlesource.com/c/chromium/src/+/994462/.

Will try to remove that now.

The second slowness I will have to look into further.
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 22

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

commit 25f5e3880294e41f2f0c23923b281dfb7f2dc237
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Thu Nov 22 05:08:15 2018

Stop re-computing compositing inputs for scrollers with sticky
descendants.

This was added in revision 548120 because at the time paint
offset baked in sticky position. This was since fixed in revision
590578 that factored sticky position into a transform paint
property node rather than paint offset.

All that is required is to re-compute paint properties for the
scroller and all sticky descendants, in order to update the scroll
offset and sticky transform offset. The latter is accomplished
by a call to PLSA::InvalidatePaintForStickyDescendants() in
PLSA::InvalidatePaintForScrollOffsetChange().

Bug:  895109 

Change-Id: Ied7ef8fd15db4f5c3d546f0f06f1adf3cf1e26c8
Reviewed-on: https://chromium-review.googlesource.com/c/1345031
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610305}
[modify] https://crrev.com/25f5e3880294e41f2f0c23923b281dfb7f2dc237/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 16

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

commit 38c20f78b9826529f672e6db5baec01a60c2260f
Author: Mason Freed <masonfreed@chromium.org>
Date: Wed Jan 16 01:30:35 2019

Remove extra calls to SetNeedsGraphicsLayerUpdate

In non-BGPT mode, we only need to do a kGraphicsLayerUpdateLocal update,
and in BGPT mode, we now call SetNeedsCommit() directly from
PropertyTreeManager::EnsureCompositorTransformNode, so no graphics layer
update is needed at all.

Bug:  895109 
Change-Id: I970b29bc05a43720cfb9747bfcfa50e42dc8639e
Reviewed-on: https://chromium-review.googlesource.com/c/1403639
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622979}
[modify] https://crrev.com/38c20f78b9826529f672e6db5baec01a60c2260f/cc/layers/layer.h
[modify] https://crrev.com/38c20f78b9826529f672e6db5baec01a60c2260f/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc
[modify] https://crrev.com/38c20f78b9826529f672e6db5baec01a60c2260f/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/38c20f78b9826529f672e6db5baec01a60c2260f/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc
[modify] https://crrev.com/38c20f78b9826529f672e6db5baec01a60c2260f/third_party/blink/web_tests/fast/events/touch/compositor-touch-hit-rects-scroll-expected.txt
[modify] https://crrev.com/38c20f78b9826529f672e6db5baec01a60c2260f/third_party/blink/web_tests/paint/invalidation/compositing/scrolling-neg-z-index-descendants-expected.txt
[add] https://crrev.com/38c20f78b9826529f672e6db5baec01a60c2260f/third_party/blink/web_tests/virtual/stable/paint/invalidation/compositing/scrolling-neg-z-index-descendants-expected.txt

Comment 14 by chrishtr@chromium.org, Jan 18 (5 days ago)

Status: Fixed (was: Assigned)

Sign in to add a comment