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

Issue 756990 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

24.4%-204.7% regression in speedometer at 494623:495119

Project Member Reported by jarin@google.com, Aug 18 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Aug 18 2017

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

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


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

android-nexus5
android-nexus5X
android-nexus6
android-nexus7v2
android-one
android-webview-nexus6
chromium-rel-mac-retina
chromium-rel-mac11
chromium-rel-mac11-air
chromium-rel-mac11-pro
chromium-rel-mac12
chromium-rel-mac12-mini-8gb
chromium-rel-win10
chromium-rel-win7-dual
chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia
chromium-rel-win7-x64-dual
chromium-rel-win8-dual
win-high-dpi
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Aug 18 2017

Cc: ashleymarie@chromium.org
Owner: ashleymarie@chromium.org

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

Hi ashleymarie@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 : Ashley Enstad
  Commit : 6437bba4ee92ecb4e526c3882eae6338d9d89ed6
  Date   : Wed Aug 16 16:32:45 2017
  Subject: Using speedometer in third_party/webkit/PerformanceTest

Bisect Details
  Configuration: winx64_10_perf_bisect
  Benchmark    : speedometer
  Metric       : EmberJS-TodoMVC/EmberJS-TodoMVC
  Change       : 148.27% | 665.211416667 -> 1651.51675

Revision             Result                  N
chromium@494747      665.211 +- 18.6923      6      good
chromium@494782      663.761 +- 11.032       6      good
chromium@494799      663.846 +- 24.2524      6      good
chromium@494808      658.127 +- 23.965       6      good
chromium@494812      663.12 +- 8.1364        6      good
chromium@494814      660.955 +- 7.49397      6      good
chromium@494815      655.778 +- 11.742       6      good
chromium@494816      1651.52 +- 47.5258      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 speedometer

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

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


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

Comment 5 by 42576172...@developer.gserviceaccount.com, Aug 18 2017


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

Suspected Commit
  Author : Ashley Enstad
  Commit : 6437bba4ee92ecb4e526c3882eae6338d9d89ed6
  Date   : Wed Aug 16 16:32:45 2017
  Subject: Using speedometer in third_party/webkit/PerformanceTest

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : speedometer
  Metric       : EmberJS-TodoMVC/EmberJS-TodoMVC
  Change       : 196.24% | 6812.35277778 -> 20181.145

Revision             Result                  N
chromium@494749      6812.35 +- 333.555      6      good
chromium@494795      6862.49 +- 383.922      6      good
chromium@494807      6765.46 +- 180.893      6      good
chromium@494813      6753.13 +- 249.991      6      good
chromium@494815      6759.67 +- 125.516      6      good
chromium@494816      20142.8 +- 437.687      6      bad       <--
chromium@494818      20254.3 +- 304.832      6      bad
chromium@494841      20181.1 +- 431.672      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 speedometer

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

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


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

Comment 6 by 42576172...@developer.gserviceaccount.com, Aug 29 2017

Cc: kraynov@chromium.org
 Issue 759736  has been merged into this issue.
What's the status of this? It looks like a massive regression in Speedometer, although the CL seems to be a re-recording of the speedometer page. Was this expected?
Cc: rmcilroy@chromium.org
Cc: nednguyen@chromium.org
The CL was changing from using the recorded version of speedometer to the actual page in third_party/
Ned, do you know if this is expected? Does this mean our recording was out-dated or wrong? Or the third_party version is?
Indeed, the reference numbers also regress, so I believe this is expected.
I think maybe the copy of speedometer in https://github.com/WebKit/webkit/tree/master/Websites/browserbench.org/Speedometer2.0 is more up to date to the WPR recording.

Anyway, since this is 1) a test change and 2) the version we change to is guaranteed to be the most up-to-date, I think we can mark this as Wont Fix (expected).
Status: WontFix (was: Assigned)
sgtm, marking wont fix
Cc: leszeks@chromium.org
Status: Assigned (was: WontFix)
Leszek just pointed out that the speedometer in third_party/webkit/PerformanceTest is actually Speedometer2.0. This benchmark is meant to be testing the Speedometer1.0 benchmark, so this CL actually changed the benchmark (we test Speedometer2.0 in benchmarks/speedometer2.py).  This would explain the apparent regression.

Can you revert this change please. If you want to pull from the webkit repo the correct place for Speedometer1.0 is https://github.com/WebKit/webkit/tree/master/Websites/browserbench.org/Speedometer

