New issue
Advanced search Search tips

Issue 599502 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.1%-10.9% regression in page_cycler.intl_ko_th_vi at 383701:383770

Project Member Reported by majidvp@chromium.org, Mar 31 2016

Issue description

Regression is mainly on android (nexus5x, galaxy-s5).
 
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 : Change net's predictor_tab_helper to use the new content Navigation API
Author  : csharrison
Commit description:
  
This change moves the trigger for preconnects/preresolves from
DidStartNavigationToPendingEntry to DidStartNavigation, broadening the
navigations that we can predict from.

BUG= 595730 

Review URL: https://codereview.chromium.org/1813553004

Cr-Commit-Position: refs/heads/master@{#383714}
Commit  : 778dddc6edcfbc8e420e0ed019acf506b7bdd864
Date    : Tue Mar 29 13:52:42 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@383712         2119.012083 21.255647   6           good
chromium@383713         2140.437045 32.136861   5           good
chromium@383714         2355.361455 33.455434   5           bad         <-
chromium@383715         2379.785864 26.411818   5           bad
chromium@383717         2366.072773 46.765307   5           bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 599502

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

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/559
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9016660258237945328


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

| 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!

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


===== SUSPECTED CL(s) =====
Subject : Change net's predictor_tab_helper to use the new content Navigation API
Author  : csharrison
Commit description:
  
This change moves the trigger for preconnects/preresolves from
DidStartNavigationToPendingEntry to DidStartNavigation, broadening the
navigations that we can predict from.

BUG= 595730 

Review URL: https://codereview.chromium.org/1813553004

Cr-Commit-Position: refs/heads/master@{#383714}
Commit  : 778dddc6edcfbc8e420e0ed019acf506b7bdd864
Date    : Tue Mar 29 13:52:42 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@383704         1473.645833 36.852408   6           good
chromium@383710         1477.5205   27.611503   5           good
chromium@383713         1460.8415   12.731434   5           good
chromium@383714         1611.884    54.767805   5           bad         <-
chromium@383715         1623.182    19.189653   5           bad
chromium@383716         1617.01     33.671729   5           bad
chromium@383728         1621.5315   28.827978   5           bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 599502

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests page_cycler.top_10_mobile
Test Metric: warm_times/https___m.facebook.com_rihanna
Relative Change: 8.44%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/560
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9016660045119090784


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

| 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: mmenke@chromium.org
This is under an experimental flag so it should only be running on bots. That being said, I'm extremely curious why this regression is happening. majidvp@, is there any insight as to which experiment groups these regressions where found in? Is browsing history retained for bots? This feature relies on a history-based prediction scheme.

mmenke@, this seems like a pretty big regression. Is there something special about Android we should be aware of for preconnect?
I'm not all the familiar with Android.

I suspect what we're seeing is the bot is in the new field trial group with the new behavior (Not sure how field trials apply to the page cyclers), and for some reason, it's causing the bot to fail to preconnect, causing the regression.  I suggest reverting and trying to figure this out offline.

I don't suppose there are page cycler trybots?
Cc: sullivan@chromium.org clamy@chromium.org
There may be. sullivan@, any advice?

Actually, I think what's happening here is a renderer spawning issue. In the new API, we get DidStartNavigation upon receipt of DidStartProvisionalLoad IPC from the renderer.

The old system can be called before the renderer is spawned. Android process spawn can take ~200ms, causing a delay in sending out preconnects.

I'll revert and think about how to fix this. Possibly I'll issue preconnects as usual with the old system, augmented by the new system if we haven't preconnected to that host.
That sounds quite plausible.  You may want to talk to creis and clamy about better places to hook.
Any update on this bug? 
I reverted my change. Sorry I should have linked the revert:
https://codereview.chromium.org/1851093003

I'm working on an improved version here:
https://codereview.chromium.org/1857573002

This can be marked as fixed if the perf bots went down.
Status: Fixed (was: Assigned)
Looking at the graph, it did indeed go back down to its original level.

Sign in to add a comment