New issue
Advanced search Search tips

Issue 772447 link

Starred by 5 users

Issue metadata

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



Sign in to add a comment

12%-260.4% regression in thread_times.tough_scrolling_cases at 506784:506854

Project Member Reported by fmea...@chromium.org, Oct 6 2017

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=772447

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=e3dbf40eed76a5356145a87b7d614323628f2120b1f44969f9704367b958829e


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

android-webview-nexus6
Cc: bokan@chromium.org
Owner: bokan@chromium.org
Status: Assigned (was: Untriaged)

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

Hi bokan@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 : David Bokan
  Commit : 771bc4ee1d609083dd81daa0ee78229b49dfa270
  Date   : Thu Oct 05 18:39:46 2017
  Subject: Use viewport coords in gpuBenchmarking gestures

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_raster_cpu_time_per_frame/text_constant_full_page_raster_20000_pixels_per_second
  Change       : 251.21% | 0.939948200927 -> 3.30114815783

Revision             Result                    N
chromium@506783      0.939948 +- 0.337521      6      good
chromium@506801      0.906562 +- 0.161839      6      good
chromium@506803      0.922872 +- 0.335862      6      good
chromium@506804      3.36261 +- 0.216506       6      bad       <--
chromium@506806      3.38203 +- 0.352175       6      bad
chromium@506810      3.38648 +- 0.302303       6      bad
chromium@506819      3.33047 +- 0.213573       6      bad
chromium@506854      3.30115 +- 0.215692       6      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=text.constant.full.page.raster.20000.pixels.per.second thread_times.tough_scrolling_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8966464622287335088


For feedback, file a bug with component Speed>Bisection

Comment 4 by bokan@chromium.org, Oct 6 2017

It's definitely not a real regression as this CL only changed some simulation methods but it's still surprising. Will take a look.
 Issue 772469  has been merged into this issue.
 Issue 772472  has been merged into this issue.
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Oct 10 2017

 Issue 773033  has been merged into this issue.
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Oct 10 2017

 Issue 773046  has been merged into this issue.
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Oct 11 2017

Cc: nzolghadr@chromium.org
 Issue 773711  has been merged into this issue.
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Oct 11 2017

 Issue 773710  has been merged into this issue.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/b0d47f5f4d4b9717bfdd43ae6d40257235d0848f

commit b0d47f5f4d4b9717bfdd43ae6d40257235d0848f
Author: David Bokan <bokan@chromium.org>
Date: Fri Oct 13 16:49:07 2017

Fix coordinate space issues in scroll.js

In Chromium r506804, I changed all gpuBenchmarking methods to expect
input arguments to be in screen/viewport coordinates. That is, they
should track the "finger on the glass" rather than Elements on the
page. Thus, they're invariant under pinch-zoom.

However, I missed a few callsites in Catapult, scroll.js being one of
them. Since gpuBenchmarking no longer applies the pageScaleFactor to the
scroll distance, scroll.js must now do it itself.

I've also cleaned how scroll distances are calculated in light of the
"inert-visual-viewport" changes that shipped in M61. Namely, that
the scrollTop value from body/window doesn't reflect the visualViepwort
any more.

Bug:  chromium:772447 
Change-Id: Ia4a55e1b5aa16a9c0c624d78f892e3299398b447
Reviewed-on: https://chromium-review.googlesource.com/711159
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: David Bokan <bokan@chromium.org>

[modify] https://crrev.com/b0d47f5f4d4b9717bfdd43ae6d40257235d0848f/telemetry/telemetry/internal/actions/scroll.js

Comment 12 by bokan@chromium.org, Oct 17 2017

The above CL fixes most of the changes in the original report. A handful seem to be unchanged so they warrant some further investigation:

