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

Issue 697081 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

5~75% regression in mean_pixels_checkerboarded on smoothness.tough_scrolling_cases on Win 10 and Mac Retina

Project Member Reported by ajuma@chromium.org, Feb 28 2017

Issue description

This regressed somewhere in the range http://test-results.appspot.com/revision_range?start=445757&end=445835

Going to run a bisect. A first attempt was http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64_10_perf_bisect/builds/896 but that didn't produce easy-to-parse output.
 
Cc: wangxianzhu@chromium.org
Owner: wangxianzhu@chromium.org

=== Auto-CCing suspected CL author wangxianzhu@chromium.org ===

Hi wangxianzhu@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : wangxianzhu
  Commit : 9686be68e53348bab6f4ce846bf07d494f1575a0
  Date   : Tue Jan 24 19:34:26 2017
  Subject: Enable SlimmingPaintInvalidation

Bisect Details
  Configuration: winx64_10_perf_bisect
  Benchmark    : smoothness.tough_scrolling_cases
  Metric       : mean_pixels_checkerboarded/mean_pixels_checkerboarded
  Change       : 3.69% | 14.2472625 -> 14.7725666667

Revision             Result                   N
chromium@445756      14.2473 +- 0.470153      6      good
chromium@445776      14.1435 +- 0.34458       6      good
chromium@445777      14.3197 +- 0.516706      9      good
chromium@445778      14.7509 +- 0.861672      9      bad       <--
chromium@445779      14.7832 +- 0.214041      6      bad
chromium@445781      14.6512 +- 0.363545      6      bad
chromium@445786      14.6996 +- 0.759984      9      bad
chromium@445796      14.6908 +- 0.67232       9      bad
chromium@445835      14.7726 +- 0.386628      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests smoothness.tough_scrolling_cases

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8986398123626414032

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5255268172038144


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection.  Thank you!
Cc: ajuma@chromium.org

Comment 4 by ajuma@chromium.org, Mar 1 2017

Cc: wkorman@chromium.org pdr@chromium.org chrishtr@chromium.org
Components: Blink>Paint
Labels: -Pri-2 M-58 OS-Mac Pri-1
There's also a significant increase on retina Mac on the same metric with the same regression range (checkerboarding went from ~2% before to ~3.5% after). Graphs for Mac and Windows:
https://chromeperf.appspot.com/report?sid=174409728d9eed51948bec518757b550f8b25e90f6435c66a202dab33fe9e70f&start_rev=441062&end_rev=453894

Are we maybe invalidating more that we did before? Or is the main thread taking longer than before?
Labels: BugSource-Team PaintTeamTriaged-20170228
Summary: 5~75% regression in mean_pixels_checkerboarded on smoothness.tough_scrolling_cases on Win 10 and Mac Retina (was: ~5% regression in mean_pixels_checkerboarded on smoothness.tough_scrolling_cases on Win 10)
Looked into the sub metrics and found that the overall regression is mostly because of regression of text_constant_full_page_raster_NNNNN_pixels_per_second:

https://chromeperf.appspot.com/report?sid=9927f2266bb7e043be7e9b1e1017d4e5a159edd18cd23ceb7ff8c9357876e6c5&start_rev=443812&end_rev=453602

=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : wangxianzhu
  Commit : 9686be68e53348bab6f4ce846bf07d494f1575a0
  Date   : Tue Jan 24 19:34:26 2017
  Subject: Enable SlimmingPaintInvalidation

Bisect Details
  Configuration: mac_retina_perf_bisect
  Benchmark    : smoothness.tough_scrolling_cases
  Metric       : mean_pixels_checkerboarded/mean_pixels_checkerboarded
  Change       : 102.71% | 1.6401375 -> 3.3247

Revision             Result                   N
chromium@445697      1.64014 +- 0.295377      6      good
chromium@445769      2.16841 +- 1.07432       9      good
chromium@445774      2.09561 +- 0.875174      6      good
chromium@445776      1.89887 +- 0.716698      6      good
chromium@445777      1.94826 +- 0.612657      5      good
chromium@445778      3.83673 +- 0.932088      6      bad       <--
chromium@445787      3.15973 +- 1.1855        6      bad
chromium@445805      4.06895 +- 0.53386       6      bad
chromium@445841      3.3247 +- 1.39642        6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests smoothness.tough_scrolling_cases

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8986305900953062112

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5796504314314752


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection.  Thank you!
Labels: -M-58 M-59
Now we target M59 for spinvalidation.
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 17 2017

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

