New issue
Advanced search Search tips

Issue 639377 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

12.9% regression in smoothness.top_25_smooth at 412759:412784

Project Member Reported by lanwei@google.com, Aug 19 2016

Issue description

See the link to graphs below.
 

Comment 1 by lanwei@google.com, Aug 19 2016

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=639377

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgzve6rQkM


Bot(s) for this bug's original alert(s):

android-nexus5X
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Aug 19 2016

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 below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Tweak priorities of paint invalidation reasons
Author  : wangxianzhu
Commit description:
  
1. Distinguish PaintInvalidationNone and PaintInvalidationProbablyNone,
   so that callers of ObjectPaintInvalidator::computePaintInvalidationReasion()
   can treat them differently to avoid unnecessary overriding
   cases that definitely no paint invalidation is needed.

2. Check emptiness of newBounds and oldBounds earlier. This avoids
   unnecessary object and layer paint invalidations when both oldBounds
   and newBounds are empty but we computed a non-none paint invalidation
   reason.

3. Renamed LayoutObject::paintedOutputOfObjectHasNoEffect to
   paintedOutputOfObjectHasNoEffectRegardlessOfSize because it doesn't check
   for empty size. The behavior is required to do correct paint invalidation
   when the size changes to empty. Now the name is better suited to the
   actual behavior.

These changes will help spv1 paint invalidation and GeometryMapper-based
spv2 paint invalidation to produce the same result.

