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

Issue 616943 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug-Regression



Sign in to add a comment

4.1%-6.3% regression in page_cycler.intl_ko_th_vi at 396961:397036

Project Member Reported by briander...@chromium.org, Jun 2 2016

Issue description

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

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


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

android-nexus6
android-nexus7v2
The functional range for this is actually super short:
https://chromium.googlesource.com/chromium/src/+log/51db06d62dc6a6a25f16be3ab52053f80abece9c..cb0f401524eb0009f583f8bb63959115013bf1a4?pretty=fuller

Trying a bisect there to see what we get.
Cc: csharrison@chromium.org
Owner: csharrison@chromium.org

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

Hi csharrison@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 : Refactor net predictor to use ResourceThrottles
Author  : csharrison
Commit description:
  
The predictor currently hooks into ChromeNetworkDelegate for callbacks.
This patch reduces the number of moving parts by attaching a throttle for
resources coming into the RDHD.

BUG= 601254 

Review-Url: https://codereview.chromium.org/1857383002
Cr-Commit-Position: refs/heads/master@{#396988}
Commit  : cb0f401524eb0009f583f8bb63959115013bf1a4
Date    : Wed Jun 01 00:14:32 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@396983  5411.23  13.3306  6  good
chromium@396986  5442.07  18.9908  5  good
chromium@396987  5433.5   12.2862  5  good
chromium@396988  5738.26  26.9342  5  bad    <--

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 616943

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests page_cycler.intl_ko_th_vi
Test Metric: warm_times/page_load_time
Relative Change: 6.07%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3043
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9008308467325267088


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

| 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!
Cc: rsch...@chromium.org mmenke@chromium.org
Looks like the only regression was in vnexpress.net.

Is there a way to get a about:tracing log from the telemetry dashboard before / after the revision lands? I may be able to do this ad-hoc with Dev and Beta to see if I can repro locally.

The one thing that might be happening is that some Chrome component might be in the critical path for page load time and isn't being predicted anymore. This change effectively moves the predictor to only predict resources coming from the renderer. Possibly if the predictor is actually useful in internal chrome land it could indicate a perf bug in that feature.
Cc: dtu@chromium.org aiolos@chromium.org eakuefner@chromium.org
Re #4: unfortunately it's not possible to get about:tracing from the dashboard automatically for these old page_cyclers; it works for the new page cycler v2. Note that the regression here is on the onload event, which is all the old page_cycler measures.

The best way to get a trace before/after is to locally revert the CL and run on perf trybots. Dave, Ethan or Kari, can you give instructions for how to do that and get a trace?
Project Member

Comment 6 by sheriffbot@chromium.org, Jul 7 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: kouhei@chromium.org
ping Dave/Ethan/Kari :) 

Also adding kouhei@, as I think he is porting many of these page cyclers to v2 which may make this easier to repro with traces locally.
Sorry for the delay; a few notes.

First, this benchmark has indeed been migrated to PCv2 as of a few weeks ago! https://codereview.chromium.org/2166723002

So you actually _can_ run locally to see traces; I'd recommend trying before and after the regression to compare. You can pass --output-format=json to run_benchmark to get vulcanized traces for each page.

Also, to use the trybots, you can try tools/perf/run_benchmark help try to see information about the try command; please note that due to a bug you must currently run run_benchmark from src/ for "run_benchmark try" to work properly. I currently have a bogus tryjob running here: https://codereview.chromium.org/2252473002 to check whether tryjobs upload traces for benchmarks that are timeline-based. They definitely should, and I'm hoping they do. I'll update this bug as that tryjob completes.
Friendly ping from perf sheriff.

csharrison@: Can the answer at #8 help you to investigate this?
I ran a perf bisect and could only really see a significant change for onload
Here is nexus 7:
http://storage.googleapis.com/chromium-telemetry/html-results/results-2016-09-14_21-35-20

Here is the CL:
https://codereview.chromium.org/2337233006/

Note "Patch" in this case is the revert. So the patch referenced in this CL may have caused a regression if "Patch" is better than "TOT". For nexus 7 this doesn't seem to be the case.

I need to get traces locally though 
I tried to use --output-format=json but that isn't giving me trace json, just results json.
csharrison: to clarify, --output-format=json gives you the results.json, but also a .html file for each story run. Those are vulcanized traces.
Cc: nednguyen@chromium.org
+nednguyen: TL;DR do we have a mechanism to support collecting traces for non-timeline-based benchmarks?

Whoops, I misunderstood what was happening here. Unfortunately page_cycler isn't trace-based, so Telemetry doesn't collect traces by default, so you won't see them with --output-format=json.

I think there used to be a flag you could pass to enable trace recording for non-trace-based benchmarks but I'm not sure we support that anymore.
Yeah, you can just press the "trace" button on the perf dashboard tooltip and it will run a perf tryjob before/after the revision in question with additional trace categories. I kicked off two:
https://codereview.chromium.org/2379713002
https://codereview.chromium.org/2377093002
Thank you, but how do I extract the traces from the perf bots?
Thanks that's so helpful!! After staring at the traces for some time I have narrowed it down to one change in the CL: we stopped learning from redirected subresources.

mmenke@, should we add this back in? I think we probably should.

Note: this regression is not really real, it just makes the subresource time out after 26 seconds rather than 30 seconds. For that reason the load even signal on this page is completely broken.

I'm fine with adding it back in, don't have strong opinions on it.
Labels: -Pri-2 Pri-3
SG. Moving this to P3 as a nice-to-have. 
Perf sheriff ping
csharrison@, would it make sense to file a new bug to track that, and close this bug?
Status: WontFix (was: Assigned)
Yes, I filed  issue 657666 . WontFixing this.

Note for perf sheriffs: the onload event for vnexpress.net is not a meaningful metric, as it is recorded when the connection times out after 30 seconds.

Sign in to add a comment