ChromiumPerf/chromium-rel-win7-dual/thread_times.tough_scrolling_cases / thread_raster_cpu_time_per_frame / text_hover_05000_pixels_per_second
ChromiumPerf/chromium-rel-win7-dual/thread_times.tough_scrolling_cases / thread_raster_cpu_time_per_frame / text_hover_10000_pixels_per_second
ChromiumPerf/chromium-rel-win7-dual/thread_times.tough_scrolling_cases / thread_raster_cpu_time_per_frame / text_hover_15000_pixels_per_second
ChromiumPerf/chromium-rel-win7-dual/thread_times.tough_scrolling_cases / thread_raster_cpu_time_per_frame / text_hover_50000_pixels_per_second

ChromiumPerf/chromium-rel-win7-x64-dual/thread_times.tough_scrolling_cases / thread_raster_cpu_time_per_frame / text_hover_20000_pixels_per_second

ChromiumPerf/android-nexus5/thread_times.key_mobile_sites_smooth / thread_IO_cpu_time_per_frame / http___cuteoverload.com


Project Member

Comment 15 by 42576172...@developer.gserviceaccount.com, Oct 17 2017


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

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_raster_cpu_time_per_frame/text_hover_50000_pixels_per_second

Revision             Result                  N
chromium@506716      7.44167 +- 3.23776      21      good
chromium@506990      7.05447 +- 2.72481      21      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 --story-filter=text.hover.50000.pixels.per.second thread_times.tough_scrolling_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8965531866456174416


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, Oct 17 2017


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

Suspected Commit
  Author : David Bokan
  Commit : 771bc4ee1d609083dd81daa0ee78229b49dfa270
  Date   : Thu Oct 05 18:39:46 2017
  Subject: Use viewport coords in gpuBenchmarking gestures

Bisect Details
  Configuration: android_nexus5_perf_bisect
  Benchmark    : thread_times.key_mobile_sites_smooth
  Metric       : thread_IO_cpu_time_per_frame/http___cuteoverload.com
  Change       : 5.12% | 3.06016511558 -> 3.21689371855

Revision             Result                    N
chromium@506756      3.06017 +- 0.0919344      6      good
chromium@506796      3.08767 +- 0.124685       6      good
chromium@506801      3.11166 +- 0.126923       6      good
chromium@506803      3.1 +- 0.0634734          6      good
chromium@506804      3.22195 +- 0.0713194      9      bad       <--
chromium@506806      3.2203 +- 0.0375002       6      bad
chromium@506816      3.23457 +- 0.0472179      6      bad
chromium@506835      3.21689 +- 0.0857355      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...cuteoverload.com thread_times.key_mobile_sites_smooth

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8965531971018189872


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 17 by 42576172...@developer.gserviceaccount.com, Oct 17 2017

Issue 774105 has been merged into this issue.

Comment 20 by bokan@chromium.org, Oct 20 2017

Kicked off wider bisects for the thread_raster_cpu_time_per_frame. Those are unlikely to be related since they're desktop - there should be no zoom involved...

Looked into the thread_times issue, interestingly, it affects only Nexus 5X - Nexus 6 is unaffected. Will try to repro on a real device.
Project Member

Comment 21 by 42576172...@developer.gserviceaccount.com, Oct 21 2017


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

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_raster_cpu_time_per_frame/text_hover_20000_pixels_per_second

Revision             Result                   N
chromium@506801      1.90001 +- 0.789815      21      good
chromium@506896      1.8644 +- 0.882397       21      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 --story-filter=text.hover.20000.pixels.per.second thread_times.tough_scrolling_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8965178378175548512


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 22 by 42576172...@developer.gserviceaccount.com, Oct 21 2017


=== BISECT JOB RESULTS ===
Perf regression found but unable to narrow commit range

Build failures prevented the bisect from narrowing the range further.


Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_raster_cpu_time_per_frame/text_hover_50000_pixels_per_second
  Change       : 5.90% | 8.67630415802 -> 8.16457328435

