Issue metadata
Sign in to add a comment
|
43.9%-682.9% regression in smoothness.sync_scroll.key_mobile_sites_smooth at 405971:405981 |
||||||||||||||||||||||
Issue descriptionForking 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.
,
Sep 29 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9000147126627781936
,
Sep 30 2016
=== 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!
,
Sep 30 2016
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?)
,
Sep 30 2016
,
Sep 30 2016
,
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?
,
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?
,
Sep 30 2016
,
Sep 30 2016
Agreed that "sync_scroll" is not one of the metrics we should be concerned with for system health.
,
Oct 3 2016
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.
,
Oct 3 2016
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ericrk@chromium.org
, Sep 29 2016