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

Issue 651607 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

43.9%-682.9% regression in smoothness.sync_scroll.key_mobile_sites_smooth at 405971:405981

Project Member Reported by ericrk@chromium.org, Sep 29 2016

Issue description

Forking this bug from  crbug.com/624630 , which was overloaded with a number of different issues within GPU raster.

This issue contains only the input event latency regressions. I believe these are all due to shader compilation, which should be minimized in the wild due to shader caching.

I will confirm that this is the case and close out this bug.
 
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Sep 30 2016

Cc: picksi@chromium.org
Owner: picksi@chromium.org

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

Hi picksi@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 : Reland of -enable GPU Rasterization for content with any author defined viewport. (patchset #1 id:1 of https://codereview.chromium.org/2156553002/ )
Author  : picksi
Commit description:
  
Reason for revert:
Reverting change now that we have collected data from telemetry.

Original issue's description:
> Revert of Re-enable GPU Rasterization for content with any author defined viewport. (patchset #2 id:20001 of https://codereview.chromium.org/2097413003/ )
>
> Reason for revert:
> This has caused an unexpectedly large regression in overall PSS (about 5MB, graph here: https://chromeperf.appspot.com/report?sid=0b072725c25637efb0d3a44383da1e24e42bbb3740a00505e4040af2173423b0&start_rev=402061&end_rev=402790).
> The owners are both OOO and this is blocking Android release. This has already been reverted in the release branch but our infrastructure cannot gather data from the branch to confirm that the revert has had the intended result.
>
> This revert will allow us to confirm that this CL was the cause of the regression. Once confirmed (or otherwise) via telemetry dashboards this revert will be re-reverted.
>
> Original issue's description:
> > Re-enable GPU Rasterization for content with any author defined viewport.
> >
> > BUG= 591179 
> >
> > Committed: https://crrev.com/a017b667a53b3ee5f8bc630be98c37ebf53a2339
> > Cr-Commit-Position: refs/heads/master@{#402702}
>
> TBR=aelias@chromium.org,chrishtr@chromium.org,vmiura@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 591179 
>
> Committed: https://crrev.com/10a77702aa0490dfaaa6e354151b24b738474fb4
> Cr-Commit-Position: refs/heads/master@{#405750}

TBR=aelias@chromium.org,chrishtr@chromium.org,vmiura@chromium.org,primiano@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 591179 

Review-Url: https://codereview.chromium.org/2154193002
Cr-Commit-Position: refs/heads/master@{#405974}
Commit  : 5f19720ded2369857bee408ab48e11e7b5d28f7b
Date    : Mon Jul 18 11:06:45 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@405972  31.491   4.85464  5  good
chromium@405973  32.3174  6.2815   5  good
chromium@405974  298.798  24.4719  5  bad    <--
chromium@405975  282.525  13.9198  5  bad
chromium@405978  292.658  12.5822  5  bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 651607

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.sync_scroll.key_mobile_sites_smooth
Test Metric: first_gesture_scroll_update_latency/http___nytimes.com_
Relative Change: 829.34%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/2138
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9000147126627781936


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

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

Comment 4 by picksi@chromium.org, Sep 30 2016

Cc: perezju@chromium.org aelias@chromium.org chrishtr@chromium.org vmi...@chromium.org
I can see big regressions and improvements over the preceding couple of months in first gesture scroll update latency. Does this match the land/revert pattern of this CL ... I know we had a hard time getting it past Android System Health.

Is this a genuine regression? It seems to have changed latency from 50ms up to 300ms, which is scary.

Also, from a meta-level, this regression was previously masked by the memory regression that we all discussed at length.  crbug.com/624630  which is linked to from a previous regression on this graph, only talks about memory, looks like we all missed the input latency regression completely (unless it was only me that missed it?)

Comment 5 by picksi@chromium.org, Sep 30 2016

Cc: primiano@chromium.org

Comment 6 by ericrk@chromium.org, Sep 30 2016

Owner: ericrk@chromium.org

Comment 7 by ericrk@chromium.org, Sep 30 2016

So, this is definitely due to GPU raster, however, per my comment in #1, I'm not sure how big of an issue this is.

A few things to note:
- This regression only occurs on the smoothness.sync_scroll.key_mobile_sites_smooth benchmark, not the smoothness.key_mobile_sites_smooth benchmark. From my understanding, forcing sync_scroll is interesting from an optimization standpoint, but does not represent a real user scenario for these sites.
- This regression is only on 2 sites out of ~25 in the page set, other sites are unaffected, and the averages for these values across all sites are ~unchanged.
- From looking at traces, this regression occurs due to shader compilation during the first scroll of the page, as new shaders are added to the cache. In the real world, android devices will have (at least) an in-memory shader cache, and possibly an on-disk shader cache, which should make this an unlikely scenario. In fact, running the test with a repeat eliminates the regression.

For these reasons, drop this issue's priority and investigate as a non-blocking nice-to-improve. WDYT?

Comment 8 by picksi@chromium.org, Sep 30 2016

This explanation makes a lot of sense. If it disappears with a repeat then I don't think we need to worry about its impact.

I wonder if we should add another metric here, along the line of cold start Vs warm start, to capture this sort of one-off cost, i.e. something like 'first_gesture_scroll_update_latency_warm' that ignores the first iteration or a page... not even sure if this is possible on current infrastructure?

Comment 9 by ericrk@chromium.org, Sep 30 2016

Components: Internals>GPU>Rasterization
Agreed that "sync_scroll" is not one of the metrics we should be concerned with for system health.
Status: WontFix (was: Assigned)
Ok, re-confirmed that subsequent runs (without a chrome restart) result in much lower numbers - it's just the first run that has issues. Per comments in #7,8,10, closing this out.
Cc: ericrk@chromium.org
 Issue 651605  has been merged into this issue.

Sign in to add a comment