Oops, I thought third_party/webkit/PerformanceTes have both copy of speedometer 1.0 & 2.0. Looks like third_party/WebKit/PerformanceTests/Speedometer/index.html is just the improved UI version of speedometer2? 
Reverting in https://chromium-review.googlesource.com/c/chromium/src/+/650547 & we can investigate later whether third_party/WebKit/PerformanceTests/Speedometer/index.html is Speedometer2 or 1
So when I run speedometer locally before the revert, the tab title says Speedometer 1.0
How can we tell if it's actually 1.0 or 2.0? The tab title might just be a red herring (a bug on Webkit's side)
Project Member

Comment 19 by bugdroid1@chromium.org, Sep 5 2017

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

commit 667e31b9c1e0ecc0ef85aa6c4fe6cce89ff56d42
Author: Ned Nguyen <nednguyen@google.com>
Date: Tue Sep 05 19:54:18 2017

Revert "Using speedometer in third_party/webkit/PerformanceTest"

This reverts commit 6437bba4ee92ecb4e526c3882eae6338d9d89ed6.

Reason for revert: suspect this is actually running speedometer2 benchmark.

BUG:756990

Original change's description:
> Using speedometer in third_party/webkit/PerformanceTest
> 
> This change switches speedometer benchmark to use the copy of
> Speedometer1.0 in third_party/webkit/PerformanceTest instead of WPR
> 
> BUG= chromium:732494 
> 
> Change-Id: Idb299ae5f23178d4c1fa8a4569d40d7d88632e04
> Reviewed-on: https://chromium-review.googlesource.com/616802
> Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
> Reviewed-by: Ned Nguyen <nednguyen@google.com>
> Cr-Commit-Position: refs/heads/master@{#494816}

TBR=nednguyen@google.com,ashleymarie@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:732494 
Change-Id: I7a991f6d99f857b15f94466d2655280902b346b2
Reviewed-on: https://chromium-review.googlesource.com/650547
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#499718}
[modify] https://crrev.com/667e31b9c1e0ecc0ef85aa6c4fe6cce89ff56d42/tools/perf/benchmarks/speedometer.py
[add] https://crrev.com/667e31b9c1e0ecc0ef85aa6c4fe6cce89ff56d42/tools/perf/page_sets/data/speedometer.json
[add] https://crrev.com/667e31b9c1e0ecc0ef85aa6c4fe6cce89ff56d42/tools/perf/page_sets/data/speedometer_000.wprgo.sha1

Owner: addyo@chromium.org
addyo@: can you help answer whether https://github.com/WebKit/webkit/blob/master/PerformanceTests/Speedometer/index.html is Speedometer1.0 or 2.0?

If it's 2.0, then why does it have "<title>Speedometer 1.0</title>"?
I'm pretty sure it's Speedometer 2.0, e.g, see the last commit on that directory "Compute the final score using geometric mean in Speedometer 2.0" -
https://github.com/WebKit/webkit/commit/974557eb4a1a2d208e68a56f583f5d0d27f00b86

and the fact that the resources in PerformanceTests/Speedometer/resources/todomvc/architecture-examples/ include frameworks which weren't tested in Speedometer 1.0 (e.g., Inferno).

Not sure why the title says 1.0 though - maybe we could change this addyo@?
#21: my original thinking was that directory contain *both* speedometer1.0 & 2.0 (I originally added it for Speedometer2). 

Waiting for addyo@ for the final answer. :-)

Comment 23 by addyo@google.com, Sep 6 2017

The current content of /Speedometer is 2.0. We'll get that title updated. 

The recommendation we'll be making to vendors is doing a wholesale switch to 2.0 once the last remaining blocker is complete.

With respect to the regression you're noticing, I'm pretty sure that's related to a combination of the geometric mean switches and some of the more recent framework loads we weren't benchmarking against before.

To set expectations: we're hoping to address the final blocker on 2.0 this week (finally :)) and should be in a more stable place with the benchmark shortly.
Thanks Addy

> With respect to the regression you're noticing, I'm pretty sure that's related to a combination of the geometric mean switches and some of the more recent framework loads we weren't benchmarking against before.

The telemetry benchmark measures the ms time of each framework independently, so this wasn't related to the switch to geometric mean or the additional frameworks (other than the regression in Total).

I'm guessing the frameworks were also updated in 2.0? If so, this suggests that either we are slower for newer versions of Ember / JQuery or those frameworks are now doing more work than they used to in 1.0. Any ideas?  

Sign in to add a comment