Skip to content
This repository has been archived by the owner on Apr 16, 2019. It is now read-only.

Window touchstart listener can hurt scroll performance #3

Closed
RByers opened this issue Jan 5, 2016 · 14 comments
Closed

Window touchstart listener can hurt scroll performance #3

RByers opened this issue Jan 5, 2016 · 14 comments
Assignees

Comments

@RByers
Copy link

RByers commented Jan 5, 2016

I've been looking into the causes of poor scroll performance on Chrome for Android, and on a few sites parse.ly's time-engaged script has come up as being the thing that prevents scrolling from starting quickly due to the touchstart listener you add on the window (and other code on the page running slowly - eg. during page load).

I'd love to work with you to find some solution to this issue. It looks like you just want to keep track of the timestamp when the user interacts with the page, right?

One simple solution might be to switch from using touchstart time to using touchend time instead (since touchend events don't necessarily block scrolling, although we'd need to fix some things in chromium to actually optimize this case). But I can understand you might not want to change the definition for your engagement metric.

Another option is to have a way of requesting a non-scroll-blocking touchstart event. I'm working on such an API in passive event listeners. It looks like it would work for you, what do you think?

@emmettbutler
Copy link
Contributor

Thanks @RByers. It's plausible that we might switch from touchstart to touchend, though I can think of some cases where it could change the results. The passive event listener proposal sounds perfect for our use case. It's very much in Parse.ly's interest to minimize the impact of this code on client sites, so I'll be interested in any updates to the specification or ways I can help test. Thanks!

@emmettbutler emmettbutler self-assigned this Jan 5, 2016
@RByers
Copy link
Author

RByers commented Jan 7, 2016

Great, thanks! If the basic idea seems solid to you, then we'll keep working on it and ping this issue once we have the feature landed in chrome (eg. behind the "enable experimental web platform features" flag). It's going to be at least a few months (implementing it turns out to be quite hard), but it's great to hear that the idea sounds good to you.

I found another interesting example where fixing this would be hugely valuable. Try browsing this article on Chrome for Android. When you scroll up/down (hiding or showing the top controls) there is a very noticeable jank in the scroll. The underlying problem is an expensive relayout (which might be a site bug or might be a chrome issue), but parse.ly is the only thing installing a touch handler. If you remove it (eg. using dev tools) the site scrolls beautifully.

@RByers
Copy link
Author

RByers commented Mar 9, 2016

One question for you: is there a reason your script adds non-capturing listeners instead of capturing listeners (3rd addEventListener arg false vs. true)? In particular if the page has some other input listener that calls stopPropagation on the event, would you prefer to consider that event as activity still or not?

@emmettbutler
Copy link
Contributor

There isn't a strong reason for the current behavior. I think it's fine not to track events that have had stopPropagation called. I can see arguments for either side, though.

@KenjiBaheux
Copy link

@emmett9001

Kenji here, web platform product manager for Chrome.
Thanks a lot for your interest in this API.

Unless something unexpected happens, passive event listeners will be available by default in Chrome 51. It's enabled by default on the latest dev channel. (crbug.com/489802)

We are eager to see Parsely take advantage of it!
Let us know if there is anything that blocks you from doing so.

@RByers
Copy link
Author

RByers commented Apr 12, 2016

See this tweet for an example of the benefit possible and the enthusiastic support from users. Anecdotally there are definitely major sites on the internet that'll go from feeling crappy to feeling great (as in the video) once this one library opts in to passive listeners.

@dtapuska
Copy link

@emmett9001

Emmett are there plans to fix this time-engaged code? I attempted to follow the instructions so I could submit a pull request with the adjusted code but it seems that it doesn't build with grunt. (I don't have PARSELY.beacon defined.

@emmettbutler
Copy link
Contributor

@dtapuska See my comment here - this repository is a public mirror not currently meant to be used with grunt. Does #4 look usable to you? If so I can work on getting it into the mainline Parse.ly tracker.

@dtapuska
Copy link

dtapuska commented Jul 8, 2016

@emmett9001 Thanks! We look forward to hearing from you once you've integrated into the Parse.ly tracker.

@KenjiBaheux
Copy link

@emmett9001 Rick's pull request should have been usable. Any updates or issues?

@emmettbutler
Copy link
Contributor

Hi @dtapuska @KenjiBaheux @RByers, thanks for the contributions. Yesterday I completed the internal rollout of this change in Parse.ly's closed-source version of the tracker. As mentioned here, this repository is not used in Parse.ly's production environment and is meant only as a public mirror of the internal codebase. This mirror does need to be updated to reflect the latest changes to our internal tracker code, but we can now close this issue since passive listeners have been merged and deployed as tracker version 0.5.1.

If you use the Chrome Inspect Network tab on a Parse.ly-enabled site, you can check the version of the tracker being used by filtering the network request list by ptrack and refreshing the page. If it shows ptrack-v0.5.1 or higher, it's using passive listeners when possible. If not, the upgrade is temporarily blocked for that customer for reasons specific to their account.

@RByers
Copy link
Author

RByers commented Sep 6, 2016

@emmett9001: Thank you very much for the update (and sorry for the slow response). We can confirm that we see some usage of Parse.ly getting passive touch listeners, and the timing correlates well with an overall increase in adoption of passive listeners across all Chrome usage (still small at ~0.03% of page loads, but climbing rapidly).

@emmettbutler
Copy link
Contributor

That's great to hear. Thanks for following up. I've done a few other tracker upgrades in the meantime and they've all included the passive listener change.

@RByers
Copy link
Author

RByers commented Sep 8, 2016

Great, thanks! We (@Summerlw) are working on doing an analysis of what scripts most often cause blocking on the top 1000 sites or so. We'll let you know if/where parse.ly (presumably older tracker versions) shows up on that list.

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

No branches or pull requests

4 participants