Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance #31

Open
paulirish opened this issue Jan 21, 2015 · 5 comments
Open

Performance #31

paulirish opened this issue Jan 21, 2015 · 5 comments

Comments

@paulirish
Copy link
Collaborator

First of all, everything is my fault. Sorry.

Second of all, I think we can improve the performance of this script. :)

We currently bind to all of these events to judge if a user is active:

events: "mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove" 

This makes sense in a way because we really want to know if the user does anything, sure. But…

The key issue here is that in many cases the browser cannot update the UI until the event handler has finished. So the user's scroll, mousemove, or touch is completely held up while it waits for our script to finish before actually getting closer to showing the user the outcome from that interaction.

image

This is described really well in this video:

https://www.youtube.com/watch?v=YyQYhhy1dZI#t=945 watch from 15:45 until 25:00

At the end, you can see a shot of the "show scroll bottlenecks" feature in devtools where this script triggers a bunch:
image

Chrome's touch maestro Rick Byers also gave a long explanation of why touch handlers wreck performance: https://plus.google.com/+RickByers/posts/cmzrtyBYPQc?e=-RedirectToSandbox

touch

The touch events are unnecessary because a touch will end up triggering mousedown anyway.
https://patrickhlauke.github.io/touch/tests/results/ has all the details

the original list:

  • mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove

If we remove superfluous events that we can catch with another binding:

  • mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove

scroll

Binding to mousewheel, mousescroll is not great, as Nat & Tom offer in that video above. Resig offered similar advice in 2011 (see best practices). A better approach is polling for changes in scrollTop.

page visibility API

it's got great support and is a very simple indicator of "idle", so we could turn off polling then.


So then we're left with..

mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove (and a page-visibility-aware scrollTop polling loop)

I think that's a fine list: mousedown mousemove keydown. We won't regress how well we monitor idle/active and we'll speed up all interactions with the pages considerably.

@thorst how does that sound to you?

@thorst
Copy link
Owner

thorst commented Jan 21, 2015

Sounds sweet.

Ill dig into these and see what I come out with. We are due for a new version.

Thanks for the tip.

Sent from !MyComputer;

On Jan 21, 2015, at 12:14 AM, Paul Irish [email protected] wrote:

First of all, everything is my fault. Sorry.

Second of all, I think we can improve the performance of this script. :)

We currently bind to all of these events to judge if a user is active:

events: "mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove"
This makes sense in a way because we really want to know if the user does anything, sure. But…

The key issue here is that in many cases the browser cannot update the UI until the event handler has finished. So the user's scroll, mousemove, or touch is completely held up while it waits for our script to finish before actually getting closer to showing the user the outcome from that interaction.

This is described really well in this video:

https://www.youtube.com/watch?v=YyQYhhy1dZI#t=945 watch from 15:45 until 25:00

At the end, you can see a shot of the "show scroll bottlenecks" feature in devtools where this script triggers a bunch:

touch

The touch events are unnecessary because a touch will end up triggering mousedown anyway.
https://patrickhlauke.github.io/touch/tests/results/ has all the details

the original list:

mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove
If we remove superfluous events that we can catch with another binding:

mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove
scroll

Binding to mousewheel, mousescroll is not great, as Nat & Tom offer in that video above. Resig offered similar advice in 2011 (see best practices). A better approach is polling for changes in scrollTop.

page visibility API

it's got great support and is a very simple indicator of "idle", so we could turn off polling then.

So then we're left with..

mousemove keydown wheel DOMMouseScroll mousewheel mousedown touchstart touchmove MSPointerDown MSPointerMove (and a page-visibility-aware scrollTop polling loop)

I think that's a fine list: mousedown mousemove keydown. We won't regress how well we monitor idle/active and we'll speed up all interactions with the pages considerably.

@thorst how does that sound to you?


Reply to this email directly or view it on GitHub.

@thorst
Copy link
Owner

thorst commented Jan 23, 2015

Polling scrollTop, however unlikely, has a potential for missed events. I know, it its insanely edge case. In addition, I would have a looper that only is used for monitoring scrolling.

Looking at Resigs example, if I were to set a bit on scrolling and have and have a looper checking the bit, I may as well use that same function for all event handlers. This way the only thing being done on any event is to set the boolean, which is minimal time wasted.

Thoughts?

@paulirish
Copy link
Collaborator Author

Passive event listeners would work for all these guys:

https://github.com/WICG/EventListenerOptions/blob/gh-pages/explainer.md

easy win.

@thorst
Copy link
Owner

thorst commented Sep 2, 2016

Nice. I can add that a while, I was planning on doing a rewrite when import/export are in. Do you know when that will be happening?

@paulirish
Copy link
Collaborator Author

ah modules? it'll be another 8 months at least. :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants