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

Issue 654765 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Bad performance regressions during input. Event queue getting swamped by input?

Reported by gmur...@gmail.com, Oct 11 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36

Steps to reproduce the problem:
1. Navigate here: http://www.igniteui.com/data-chart/binding-high-volume-data
2. Zoom into the chart by rolling the mouse wheel. You may experience some odd frame drops.
3. Pan the chart by holding shift and mouse dragging. You may see extremely low framerates.
4. If you have a touch screen, touch the chart.
5. Try panning again, noticing how everything is smooth.
6. If you don't have a touch screen, continue to interact, maybe wait and come back, eventually things will suddently be smooth.

What is the expected behavior?
Interacting with the component should have smooth framerates as in other browsers.

What went wrong?
The component is deferring work, at points using window.setTimeout(callback, 0) and this seems not fire often during the problem periods, presumably because something about the input is swamping the event queue. This leads to unacceptably low framerates in the component.

Did this work before? Yes Uncertain.

Does this work in other browsers? Yes

Chrome version: 53.0.2785.143  Channel: stable
OS Version: 10.0
Flash Version: Shockwave Flash 23.0 r0

I can only theorize that higher priority stuff is allowed to jump to the head of the queue, or other traffic is so thoroughly swamping the queue that user events on the queue are spaced far apart. This didn't behave like this in the past. Using requestAnimationFrame does work around the issue, but there are reasons why this component wishes to leave its update frequency uncapped, for the moment, and this seems indicative of a larger problem if only chromium has this issue with setTimeout.
 

Comment 1 by gmur...@gmail.com, Oct 11 2016

I've noticed not everyone seems to be able to reproduce the issue, but I can on two windows machines and one OSX one.

Comment 2 by gmur...@gmail.com, Oct 11 2016

Its seeming like leaving the page open and foregrounded for a while can also lead to the issue not appearing.
Cc: dtapu...@chromium.org skyos...@chromium.org
Components: Blink>Scheduling
Can you try on Chrome Canary and report whether it is fixed? Some scheduling changes have landed since the M53 stable build. We believe it is better in M54.

Comment 4 by gmur...@gmail.com, Oct 11 2016

I can still repro it in Canary. Just like stable, sometimes the issue goes away after the tab has been foregrounded for a bit. But I can reliably repro by opening up a fresh canary and following the steps above. 

Problem much easier to see with a touchpad that does two finger scroll than a mouse wheel.

I'm having trouble reproducing the issue in a more isolated scenario, so there could be some bad interaction with the surrounding content. I'm reasonably sure I've seen the degradation occur in a simpler environment though.

I'm reasonably confident, from a lot of tracing, that it boils down to window.setTimeout's callback not firing very often at all, during some periods, which is what leads me to believe the queue is getting swamped, or not serviced frequently enough?

Comment 6 by gmur...@gmail.com, Oct 11 2016

Here's another data point. If you open this sample:

http://www.igniteui.com/data-chart/binding-real-time-data

and click to start the feed. You'll notice everything is fine and proceeding with nice framerates, even though this is also using window.setTimeout to defer rendering of each frame.

However, if you start mouse wheeling over the component, you may notice a frame drop. This is a bit difficult to work with as the issue keeps fixing itself randomly as the page stays open, or more events are fired, potentially. I initially thought it was a JIT optimization thing going on, whereby once enough statistics were collected something wound up getting optimized, but in light of setTimeout's callback firing so infrequently I think something else must be going on.

Comment 7 by gmur...@gmail.com, Oct 11 2016

Which profile should I select when recording the trace, and do you want it just for canary? Or for canary and stable?

Comment 8 by gmur...@gmail.com, Oct 11 2016

trace_swamping_bug.json.gz
2.3 MB Download

Comment 9 by gmur...@gmail.com, Oct 11 2016

That's stable with the "JavaScript and Rendering" profile selected.

Comment 10 by gmur...@gmail.com, Oct 11 2016

Seems like the issue stops 100% of the time if you send the component a piece of touch input, and then switch back to mouse.

Comment 11 by gmur...@gmail.com, Oct 11 2016