commit cabebfee33715afb9e577f6d133898b408390ab1
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Fri Mar 17 17:43:25 2017

Add 'WithoutGeometryChange' variants of paint invalidation flag setters

Add a flag needsPaintOffsetAndVisualRectUpdate() to indicate that the
object needs paint offset and visual rect update during
PrePaintTreeWalk.

The original paint invalidation flag setters setMayNeedPaintInvalidation,
setShouldDoFullPaintInvalidaiton and setters calling the two will set
the new flag besides the original flags.

Add 'WithoutGeometryChange' variants of the two setters to allow
PrePaintTreeWalk to skip paint offset and visual rect update (to be
implemented in later CLs).

BUG= 697081 ,685179

Review-Url: https://codereview.chromium.org/2751383003
Cr-Commit-Position: refs/heads/master@{#457809}

[modify] https://crrev.com/cabebfee33715afb9e577f6d133898b408390ab1/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/cabebfee33715afb9e577f6d133898b408390ab1/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/cabebfee33715afb9e577f6d133898b408390ab1/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/cabebfee33715afb9e577f6d133898b408390ab1/third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 18 2017

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

commit 5b49d1384ce8d1a8594bdb49f9f392da784b43bf
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Sat Mar 18 02:20:11 2017

Reland of Add 'WithoutGeometryChange' variants of paint invalidation flag setters (patchset #1 id:1 of https://codereview.chromium.org/2752093005/ )

Fix the issue by changing a setNeedsPaintOffsetAndVisualRectUpdate() call
to setMayNeedPaintInvaldiation() call.

Original issue's description:
> Revert of Add 'WithoutGeometryChange' variants of paint invalidation flag setters (patchset #2 id:20001 of https://codereview.chromium.org/2751383003/ )
>
> Reason for revert:
> Broke browser_tests: https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/23685
>
> https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/23684
>
> Original issue's description:
> > Add 'WithoutGeometryChange' variants of paint invalidation flag setters
> >
> > Add a flag needsPaintOffsetAndVisualRectUpdate() to indicate that the
> > object needs paint offset and visual rect update during
> > PrePaintTreeWalk.
> >
> > The original paint invalidation flag setters setMayNeedPaintInvalidation,
> > setShouldDoFullPaintInvalidaiton and setters calling the two will set
> > the new flag besides the original flags.
> >
> > Add 'WithoutGeometryChange' variants of the two setters to allow
> > PrePaintTreeWalk to skip paint offset and visual rect update (to be
> > implemented in later CLs).
> >
> > BUG= 697081 ,685179
> >
> > Review-Url: https://codereview.chromium.org/2751383003
> > Cr-Commit-Position: refs/heads/master@{#457809}
> > Committed: https://chromium.googlesource.com/chromium/src/+/cabebfee33715afb9e577f6d133898b408390ab1
>
> TBR=pdr@chromium.org,wangxianzhu@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 697081 ,685179
>
> Review-Url: https://codereview.chromium.org/2752093005
> Cr-Commit-Position: refs/heads/master@{#457872}
> Committed: https://chromium.googlesource.com/chromium/src/+/75c75092b256d5e3e0ae7ecafd1f8b3430246e1b

TBR=pdr@chromium.org,dimich@chromium.org
BUG= 697081 ,685179

Review-Url: https://codereview.chromium.org/2751523011
Cr-Commit-Position: refs/heads/master@{#457928}

[modify] https://crrev.com/5b49d1384ce8d1a8594bdb49f9f392da784b43bf/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/5b49d1384ce8d1a8594bdb49f9f392da784b43bf/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/5b49d1384ce8d1a8594bdb49f9f392da784b43bf/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/5b49d1384ce8d1a8594bdb49f9f392da784b43bf/third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp

Blockedon: 704732
Blockedon: -704732
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 2 2017

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

commit 81f28ed19f771fe93406496d1d23da137abcd602
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Sun Apr 02 21:45:28 2017

Skip paint property update and visual rect update if no geometry change

This avoids unnecessary visual rect computation and update of paint
offset related paint properties.

BUG= 697081 ,685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2732573003
Cr-Commit-Position: refs/heads/master@{#461348}

[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/layout/PaintInvalidationState.h
[add] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/FindPaintOffsetAndVisualRectNeedingUpdate.h
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/PaintInvalidator.h
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
[modify] https://crrev.com/81f28ed19f771fe93406496d1d23da137abcd602/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 3 2017

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

commit 6d954094c746673d37970ab1321e810622c8fd49
Author: timloh <timloh@chromium.org>
Date: Mon Apr 03 01:43:21 2017

Revert of Skip paint property update and visual rect update if no geometry change (patchset #25 id:480001 of https://codereview.chromium.org/2732573003/ )

Reason for revert:
Looks like this is making tests fail on most builders, e.g.

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty/builds/25101

Original issue's description:
> Skip paint property update and visual rect update if no geometry change
>
> This avoids unnecessary visual rect computation and update of paint
> offset related paint properties.
>
> BUG= 697081 ,685179
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
>
> Review-Url: https://codereview.chromium.org/2732573003
> Cr-Commit-Position: refs/heads/master@{#461348}
> Committed: https://chromium.googlesource.com/chromium/src/+/81f28ed19f771fe93406496d1d23da137abcd602

TBR=pdr@chromium.org,chrishtr@chromium.org,wangxianzhu@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 697081 ,685179

Review-Url: https://codereview.chromium.org/2792863002
Cr-Commit-Position: refs/heads/master@{#461357}

[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/layout/PaintInvalidationState.h
[delete] https://crrev.com/542dc4c9f70f7c9e1a3ec096c6b62bceceb5e47f/third_party/WebKit/Source/core/paint/FindPaintOffsetAndVisualRectNeedingUpdate.h
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/PaintInvalidator.h
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
[modify] https://crrev.com/6d954094c746673d37970ab1321e810622c8fd49/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 4 2017

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

commit d2819bd5ebfb32f0866d92ad6e62a45499e05517
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Tue Apr 04 23:13:31 2017

Reland of Skip paint property update and visual rect update if no geometry change (patchset #1 id:1 of https://codereview.chromium.org/2792863002/ )

Reland after https://codereview.chromium.org/2791933003/ which fixed the
layout test failures (about overflow recalc).

Also fixed a bug of false-negative of visual rect change in
FindPaintOffsetAndVisualRectNeedingUpdate.h.

Original issue's description:
> Revert of Skip paint property update and visual rect update if no geometry change (patchset #25 id:480001 of https://codereview.chromium.org/2732573003/ )
>
> Reason for revert:
> Looks like this is making tests fail on most builders, e.g.
>
> https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty/builds/25101
>
> Original issue's description:
> > Skip paint property update and visual rect update if no geometry change
> >
> > This avoids unnecessary visual rect computation and update of paint
> > offset related paint properties.
> >
> > BUG= 697081 ,685179
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> >
> > Review-Url: https://codereview.chromium.org/2732573003
> > Cr-Commit-Position: refs/heads/master@{#461348}
> > Committed: https://chromium.googlesource.com/chromium/src/+/81f28ed19f771fe93406496d1d23da137abcd602
>
> TBR=pdr@chromium.org,chrishtr@chromium.org,wangxianzhu@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 697081 ,685179
>
> Review-Url: https://codereview.chromium.org/2792863002
> Cr-Commit-Position: refs/heads/master@{#461357}
> Committed: https://chromium.googlesource.com/chromium/src/+/6d954094c746673d37970ab1321e810622c8fd49

TBR=pdr@chromium.org
BUG= 697081 ,685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2792063002
Cr-Commit-Position: refs/heads/master@{#461885}

[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/layout/PaintInvalidationState.h
[add] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/FindPaintOffsetAndVisualRectNeedingUpdate.h
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/PaintInvalidator.h
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
[modify] https://crrev.com/d2819bd5ebfb32f0866d92ad6e62a45499e05517/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h

Status: Fixed (was: Assigned)
Performance recovered.

Sign in to add a comment