8.1%-23.5% regression in system_health.common_desktop at 508286:508346 |
|||||||||
Issue descriptionSee the link to graphs below.
,
Oct 20 2017
๐ Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/12e0d927780000
,
Oct 20 2017
๐ฟ Pinpoint job stopped with an error. https://pinpoint-dot-chromeperf.appspot.com/job/12e0d927780000
,
Oct 20 2017
๐ Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14e9b2b7780000
,
Oct 21 2017
๐ Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/14e9b2b7780000 Prevent squashing of border-radius layers By schenney@chromium.org ยท Thu Oct 12 13:11:52 2017 chromium @ 8ff2371257ac4f24d77da4a62569840a53c49b47 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Oct 23 2017
I'll look into whether we can use a smaller hammer to fix the rendering error.
,
Oct 23 2017
,
Oct 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2faf4aa8f809aee54bd228d0fb375bbd67b4d2be commit 2faf4aa8f809aee54bd228d0fb375bbd67b4d2be Author: Stephen Chenney <schenney@chromium.org> Date: Wed Oct 25 18:58:49 2017 Tighten the restriction on squashing for border radius The fix for https://bugs.chromium.org/p/chromium/issues/detail?id=761298 prevents sqaushing of anything that has a border radius, but in reality we only need to prevent squashing when the layer actually clips a child composited layer. The composited part is not known at the time we compute squashing disallowed reasons, so we can't use that. But we can make sure that overflow does clip. R=chrishtr@chromium.org BUG= 776736 , 774544 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I0ec1b8d8e2c0ebfef5004f313d49ae41f14e647e Reviewed-on: https://chromium-review.googlesource.com/734140 Commit-Queue: Stephen Chenney <schenney@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#511540} [modify] https://crrev.com/2faf4aa8f809aee54bd228d0fb375bbd67b4d2be/third_party/WebKit/Source/core/paint/compositing/CompositingLayerAssigner.cpp
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c24e45d72fd1003e4b49341650d9c40b709752c2 commit c24e45d72fd1003e4b49341650d9c40b709752c2 Author: Stephen Chenney <schenney@chromium.org> Date: Fri Nov 03 19:25:48 2017 Use sibling HasInlineTransform to enable more squashing Traverse previous siblings of a layer looking for inline transforms as a way to enable more squashing when border radius is present. This targets the squashing restriction at the specific case that generated the original bug, and does not cause issues in other tests (that is, next siblings do not cause the original bug and only inline transforms cause the original bug). Telemetry tests on MacOS show a 3% or more improvement on time to first paint for load tests. TBR=chrishtr@chromium.org Bug: 776736 , 774544 , 780244 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I1f9973e5c52d40a47c05dd2d8310bb66de3e0dfa Reviewed-on: https://chromium-review.googlesource.com/743694 Commit-Queue: Stephen Chenney <schenney@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#513869} [modify] https://crrev.com/c24e45d72fd1003e4b49341650d9c40b709752c2/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/c24e45d72fd1003e4b49341650d9c40b709752c2/third_party/WebKit/Source/core/paint/PaintLayer.h [modify] https://crrev.com/c24e45d72fd1003e4b49341650d9c40b709752c2/third_party/WebKit/Source/core/paint/compositing/CompositingLayerAssigner.cpp [modify] https://crrev.com/c24e45d72fd1003e4b49341650d9c40b709752c2/third_party/WebKit/Source/core/paint/compositing/CompositingRequirementsUpdater.cpp
,
Nov 8 2017
The patch from comment #9, that really improved performance, appears first in 64.0.3258.0 The patch from comment #8, which is minimally helpful, appears first in 64.0.3250.0 The patch that regressed things horribly appeared first in 63.0.3239.0. We're seeing major paint update time regressions in UMA data, https://bugs.chromium.org/p/chromium/issues/detail?id=782353 So requesting merge of both patches to M-63 after a little time to bake on Canary.
,
Nov 8 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 8 2017
Pls apply appropriate OSs label.
,
Nov 8 2017
Please do not merge c24e45d72fd1003e4b49341650d9c40b709752c2, this commit is still under discussion.
,
Nov 8 2017
We're not merging anything yet, but are asking for approval in the event the team decides it's safe, and to bring it to release manager's attention.
,
Nov 8 2017
,
Nov 14 2017
A decision has been made to revert this and two earlier patches, and one of those patches should be reverted on the M-63 branch. I propose we revert them all on trunk and then merge back the ones we need for M-63.
,
Nov 14 2017
Thank you schenney@. Please update the bug once revert looks good to merge to M63 after change is well baked/verified in canary and safe to merge to M63.
,
Nov 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/90bb5240fe42935776901f33bc7c515e8c24da1c commit 90bb5240fe42935776901f33bc7c515e8c24da1c Author: Stephen Chenney <schenney@chromium.org> Date: Wed Nov 15 06:17:32 2017 Revert "Use sibling HasInlineTransform to enable more squashing" This reverts commit c24e45d72fd1003e4b49341650d9c40b709752c2. Reason for revert: This does not fix all cases. The plan is to revert all the patches trying to fix the underlying border radius and squashing issue and fix it correctly. Original change's description: > Use sibling HasInlineTransform to enable more squashing > > Traverse previous siblings of a layer looking for inline transforms > as a way to enable more squashing when border radius is present. > This targets the squashing restriction at the specific case that > generated the original bug, and does not cause issues in other > tests (that is, next siblings do not cause the original bug and > only inline transforms cause the original bug). > > Telemetry tests on MacOS show a 3% or more improvement on time > to first paint for load tests. > > TBR=chrishtr@chromium.org > > Bug: 776736 , 774544 , 780244 > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 > Change-Id: I1f9973e5c52d40a47c05dd2d8310bb66de3e0dfa > Reviewed-on: https://chromium-review.googlesource.com/743694 > Commit-Queue: Stephen Chenney <schenney@chromium.org> > Reviewed-by: Philip Rogers <pdr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#513869} TBR=pdr@chromium.org,chrishtr@chromium.org,schenney@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 776736 , 774544 , 780244 Change-Id: I3e620a3c12c0f383bb3f57189afb8e3f05e9f960 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Reviewed-on: https://chromium-review.googlesource.com/770230 Reviewed-by: Stephen Chenney <schenney@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#516598} [modify] https://crrev.com/90bb5240fe42935776901f33bc7c515e8c24da1c/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/90bb5240fe42935776901f33bc7c515e8c24da1c/third_party/WebKit/Source/core/paint/PaintLayer.h [modify] https://crrev.com/90bb5240fe42935776901f33bc7c515e8c24da1c/third_party/WebKit/Source/core/paint/compositing/CompositingLayerAssigner.cpp [modify] https://crrev.com/90bb5240fe42935776901f33bc7c515e8c24da1c/third_party/WebKit/Source/core/paint/compositing/CompositingRequirementsUpdater.cpp
,
Nov 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/152e7cf991c83b8ebac89db3f67cfe80511d47e5 commit 152e7cf991c83b8ebac89db3f67cfe80511d47e5 Author: Stephen Chenney <schenney@chromium.org> Date: Wed Nov 15 18:58:04 2017 Revert "Tighten the restriction on squashing for border radius" This reverts commit 2faf4aa8f809aee54bd228d0fb375bbd67b4d2be. Reason for revert: <INSERT REASONING HERE> Original change's description: > Tighten the restriction on squashing for border radius > > The fix for https://bugs.chromium.org/p/chromium/issues/detail?id=761298 > prevents sqaushing of anything that has a border radius, but in reality > we only need to prevent squashing when the layer actually clips a > child composited layer. The composited part is not known at the > time we compute squashing disallowed reasons, so we can't use that. But > we can make sure that overflow does clip. > > R=โchrishtr@chromium.org > BUG= 776736 , 774544 > > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 > Change-Id: I0ec1b8d8e2c0ebfef5004f313d49ae41f14e647e > Reviewed-on: https://chromium-review.googlesource.com/734140 > Commit-Queue: Stephen Chenney <schenney@chromium.org> > Reviewed-by: Chris Harrelson <chrishtr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#511540} TBR=chrishtr@chromium.org,schenney@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 776736 , 774544 Change-Id: I9092431dd162191b5ee92f64efb45aba164c382e Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Reviewed-on: https://chromium-review.googlesource.com/771990 Reviewed-by: Stephen Chenney <schenney@chromium.org> Commit-Queue: Stephen Chenney <schenney@chromium.org> Cr-Commit-Position: refs/heads/master@{#516768} [modify] https://crrev.com/152e7cf991c83b8ebac89db3f67cfe80511d47e5/third_party/WebKit/Source/core/paint/compositing/CompositingLayerAssigner.cpp
,
Nov 16 2017
What is the merge review for? There are many things going on here and it is confusing. Regrading M63, please be explicit about: * What CL you are planning to merge into M63 and why? * What CL you are planning to revert from M63 and why?
,
Nov 16 2017
Sorry for the confusion. The Merge-Review is not for anything here - these are patches that needed to be reverted in order to revert the patch of interest. Head on over to https://bugs.chromium.org/p/chromium/issues/detail?id=761298 for the correct merge request.
,
Nov 16 2017
,
Jan 18 2018
schenney: Thanks for all your hard work here! Looks like bug 761298 was merged. Is there further work to do on this bug? Or should we mark Fixed?
,
Jan 18 2018
Yep, time to mark Fixed. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Oct 20 2017