Skip to content
This repository has been archived by the owner on Aug 10, 2022. It is now read-only.

Improve "Debounce your scroll handlers" and establish better best practice for "Avoid layout thrashing" #2227

Closed
aFarkas opened this issue Oct 26, 2015 · 5 comments

Comments

@aFarkas
Copy link

aFarkas commented Oct 26, 2015

The article and especially the code in "Debounce your scroll handlers" has some issues and can be improved a lot. It has to be said, that there are many different use cases to listen for scroll/resize events and not all of them can be handled with the same pattern.

Also the article Avoid large, complex layouts and layout thrashing
can be improved by establishing a new best practice, that can be easily fulfilled in practice.

So the pattern used in this article looks like this:

function onScroll (evt) {

  // Store the scroll value for laterz.
  lastScrollY = window.scrollY;

  // Prevent multiple rAF callbacks.
  if (scheduledAnimationFrame)
    return;

  scheduledAnimationFrame = true;
  requestAnimationFrame(readAndUpdatePage);
}

Let's go through it line by line.

  1. sync read in handler
// Store the scroll value for laterz.
lastScrollY = window.scrollY;

Access to scrollY is a read operation in terms of style/layout and can be done at any time. While the article says you shouldn't read layout in input handler, the code just does this. This makes absolutely no sense. (I come back later to this point).

  1. Throttle something, that is already throttled the same way.
// Prevent multiple rAF callbacks.
if (scheduledAnimationFrame)
  return;

scheduledAnimationFrame = true;

While there are some old browsers out there, where the resize/scroll events go wild. All modern browsers already throttle these events and this makes totally sense. A resize/scroll that is not painted, doesn't need to be dispatched.

Because a requestAnimationFrame callback is always scheduled before calculating the layout/style not only the handler but also everything inside of the rAf callback is blocking the browser from rendering the next frame.

This means the pattern doesn't really include any efficient throttling or debouncing.

This leaves us with the following pattern, which is async but not really debounced as described above:

function onScroll (evt) {
  requestAnimationFrame(readAndUpdatePage);
}

A simple throttle pattern could look something like this:

function onScroll (evt) {

  // Store the scroll value for laterz.
  lastScrollY = window.scrollY;

  // Prevent multiple rAF callbacks.
  if (scheduledAsync)
    return;

  scheduledAsync = true;
  setTimeout(readAndUpdatePage, 66);
}

Better best practice to avoid layout thrashing

The pattern also suggest to readAndWrite layout in a requestAnimationFrame. With this you don't win anything. As soon as multiple different scripts are following this advice, you end up with read -> write -> read -> write layout thrashing inside of a requestAnimationFrame:

function onScrollComponent1 (evt) {
  requestAnimationFrame(readAndUpdatePage);
}

function onScrollComponent2 (evt) {
  requestAnimationFrame(readAndUpdatePage);
}

