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

Issue 704728 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

2.1%-45% regression in smoothness.sync_scroll.key_mobile_sites_smooth at 457406:458752

Project Member Reported by jasontiller@chromium.org, Mar 23 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Mar 24 2017

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 : 4e0d0f9ddd68e59dccda0a8c9a096c1813a1bd44
  Date   : Tue Mar 21 05:00:51 2017
  Subject: Add StyleDifference::needsVisualRectUpdate

Bisect Details
  Configuration: android_nexus5_perf_bisect
  Benchmark    : smoothness.sync_scroll.key_mobile_sites_smooth
  Metric       : mean_input_event_latency/http___mobile-news.sandbox.google.com_news_pt0
  Change       : 37.94% | 23.8551666667 -> 32.9051666667

Revision             Result                   N
chromium@457405      23.8552 +- 3.6566        6      good
chromium@458079      25.8462 +- 9.31702       9      good
chromium@458248      24.5262 +- 4.33493       6      good
chromium@458290      25.0933 +- 5.43105       6      good
chromium@458311      23.5018 +- 0.972929      6      good
chromium@458312      24.0252 +- 2.76043       6      good
chromium@458313      33.6768 +- 1.78101       6      bad       <--
chromium@458314      33.8847 +- 1.65937       6      bad
chromium@458317      34.2155 +- 1.31045       6      bad
chromium@458322      34.4253 +- 1.28576       6      bad
chromium@458332      33.4378 +- 0.839519      6      bad
chromium@458416      34.2807 +- 1.55254       6      bad
chromium@458752      32.9052 +- 4.30205       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...mobile.news.sandbox.google.com.news.pt0 smoothness.sync_scroll.key_mobile_sites_smooth

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

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


| 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!
Components: Blink>Paint
Status: Assigned (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 27 2017

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

commit bedba279ba5dc64257efc7bc8a13d697c049d3de
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Mon Mar 27 18:11:13 2017

Revert of Add StyleDifference::needsVisualRectUpdate (patchset #5 id:80001 of https://codereview.chromium.org/2761863002/ )

Reason for revert:
BUG= 704728 

Original issue's description:
> Add StyleDifference::needsVisualRectUpdate
>
> It's set when a style change will change visual rect (without changing
> layout, as visual rect update is implied by layout change).
>
> BTW optimized paint invalidation of box with resizer. We no longer
> force full paint invalidation for box with resizer when resized.
> The resizer will be invlaidated in PaintInvalidationCapableScrollableArea
> if needed.
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
>
> Review-Url: https://codereview.chromium.org/2761863002
> Cr-Commit-Position: refs/heads/master@{#458313}
> Committed: https://chromium.googlesource.com/chromium/src/+/4e0d0f9ddd68e59dccda0a8c9a096c1813a1bd44

TBR=pdr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.

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

[modify] https://crrev.com/bedba279ba5dc64257efc7bc8a13d697c049d3de/third_party/WebKit/LayoutTests/paint/invalidation/textarea-appearance-none-resize-handle-expected.txt
[modify] https://crrev.com/bedba279ba5dc64257efc7bc8a13d697c049d3de/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/bedba279ba5dc64257efc7bc8a13d697c049d3de/third_party/WebKit/Source/core/paint/BoxPaintInvalidator.cpp
[modify] https://crrev.com/bedba279ba5dc64257efc7bc8a13d697c049d3de/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/bedba279ba5dc64257efc7bc8a13d697c049d3de/third_party/WebKit/Source/core/style/ComputedStyle.h
[modify] https://crrev.com/bedba279ba5dc64257efc7bc8a13d697c049d3de/third_party/WebKit/Source/core/style/StyleDifference.h

Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Mar 28 2017


=== BISECT JOB RESULTS ===
NO Perf regression found

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : smoothness.sync_scroll.key_mobile_sites_smooth
  Metric       : frame_times/http___mobile-news.sandbox.google.com_news_pt0

Revision             Result                   N
chromium@459831      16.8619 +- 0.384957      21      good
chromium@459846      16.8509 +- 0.407055      21      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...mobile.news.sandbox.google.com.news.pt0 smoothness.sync_scroll.key_mobile_sites_smooth

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

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


| 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!
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Mar 28 2017


=== BISECT JOB RESULTS ===
Perf regression found but unable to continue

Bisect was stopped because a commit couldn't be classified as either
good or bad.


Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : smoothness.sync_scroll.key_mobile_sites_smooth
  Metric       : frame_times/http___mobile-news.sandbox.google.com_news_pt0

Revision             Result                   N
chromium@459831      16.8438 +- 0.27427       21      good
chromium@459839      16.8369 +- 0.304222      21      unknown
chromium@459846      16.8847 +- 0.249712      21      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...mobile.news.sandbox.google.com.news.pt0 smoothness.sync_scroll.key_mobile_sites_smooth

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

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


| 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: BugSource-Chromium PaintTeamTriaged-20170327
Project Member

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

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

commit 36f76920b08dcd81e97f74afd04474d88f0441e2
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Fri Mar 31 20:43:08 2017

Reland of Add StyleDifference::needsVisualRectUpdate (patchset #1 id:1 of https://codereview.chromium.org/2772353002/ )

Abandon the changes about StyleDifference::
m_scrollAnchorDisablingPropertyChanged in the original patch
to avoid performance regressions.

Original issue's description:
> Revert of Add StyleDifference::needsVisualRectUpdate (patchset #5 id:80001 of https://codereview.chromium.org/2761863002/ )
>
> Reason for revert:
> BUG= 704728 
>
> Original issue's description:
> > Add StyleDifference::needsVisualRectUpdate
> >
> > It's set when a style change will change visual rect (without changing
> > layout, as visual rect update is implied by layout change).
> >
> > BTW optimized paint invalidation of box with resizer. We no longer
> > force full paint invalidation for box with resizer when resized.
> > The resizer will be invlaidated in PaintInvalidationCapableScrollableArea
> > if needed.
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> >
> > Review-Url: https://codereview.chromium.org/2761863002
> > Cr-Commit-Position: refs/heads/master@{#458313}
> > Committed: https://chromium.googlesource.com/chromium/src/+/4e0d0f9ddd68e59dccda0a8c9a096c1813a1bd44
>
> TBR=pdr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
>
> Review-Url: https://codereview.chromium.org/2772353002
> Cr-Commit-Position: refs/heads/master@{#459827}
> Committed: https://chromium.googlesource.com/chromium/src/+/bedba279ba5dc64257efc7bc8a13d697c049d3de

TBR=pdr@chromium.org
BUG= 704728 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/LayoutTests/paint/invalidation/textarea-appearance-none-resize-handle-expected.txt
[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/Source/core/paint/BoxPaintInvalidator.cpp
[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/Source/core/style/ComputedStyle.h
[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/Source/core/style/StyleDifference.cpp
[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/Source/core/style/StyleDifference.h
[modify] https://crrev.com/36f76920b08dcd81e97f74afd04474d88f0441e2/third_party/WebKit/Source/core/style/StyleDifferenceTest.cpp

Status: Fixed (was: Assigned)

Sign in to add a comment