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

Issue 609118 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

84%-99.1% regression in smoothness.top_25_smooth at 391215:391246

Project Member Reported by mustaq@chromium.org, May 4 2016

Issue description

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

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


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

chromium-rel-mac-hdd
Cc: skyos...@chromium.org
Owner: skyos...@chromium.org

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

Hi skyostil@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 : scheduler: Only prioritize rAF if it is fast
Author  : skyostil
Commit description:
  
To avoid starving other tasks, only prioritize main thread compositing (i.e.,
requestAnimationFrame) if there is more than 80% idle time left in a frame. This can
reduce animation frame rate on sites which can't keep up with the display refresh
rate, but will ensure other tasks (e.g., postMessage) aren't starved as a
side-effect.

BUG= 600385 

Review-Url: https://codereview.chromium.org/1914373004
Cr-Commit-Position: refs/heads/master@{#391221}
Commit  : 6f3d26fe1385dc74f22bf2cf6bddc6830dd00d6c
Date    : Tue May 03 13:41:56 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@391214  28.9018  5.3555   5  good
chromium@391218  25.2015  1.99516  5  good
chromium@391220  27.1706  3.64949  5  good
chromium@391221  50.1909  2.0702   5  bad    <--
chromium@391222  49.1134  6.14449  5  bad
chromium@391230  51.2913  4.6802   5  bad
chromium@391246  49.1628  1.83725  5  bad

Bisect job ran on: mac_hdd_perf_bisect
Bug ID: 609118

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.top_25_smooth
Test Metric: frame_times/https___mail.google.com_mail_
Relative Change: 70.10%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_hdd_perf_bisect/builds/516
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9013585093445792448


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

| 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!
Just added this regression to the bug:
bot=chromium-rel-mac-hdd
TestSuite=smoothness.gpu_rasterization.top_25_smooth
Test=mean_input_event_latency

https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg1PKWuAoM


Note that mail.google.com regression is prominent.
Cc: alexclarke@chromium.org tdres...@chromium.org
This is an unfortunate side-effect of the linked patch: gmail's combined main thread compositing work (input handling + BeginMainFrame) takes about 20ms on this machine, so it's not safe to run that at a high priority without potentially starving other work. Since this is main thread scrolling, the scheduler doesn't really have a clear view into which work is essential for the use case and which isn't, so it will treat all work as equal priority.

One option would be for gmail to switch to using requestIdleCallback for the other work which is competing against input handling. Another option would be for the scheduler to start treating main thread mouse wheel scrolling as a special case and still apply prioritization to it.

+tdresser@ as FYI for the input latency regression.
If we wanted to identify the main thread scrolling case, how would we do it?

Some kind of heuristic where we look for event handlers which modify scroll offsets?

I doubt it's worth the investment, as we'd prefer folks to move away from main thread scrolling, but it might at least be worth discussing.
In this case I think we could just treat main thread wheel events as gestures where we think it's okay to starve things like timers or network traffic to improve smoothness for the gesture. Right now pretty much all main thread handled input causes us to switch to a super conservative mode where we treat all things equally.
Hmmm, that doesn't sound unreasonable.
We already treat touch events this way, don't we?
That's correct. We're gonna revamp some of our use case detection some time next week -- we'll keep this on the list as a potential improvement too.
SGTM!
Sami, should we WontFix this bug, and track the work described in #6 elsewhere, or should we leave this open to track it?
Status: Started (was: Assigned)
Let me look into what's going on and then update the bug.
Fix here: https://codereview.chromium.org/1977863004/

I'll land it against this bug since gmail is a good representation of the use case it is intending to address.
Project Member

Comment 13 by bugdroid1@chromium.org, May 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00b626512ee991e03b8482536583c5ddb073c7b8

commit 00b626512ee991e03b8482536583c5ddb073c7b8
Author: skyostil <skyostil@chromium.org>
Date: Mon May 16 12:53:51 2016

scheduler: Add main thread gesture as a detected use case

Distinguish between a main thread gesture (e.g., a scroll which is
handled on the main thread) and main thread custom input handling (e.g.,
a touchmove listener which prevents scrolling). In the former case we
know the gesture type (scrolling), so we can be a little more aggressive
about prioritizing input handling and compositing in order to improve
frame rate.

BUG= 609118 

Review-Url: https://codereview.chromium.org/1977863004
Cr-Commit-Position: refs/heads/master@{#393823}

[modify] https://crrev.com/00b626512ee991e03b8482536583c5ddb073c7b8/components/scheduler/renderer/renderer_scheduler.cc
[modify] https://crrev.com/00b626512ee991e03b8482536583c5ddb073c7b8/components/scheduler/renderer/renderer_scheduler.h
[modify] https://crrev.com/00b626512ee991e03b8482536583c5ddb073c7b8/components/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/00b626512ee991e03b8482536583c5ddb073c7b8/components/scheduler/renderer/renderer_scheduler_impl.h
[modify] https://crrev.com/00b626512ee991e03b8482536583c5ddb073c7b8/components/scheduler/renderer/renderer_scheduler_impl_unittest.cc

Status: Fixed (was: Started)
Looks like the regression is gone: https://chromeperf.appspot.com/group_report?bug_id=609118 (you might need to adjust the cl range.)

Sign in to add a comment