Issue metadata
Sign in to add a comment
|
67.5% regression in smoothness.scrolling_tough_ad_cases at 417718:417797 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Sep 12 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9001712911492358272
,
Sep 12 2016
=== Auto-CCing suspected CL author sunnyps@chromium.org === Hi sunnyps@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 : cc: Remove frame queuing from the scheduler. Author : sunnyps Commit description: CC scheduler has a frame queuing mechanism called "retro frames". This has been responsible for a lot of complexity and hard to fix bugs. The original intent for adding retro frames was to allow the scheduler to handle multiple frames in flight but that goal doesn't seem feasible or even desirable any more. This CL removes the retro frames queue and instead makes the Scheduler end the previous frame when it receives a BeginFrame message. One surprising behavior was that SyntheticBFS MISSED frames would be queued as retro frames and this would convert the synchronous begin frame call (inside Scheduler::ProcessScheduledActions) to an asynchronous retro frame PostTask. To work around this the Scheduler keeps track of a single CancelableClosure that's used for MISSED frames. The above behavior was also causing the BeginFramesNotFromClient tests to work even though there was an extra MISSED frame in the queue. This is more elegantly solved in another way by using frame number to advance the task runner instead of just running pending tasks. Lastly SchedulerStateMachine is modified so that it's possible to end the previous frame and still have the same behavior as before in the commit to active tree (browser compositor) mode. R=brianderson@chromium.org,enne@chromium.org,danakj@chromium.org BUG= 602485 , 644992 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2323063004 Cr-Commit-Position: refs/heads/master@{#417764} Commit : 559280b26cc5672f5f054e8ac35281e804c14d78 Date : Fri Sep 09 23:38:51 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@417717 8.76716 1.19988 8 good chromium@417758 8.94085 1.2135 8 good chromium@417761 8.5356 0.690985 5 good chromium@417763 7.82512 1.01593 5 good chromium@417764 15.4287 1.67784 5 bad <-- chromium@417768 15.7923 2.82044 8 bad chromium@417777 15.9312 2.42161 5 bad chromium@417797 14.7731 3.52557 8 bad Bisect job ran on: android_nexus6_perf_bisect Bug ID: 645982 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.scrolling_tough_ad_cases Test Metric: first_gesture_scroll_update_latency/http___www.cnn.com_2015_01_09_politics_nebraska-keystone-pipeline_index.html Relative Change: 54.42% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2554 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9001712911492358272 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5784027141767168 | 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 13 2016
sunnyps, here's an example of the scheduler change actually regressing something (not making it better, like the other bugs I cced you on).
,
Sep 13 2016
Yup, the CL has been reverted for another reason but I'll look into this too before landing again.
,
Sep 23 2016
Remove -performance-sherrif, but please watch out for potential regression when you reland your patch
,
Nov 18 2016
,
May 6 2017
This regression has been included in a stable release and that stable channel is now deprecated. I'm closing this so that we won't have unresponsive performance regressions in the future.
,
May 6 2017
Note that this regression didn't actually ship to a release. The CL was reverted and then relanded (twice) and the regression didn't show up after that AFAICT. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by rsch...@chromium.org
, Sep 12 2016