This does not appear to be anything that the component does upon receiving a touch event, however. Since the component uses jQuery bind to receive events, I'm able to simulate them using trigger. If I simulate a touch, it does not make the issue vanish. However, if I place a real finger on the component, it immediate makes the issue vanish.
Thanks for the trace. Are you calling preventDefault() in your mouse wheel handler? I suspect we might be falsely thinking you're trying to scroll the page and deprioritizing timers as a result.

Comment 13 by gmur...@gmail.com, Oct 24 2016

Yes we are.

Comment 14 by gmur...@gmail.com, Oct 25 2016

BTW, is there a good work around to keep it from being fooled into thinking the page is scrolling?
Cc: alexclarke@chromium.org
Owner: skyos...@chromium.org
Status: Started (was: Unconfirmed)
Apologies for the delay here. I did some investigation and turns out we've got a bug where we don't realize wheel events are being handled by the main thread. This causes Chrome to think it is scrolling the page using the compositor thread instead, leading to the slow main thread frame rate you're observing.

I'll put together a fix shortly.
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 10 2016

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

commit 4867673a4d5bad7c3030ac7bab0ee5bf41a6f634
Author: skyostil <skyostil@chromium.org>
Date: Thu Nov 10 19:46:25 2016

scheduler: Handle mouse wheel event disposition correctly

Previously the renderer scheduler would treat mouse wheel events as
prioritized events, but failed to keep track of whether they were
processed (and preventDefault()'ed) on the main thread or whether they
turned into a smooth scroll gesture. Making this distinction is
important because in the former case the main thread is doing custom
input handling (which we should be careful about scheduling) and in the
latter we are dealing with a regular scroll gesture handled either by
the main or the compositor thread.

This patch fixes the problem by adding explicit processing for the
MouseWheel event and a number of tests which cover all the relevant
scenarios.

BUG= 654765 

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

[modify] https://crrev.com/4867673a4d5bad7c3030ac7bab0ee5bf41a6f634/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/4867673a4d5bad7c3030ac7bab0ee5bf41a6f634/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc

Status: Fixed (was: Started)
Let's let this bake for a few days and then backport to M55.

Comment 19 by gmur...@gmail.com, Nov 11 2016

Thanks guys!
Labels: M-55 Merge-Request-55
Requesting a merge to M55.

Comment 21 by dimu@chromium.org, Nov 14 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 15 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94ca2b388cd38f64cd75af5766a3577c21f82411

commit 94ca2b388cd38f64cd75af5766a3577c21f82411
Author: Sami Kyostila <skyostil@chromium.org>
Date: Tue Nov 15 01:25:56 2016

scheduler: Handle mouse wheel event disposition correctly

Previously the renderer scheduler would treat mouse wheel events as
prioritized events, but failed to keep track of whether they were
processed (and preventDefault()'ed) on the main thread or whether they
turned into a smooth scroll gesture. Making this distinction is
important because in the former case the main thread is doing custom
input handling (which we should be careful about scheduling) and in the
latter we are dealing with a regular scroll gesture handled either by
the main or the compositor thread.

This patch fixes the problem by adding explicit processing for the
MouseWheel event and a number of tests which cover all the relevant
scenarios.

BUG= 654765 

Review-Url: https://codereview.chromium.org/2495593003
Cr-Commit-Position: refs/heads/master@{#431321}
(cherry picked from commit 4867673a4d5bad7c3030ac7bab0ee5bf41a6f634)

Review URL: https://codereview.chromium.org/2500253002 .

Cr-Commit-Position: refs/branch-heads/2883@{#570}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/94ca2b388cd38f64cd75af5766a3577c21f82411/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/94ca2b388cd38f64cd75af5766a3577c21f82411/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc

Cc: tkonch...@chromium.org
Labels: TE-Verified-55.0.2883.52 TE-Verified-M55
Tested the same on win10 chrome version  55.0.2883.52 - Zoom into the chart by rolling the mouse wheel and observed that the frame rate around 8fps

observed around 0.5 fps before the fix

hence tagging with verified labels
654765.png
215 KB View Download

Sign in to add a comment