Suspected Commit Range
  2 commits in range
  https://chromium.googlesource.com/chromium/src/+log/be4381d4036bfc0eee0cf74561711b98e08d8ba2..12234368f826f07d8a0cef2dd0f9b45f290e1464


Revision             Result                   N
chromium@506716      8.6763 +- 1.59258        14       good
chromium@506785      8.42198 +- 1.4031        14       good
chromium@506820      8.33737 +- 1.44791       14       good
chromium@506827      8.15392 +- 1.52813       21       good
chromium@506829      8.41134 +- 0.599027      6        good
chromium@506830      ---                      ---      build failure
chromium@506831      7.37399 +- 0.712605      6        bad
chromium@506833      7.91581 +- 2.13848       14       bad
chromium@506838      7.64987 +- 1.84151       14       bad
chromium@506853      7.90687 +- 1.2208        14       bad
chromium@506990      8.16457 +- 1.43815       14       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 --story-filter=text.hover.50000.pixels.per.second thread_times.tough_scrolling_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8965178440526191920


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 24 by 42576172...@developer.gserviceaccount.com, Oct 23 2017


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

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_raster_cpu_time_per_frame/text_hover_50000_pixels_per_second

Revision             Result                  N
chromium@506365      7.93602 +- 2.40847      21      good
chromium@506990      7.68554 +- 1.43743      21      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 --story-filter=text.hover.50000.pixels.per.second thread_times.tough_scrolling_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8964942332746547936


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 27 by 42576172...@developer.gserviceaccount.com, Oct 23 2017


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

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_raster_cpu_time_per_frame/text_hover_15000_pixels_per_second

Revision             Result                   N
chromium@506801      1.13748 +- 0.429995      21      good
chromium@506896      1.2252 +- 0.479583       21      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 --story-filter=text.hover.15000.pixels.per.second thread_times.tough_scrolling_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8964912507621475328


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 28 by 42576172...@developer.gserviceaccount.com, Oct 23 2017


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

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_raster_cpu_time_per_frame/text_hover_05000_pixels_per_second

Revision             Result                    N
chromium@506801      0.32881 +- 0.129895       21      good
chromium@506896      0.325955 +- 0.110977      21      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 --story-filter=text.hover.05000.pixels.per.second thread_times.tough_scrolling_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8964912486689508864


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 29 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/dbab215523c8d77a7b54fcf3776357b079c36899

commit dbab215523c8d77a7b54fcf3776357b079c36899
Author: David Bokan <bokan@chromium.org>
Date: Tue Oct 24 14:24:10 2017

Fix telemetry gesture speeds

In b0d47f5f4d4b9717bfdd43ae6d40257235d0848f I made it so telemetry JS
scripts pass values to gpuBenchmarking in the coordinate space of the
(visual) viewport. This missed the speed value which is assumed to be
in CSS pixels/sec. Not performing this adjustment meant that the max
scrollable distance calculation based on speed and maximum time was
wrong.

Bug:  chromium:772447 
Change-Id: Ifdf56781f90b8efabfec5f9e72dea8b125db7bfd
Reviewed-on: https://chromium-review.googlesource.com/733980
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/dbab215523c8d77a7b54fcf3776357b079c36899/telemetry/telemetry/internal/actions/scroll.js
[modify] https://crrev.com/dbab215523c8d77a7b54fcf3776357b079c36899/telemetry/telemetry/internal/actions/drag.js
[modify] https://crrev.com/dbab215523c8d77a7b54fcf3776357b079c36899/telemetry/telemetry/internal/actions/pinch.js
[modify] https://crrev.com/dbab215523c8d77a7b54fcf3776357b079c36899/telemetry/telemetry/internal/actions/swipe.js

Comment 30 by bokan@chromium.org, Oct 27 2017

Status: Fixed (was: Assigned)
I've fixed an issue I found in telemetry and split off all the other dup'd regressions that were unrelated so I'm marking this as fixed.

Sign in to add a comment