New issue
Advanced search Search tips

Issue 645982 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

67.5% regression in smoothness.scrolling_tough_ad_cases at 417718:417797

Project Member Reported by rsch...@chromium.org, Sep 12 2016

Issue description

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

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


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

android-nexus6
Project Member

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

Cc: sunn...@chromium.org
Owner: sunn...@chromium.org

=== 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!
sunnyps, here's an example of the scheduler change actually regressing something (not making it better, like the other bugs I cced you on).
Yup, the CL has been reverted for another reason but I'll look into this too before landing again.
Labels: -Performance-Sheriff
Remove -performance-sherrif, but please watch out for potential regression when you reland your patch

Comment 7 by rbyers@chromium.org, Nov 18 2016

Labels: Performance
Status: WontFix (was: Assigned)
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. 
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