Of course you can say, that this has to be managed by another third party script like fastdom, but it is escapist that all scripts (especially those you don't own) in a page are using fastdom.

In fact there is a much simpler pattern to fully avoid layout thrashing, that can be established without the need of a library, that everybody has to use.

It's simple: Put all your writes inside a requestAnimationFrame and never read inside a requestAnimationFrame.

If you think about it, this makes much more sense. requestAnimationFrame main purpose was always to do animation, hence layout writes. If the browser has calculated the layout and it is not invalidated, layout reads are extremely fast no matter at which point they are called. But the point where you can be sure of it is the point right after layout calculation. requestAnimationFrame gives you the time right before layout calculation, quite the opposite. It is the latest point to do layout reads before the browser would do layout himself, hence it is the most dangerous point to do reads. I know my explanation isn't so good, but I hope this makes it clear. See also this issue. If you have any doubts in this, just explain those. I will try to clear them.

This basically means for our debounce input handler article the following patterns:

Use case: Instant visual reaction to the event

function onScroll (evt) {
    // do all your reads...
    lastScrollY = window.scrollY;

    // do all your writes inside rAF
    requestAnimationFrame(updatePage);
}

window.addEventListener('scroll', onScroll);

Use case: lazy reaction to the event

// helper function (there are better ways to do it....)
function throttle(fn, delay) {
    var _this, args;
    var scheduled = false;
    var call = function () {
        scheduled = false;
        fn.apply(_this, args);
    };
    return function() {
        _this = this;
        args = arguments;
        if (scheduled) {
            return;
        }
        scheduled = true;
        setTimeout(call, delay || 66);
    }
}


var onScroll = throttle(function(evt) {
    // do all your reads...
    lastScrollY = window.scrollY;

    // do all your writes inside rAF
    requestAnimationFrame(updatePage);
});

window.addEventListener('scroll', onScroll);

Use case: React after event is settled

// helper function
function debounce(func, wait) {
    // we need to save these in the closure
    var timeout, args, context, timestamp;

    // this is where the magic happens
    var later = function() {

        // how long ago was the last call
        var last = Date.now() - timestamp;

        // if the latest call was less that the wait period ago
        // then we reset the timeout to wait for the difference
        if (last < wait) {
            timeout = setTimeout(later, wait - last);

            // or if not we can null out the timer and run the latest
        } else {
            timeout = null;
            func.apply(context, args);
        }
    };

    return function() {

        // save details of latest call
        context = this;
        args = arguments;
        timestamp = Date.now();

        // we only need to set the timer now if one isn't already running
        if (!timeout) {
            timeout = setTimeout(later, wait);
        }
    }
}


var onScroll = debounce(function(evt) {
    // do all your reads...
    lastScrollY = window.scrollY;

    // do all your writes inside rAF
    requestAnimationFrame(updatePage);
});

window.addEventListener('scroll', onScroll);

Of course you can also add a forth pattern using a mixture of throttle and requestIdleCallback.

@petele
Copy link
Member

petele commented Oct 26, 2015

@paullewis - can you please take a look and adjust as necessary. thanks!

@paullewis
Copy link
Contributor

Ok, let's go through this bit by bit:

Debouncing

The general idea of the code I wrote was to suggest that you want to debounce all handlers in this way, not just scroll. If you were to add listeners for two events, say a touchend and click, like this:

document.addEventListener('touchend', function () {
  requestAnimationFrame(update);
});

document.addEventListener('click', function () {
  requestAnimationFrame(update);
});

function update(now) {
  console.log(now);
}

You will get two rAF callbacks firing immediately, one after the other. The generalized solution here would be:

var scheduledAnimationFrame = false;
var lastTouchInfo = null;
var lastClickInfo = null;
document.addEventListener('touchend', function (evt) {

  if (scheduledAnimationFrame) return;

  scheduledAnimationFrame = true;
  lastTouchInfo = evt.touches;
  requestAnimationFrame(update);
});

document.addEventListener('click', function (evt) {

  if (scheduledAnimationFrame) return;

  scheduledAnimationFrame = true;
  lastClickInfo = {x: evt.clientX, y: evt.clientY};
  requestAnimationFrame(update);
});

This will now mean you get one animation frame callback fired, in which you can now handle. Your rAF callback would be something like:

function update () {
  // Process the input: scrolls, touchmove, click, etc.
  handleInputFromFrame();

  // Do any visual work.
  updateElementsAndOtherAnimations();
}

So you're right that we don't need to debounce for a single handler, but by doing all handlers this way we are protected if we have multiple events that fire in a single frame.

Next, your proposed solution of running the callback in a setTimeout would mean that you get JavaScript fired at a random position in a frame. That's not good if the JS fires during a busy frame.

When to read, when to write

You're suggesting that we should write only inside of a requestAnimationFrame, and never read there. What would you do if you needed to read some styles or position information? You're coupling to input events that may not fire for reading.

If you have multiple components you do need to do something similar to FastDOM, where each component separately registers its need to read, and the mutate the DOM. Your rAF then becomes:

function update () {
  // Process the input: scrolls, touchmove, click, etc. if needed
  handleInputFromFrame();

  // Process all reading... If needed.
  readers.forEach(function(reader) {
    reader.read();
  });

  // Then all writing... If needed.
  writers.forEach(function(writer) {
    writer.write();
  });

  // Do any visual work. If needed.
  updateElementsAndOtherAnimations();
}

Other miscellaneous bits

If the browser has calculated the layout and it is not invalidated, layout reads are extremely fast no matter at which point there are called.

and ...

There is simply no valid fact suggesting that reads are faster inside of a rAF.

I'm not sure anyone is suggesting it's faster, just more in-line with the browser's frame lifecycle. In fact you can do reads inside of an input event handler, and the values would be available from the previous frame's layout pass in the same way as with rAF. However, if your event handlers runs long for some reason and you have a rAF then it will run late, i.e. not close to vsync, and generally it's the case we want rAF to run as close to vsync as possible.

requestAnimationFrame gives you the time right before layout calculation, which means it is the latest point to do layout reads before the browser would do layout himself, hence it is the most dangerous point to do reads.

That's only sort-of true in Blink's case. After the previous frame commits, and the next frame starts, Blink will process input events, then rAF, then it will apply styles. Your options are to read either in your event handlers which may not run, and so can't be relied up for anything but reacting to the user, or to read at the start of rAF. But there are only two places to read: input events (may fire, may not depending on what the user does), or rAF (will fire for sure if you need it to, and you can bail out of doing anything if you don't have any new data to process).

