New issue
Advanced search Search tips

Issue 902812 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

67%-76.1% regression in dromaeo at 605214:605558

Project Member Reported by lanwei@chromium.org, Nov 7

Issue description

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

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


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

Android Nexus5 Perf
Android Nexus6 WebView Perf
android-nexus5x-perf

dromaeo - Benchmark documentation link:
  None
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/117548dee40000

HTTP status code 401: Login Required

Cc: hharaken@chromium.org yukishiino@chromium.org
Owner: peria@chromium.org
Status: Assigned (was: Untriaged)
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/119e8699e40000

HTTP status code 401: Login Required

Labels: -Pri-2 OS-Android Pri-1
Cc: -hharaken@chromium.org haraken@chromium.org
This looks like a real regression. Kicked a bisect job again.


😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/16300c2ee40000

HTTP status code 401: Login Required

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/16b2a8cfe40000

HTTP status code 401: Login Required

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/13c8e18de40000

All of the runs failed. The most common error (20/20 runs) was:
BuildError: Build failed: BUILD_FAILURE
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14682253e40000

All of the runs failed. The most common error (20/20 runs) was:
BuildError: Build failed: BUILD_FAILURE
Cc: isherman@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/16b3a307e40000

Clean up the Java method name for recording a sparse histogram by isherman@chromium.org
https://chromium.googlesource.com/chromium/src/+/9dae03c6b8e1255ef565d738296e73cf4325a6ff
dom: 1.909e+05 → No values

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  None
Labels: OS-Linux OS-Mac
I couldn't find a buildable revision on Android between the range (due to #19), but this regression looks happening also on other platforms. I kicked a bisect on Linux. (#20)
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/1275ee07e40000

HTTP status code 401: Login Required

Cc: peria@chromium.org
Components: Speed>Benchmarks
Owner: eyaich@chromium.org
Looking at the pinpoint job, r605348 looks good, and r605349 looks bad.
I guess r605349 changes some measured numbers, and the actual performance did not change so much.
https://chromium.googlesource.com/chromium/src/+/a3dd9802add87e6a5c3385e55777ea50722c51f6

eyaich@, could you take a look?
Cc: eyaich@google.com
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1676bd27e40000

Implement press benchmark harness. by eyaich@google.com
https://chromium.googlesource.com/chromium/src/+/a3dd9802add87e6a5c3385e55777ea50722c51f6
dom: 1.629e+06 → 7.227e+04 (-1.556e+06)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  None
Cc: benjhayden@chromium.org nedngu...@google.com
Status: Started (was: Assigned)
Yes this is a bug in the new press benchmark harness when there are multiple stories.  Currently Dromaeo is the only benchmark using this harness that has more than one story.  

The bug is here: https://cs.chromium.org/chromium/src/tools/perf/measurements/dual_metric_measurement.py?q=dual_metric&sq=package:chromium&g=0&l=42

Basically we are allowing both TMBv2 metrics as well as legacy values in our press results.  Therefore, in order to get all the metric values to be correctly populated in the histograms set we need to add all legacy values BEFORE adding any TBMv2 metrics.  

As you can see when there are multiple pages this is a problem, since for every page we call this PopulatHistogramSet which will return if there are currently any histograms (https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/internal/results/page_test_results.py?q=PopulateHistogramSet+file:%5Esrc/third_party/catapult/telemetry/telemetry/+package:%5Echromium$&dr=CSs&l=271)

Therefore, this only populates the results with the first page that is run since very subsequent page returns before converting them to histograms.

I feel like the solution here should probably be in telemetry but I am not totally sure of the best course.  

Possible solutions:

1) store the results in the StoryTest press measurement and only add them to the actual results after all the stories have run.  Not sure how to know when all stories have run, might require a hook in telemetry.

2) Update the page_test_results to add a method to convert all present legacy values regardless of the state of the histograms array.  I am not sure if this would cause duplicate histograms by calling this multiple times on the present legacy values. 

Ben or Ned do you have any thoughts on how this should look in telemetry?

How about using the python histogram values ( to add dromaeo metrics instead of the legacy values?

Ben can help with using the new python histogram values
That solves this problem for dromaeo but what about any other tests that may have multiple stories?  I realize dromaeo is the only one at the moment, but this is a large bug for those benchmarks.
@28: converting to use python histogram values align with our direction to deprecate the legacy values system, so I think we can follow that strategy if we encounter problem like these for other press benchmark
Project Member

Comment 30 by bugdroid1@chromium.org, Nov 27

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

commit 1fa211c4aab61d4b9cfec802a4a4979b35508cf7
Author: Emily Hanley <eyaich@google.com>
Date: Tue Nov 27 16:46:26 2018

Convert dromaeo value system to histograms.

Dependent on crrev.com/c/1337454 landing.

Bug: 714231, 902812 
Change-Id: Idae1a45c02b2810a72c7725388014b1f1430b8bf
Reviewed-on: https://chromium-review.googlesource.com/c/1338171
Commit-Queue: Emily Hanley <eyaich@chromium.org>
Reviewed-by: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#611143}
[modify] https://crrev.com/1fa211c4aab61d4b9cfec802a4a4979b35508cf7/tools/perf/measurements/dual_metric_measurement.py
[modify] https://crrev.com/1fa211c4aab61d4b9cfec802a4a4979b35508cf7/tools/perf/page_sets/dromaeo_pages.py
[modify] https://crrev.com/1fa211c4aab61d4b9cfec802a4a4979b35508cf7/tools/perf/page_sets/press_story.py

Status: Verified (was: Started)
This last CL fixed this issue for dromaeo.  I have verified on the dashboard.

Sign in to add a comment