BUG= 510908 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2247543003
Cr-Commit-Position: refs/heads/master@{#412763}
Commit  : bb6578fd92bd2477a4d781aa51ef285367617eee
Date    : Thu Aug 18 08:13:21 2016


===== TESTED REVISIONS =====
Revision         Mean    Std Dev    N  Good?
chromium@412758  6.7494  0.0920152  5  good
chromium@412762  6.7798  0.125284   5  good
chromium@412763  7.579   0.256203   5  bad    <--
chromium@412764  7.6198  0.266591   5  bad
chromium@412766  7.6846  0.12054    5  bad
chromium@412774  7.7844  0.2797     5  bad

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 639377

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.top_25_smooth
Test Metric: mean_input_event_latency/http___games.yahoo.com
Relative Change: 15.33%
Score: 99.8

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/541
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003876229611863616


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5225341866475520

| 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 Tests>AutoBisect.  Thank you!
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 20 2016

Labels: Hotlist-Google
Status: Started (was: Assigned)
Summary: 12.9% regression in smoothness.top_25_smooth at 412759:412784 (was: 12.9% regression in smoothness.top_25_smooth at 412775:412796)
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Aug 24 2016


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Tweak priorities of paint invalidation reasons
Author  : wangxianzhu
Commit description:
  
1. Distinguish PaintInvalidationNone and PaintInvalidationProbablyNone,
   so that callers of ObjectPaintInvalidator::computePaintInvalidationReasion()
   can treat them differently to avoid unnecessary overriding
   cases that definitely no paint invalidation is needed.

2. Check emptiness of newBounds and oldBounds earlier. This avoids
   unnecessary object and layer paint invalidations when both oldBounds
   and newBounds are empty but we computed a non-none paint invalidation
   reason.

3. Renamed LayoutObject::paintedOutputOfObjectHasNoEffect to
   paintedOutputOfObjectHasNoEffectRegardlessOfSize because it doesn't check
   for empty size. The behavior is required to do correct paint invalidation
   when the size changes to empty. Now the name is better suited to the
   actual behavior.

These changes will help spv1 paint invalidation and GeometryMapper-based
spv2 paint invalidation to produce the same result.

BUG= 510908 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2247543003
Cr-Commit-Position: refs/heads/master@{#412763}
Commit  : bb6578fd92bd2477a4d781aa51ef285367617eee
Date    : Thu Aug 18 08:13:21 2016


===== TESTED REVISIONS =====
Revision         Mean    Std Dev   N  Good?
chromium@412758  6.8482  0.103652  5  good
chromium@412762  6.922   0.129138  5  good
chromium@412763  7.7604  0.217783  5  bad    <--
chromium@412764  7.7962  0.32195   5  bad
chromium@412766  7.6342  0.290102  5  bad
chromium@412774  7.8818  0.26666   5  bad

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 639377

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.top_25_smooth
Test Metric: mean_input_event_latency/http___games.yahoo.com
Relative Change: 15.09%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/589
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003505201585175776


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5784462688780288

| 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 Tests>AutoBisect.  Thank you!
Project Member

Comment 11 by 42576172...@developer.gserviceaccount.com, Aug 24 2016


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Tweak priorities of paint invalidation reasons
Author  : wangxianzhu
Commit description:
  
1. Distinguish PaintInvalidationNone and PaintInvalidationProbablyNone,
   so that callers of ObjectPaintInvalidator::computePaintInvalidationReasion()
   can treat them differently to avoid unnecessary overriding
   cases that definitely no paint invalidation is needed.

2. Check emptiness of newBounds and oldBounds earlier. This avoids
   unnecessary object and layer paint invalidations when both oldBounds
   and newBounds are empty but we computed a non-none paint invalidation
   reason.

3. Renamed LayoutObject::paintedOutputOfObjectHasNoEffect to
   paintedOutputOfObjectHasNoEffectRegardlessOfSize because it doesn't check
   for empty size. The behavior is required to do correct paint invalidation
   when the size changes to empty. Now the name is better suited to the
   actual behavior.

These changes will help spv1 paint invalidation and GeometryMapper-based
spv2 paint invalidation to produce the same result.

BUG= 510908 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2247543003
Cr-Commit-Position: refs/heads/master@{#412763}
Commit  : bb6578fd92bd2477a4d781aa51ef285367617eee
Date    : Thu Aug 18 08:13:21 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@412758  16.5334  0.26241   5  good
chromium@412762  16.7496  0.196363  5  good
chromium@412763  14.3396  0.395257  5  bad    <--
chromium@412764  14.3858  0.311974  5  bad
chromium@412765  14.3716  0.161947  5  bad
chromium@412771  13.9216  0.878545  5  bad
chromium@412784  14.4524  0.198817  5  bad

Bisect job ran on: mac_retina_perf_bisect
Bug ID: 639377

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.top_25_smooth
Test Metric: mean_input_event_latency/http___games.yahoo.com
Relative Change: 12.59%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_bisect/builds/1606
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003504579578461296


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5869073544511488

| 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 Tests>AutoBisect.  Thank you!
Project Member

Comment 12 by 42576172...@developer.gserviceaccount.com, Aug 24 2016


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Tweak priorities of paint invalidation reasons
Author  : wangxianzhu
Commit description:
  
1. Distinguish PaintInvalidationNone and PaintInvalidationProbablyNone,
   so that callers of ObjectPaintInvalidator::computePaintInvalidationReasion()
   can treat them differently to avoid unnecessary overriding
   cases that definitely no paint invalidation is needed.

2. Check emptiness of newBounds and oldBounds earlier. This avoids
   unnecessary object and layer paint invalidations when both oldBounds
   and newBounds are empty but we computed a non-none paint invalidation
   reason.

3. Renamed LayoutObject::paintedOutputOfObjectHasNoEffect to
   paintedOutputOfObjectHasNoEffectRegardlessOfSize because it doesn't check
   for empty size. The behavior is required to do correct paint invalidation
   when the size changes to empty. Now the name is better suited to the
   actual behavior.

These changes will help spv1 paint invalidation and GeometryMapper-based
spv2 paint invalidation to produce the same result.

BUG= 510908 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2247543003
Cr-Commit-Position: refs/heads/master@{#412763}
Commit  : bb6578fd92bd2477a4d781aa51ef285367617eee
Date    : Thu Aug 18 08:13:21 2016


===== TESTED REVISIONS =====
Revision         Mean    Std Dev   N  Good?
chromium@412758  6.7908  0.167715  5  good
chromium@412762  6.7474  0.142607  5  good
chromium@412763  7.7752  0.338859  5  bad    <--
chromium@412764  7.635   0.260388  5  bad
chromium@412766  7.67    0.213795  5  bad
chromium@412774  7.7932  0.239512  5  bad

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 639377

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.top_25_smooth
Test Metric: mean_input_event_latency/http___games.yahoo.com
Relative Change: 14.76%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/590
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003505186495845264


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5849574392987648

| 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 Tests>AutoBisect.  Thank you!
Status: WontFix (was: Started)
Tried to revert the change and run perf try jobs. The results seem very flaky:
https://codereview.chromium.org/2271783002/
https://codereview.chromium.org/2271773002/
https://codereview.chromium.org/2274663002/
https://codereview.chromium.org/2272723002/
https://codereview.chromium.org/2276573002/

Also the CL had not only regressions, but also progression at least for ChromiumPerf/chromium-rel-mac-retina/smoothness.top_25_smooth/ mean_input_event_latency/:
https://chromeperf.appspot.com/report?sid=ad90d136a6cef801132d257caa48748d767612d6ac8fd062e6a4039deb102ad8&start_rev=410609&end_rev=413760


Sign in to add a comment