I don't think we disagree on the fact that one should do reads then writes, but here's my position on it:

  1. We need to treat all input events as optional; they may be fired in a given frame, they may not, depending on what the user does.
  2. We should simply capture any events that do get fired (sometimes multiples ones get fired in the same frame, like touchend and click), i.e. treat them as read-only. To treat them as able to read styles or layout is fine, but in my experience it's more robust to not do that.
  3. We can either always fire a requestAnimationFrame if we have visual effects that are independent of input or, if we don't, we can schedule one on demand.
  4. If you need to do style / layout reads you can do them in the input handler, but if it doesn't fire you would have been better off doing them at the start of rAF.
  5. If multiple components need to read from, and mutate the DOM, they should form two queues: the readers, then the writers. Otherwise we will get ourselves into Forced Sync Layout territory.

Finally, this approach extends to requestIdleCallback, which can run after the previous frame's style and layout tasks, and which would also be safe for reading. However, if you wanted to read and write after some idle computation, I would recommend joining the queue in the same way as components and other input handlers.

@aFarkas
Copy link
Author

aFarkas commented Oct 26, 2015

@paullewis
You might not be aware of this, but your code is copied a million times. For x different use cases, therefore it might be wise to clarify some things. So that developers are actually knowing what the code exactly does and why.

So you're right that we don't need to debounce for a single handler, but by doing all handlers this way we are protected if we have multiple events that fire in a single frame.

This is an important information/fact, that is not clear from the article. The example only uses one handler for one event. And the article doesn't state this as a problem, that you want to (also) solve.

Also your example is quite constructed. It looks like a simple implementation of fastclick but if you know how this works you will know that there can be a 300ms gap. And you cannot

Next, your proposed solution of running the callback in a setTimeout would mean that you get JavaScript fired at a random position in a frame. That's not good if the JS fires during a busy frame.

At the end: this totally depends on the use case. Which I was talking about in my original issue. For example your code is also used as backbone for lazyloaders, I can demonstrate that using the setTimeout approach is much better for this use case, than the requestAnimationFrame pattern.

This is also important, because you have used the term debounce, which was used until then for something else. And debouncing and throttling the old school way (using setTimeout) have still valid use cases. People should know this by reading this article.

At the same time you are changing the topic here. We speak about your example that uses rAF not requestIdleFrame. rAF is called every frame including the busy ones.

If you have multiple components you do need to do something similar to FastDOM, where each component separately registers its need to read, and the mutate the DOM.

Yes you would need something to make it more convenient and remove the amount of closures. But what I was stating is that you only need to manage the writes (with or without fastdom.write) and that you don't need to ensure the use of one library to get the work done. As soon as all writes happen inside a rAF, reads are save anywhere else (outside rAF).

I wrote a lot of components lately and they do not thrash layout. And if combined with third party components, they do not provoke layout thrashing by invalidating layout. If I would have used old fastdom.read, it would automatically and always thrash layout if combined with any JS animation framework that uses rAF.

Your options are to read either in your event handlers which may not run, and so can't be relied up for anything but reacting to the user....

Yeah, article was all about input events. This is why I choose my example this way.

As said, I wrote a lot of components and they read either immediately on initialization or immediately inside a sync or in a throttled event handler. Due to the fact that all writes are moved, there is no layout thrashing.

To treat them as able to read styles or layout is fine, but in my experience it's more robust to not do that.

If at the time of a handler is called, the styles are invalidated, a rAF/fastdom.write doesn't solve the problem either, the styles remain invalidated until the rAF callback is invoked and forces layout.

If multiple components need to read from, and mutate the DOM, they should form two queues: the readers, then the writers.

With my approach a readers queue is only needed, if you need to read right after a write/mutation (because we are in rAF land now) otherwise just do your read as you need the data/information.

[reading inside a rAF is]... just more in-line with the browser's frame lifecycle.

This seems to be more of a feeling than a necessity. Normally you want to gather information right after they are changed/created and not right before they become invalid.

@aFarkas
Copy link
Author

aFarkas commented Oct 28, 2015

I made a simple proof of concept. I took the following demo:
http://www.html5rocks.com/en/tutorials/speed/animations/post.html

The timeline of this looks like this:
original

I haven't cared about the layout thrashing, that is already there. Instead I have focused on the additional layout thrashing (i.e. fortunately in our case only style recalculations), that happens, if you produce multiple instances with the code. This can also simulate multiple components in a website. The demo of this can be seen here:
http://afarkas.github.io/scroll-perf/original.html

And the timeline looks like this:
original-multi

As expected the reads of a following instance are causing layout thrashing.

As we all know this can be solved by using fastdom's read and write. But it is important to note, that this can also be solved by just using fastdom write or even rAF alone. Which is proven on this demo page:
http://afarkas.github.io/scroll-perf/optimized.html

And here is the corresponding timeline:
fastdom-write-multi

In my eyes this solution can't be underestimated, because of two things:

1. Simplicity
Building two queues one for reads and one for writes needs some code and therefore
most tutorials and developers only concentrate on solving layout thrashing inside of one instance of one component. (For example I haven't seen any vanilla JS example, that avoids layout thrashing, if multiple instances of the same component are created. Beside the article about fastdom itself.) Using rAF on the other hand to move all writes after all reads is so simple and straight forward, that it could be used in every little tutorial.
2. Unavoidable conflicts
By using two queues and moving those at the same spot (i.e.: rAF callback) right after another. Developers have to make sure, that not only their code, but also any other code on the webpage has - not only - to use the same pattern to avoid layout thrashing, they also have to make sure that any script is using the same instance of the same library, that ensures reads and writes. Basically any direct use of requestAnimationFrame is prohibited. This is hard to achieve, if you think of the many scripts, libraries and frameworks, that are using rAF.

A demo that simulates this problem by using fastdom read and write in combination with a "third party script", that uses requestAnimationFrame can be seen here:
http://afarkas.github.io/scroll-perf/fastdom-animate.html

The corresponding timeline of this looks like this:
fastdom-read-write-animate-multi

The corresponding demo, that uses the much simpler pattern (only fastdom.write/rAF only for writes), can be seen here:
http://afarkas.github.io/scroll-perf/optimized-animate.html

And here is the corresponding timeline:
fastdom-write-animate-multi

@tryhardest
Copy link

@paullewis @jakearchibald could I kindly implore either of you to check out and give your response to @aFarkas plea to reopen issue from Aug.

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

No branches or pull requests

4 participants