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

Improve accuracy of _.throttle #1952

Merged
merged 2 commits into from
Feb 7, 2016
Merged

Improve accuracy of _.throttle #1952

merged 2 commits into from
Feb 7, 2016

Conversation

dcorb
Copy link
Contributor

@dcorb dcorb commented Feb 7, 2016

Repeated throttled executions are not as accurate as they could be.
For example, when

var throttled = _.throttle(doSomething,400)
and calling repeatedly throttled(); every 200ms.

I expect the executions happen once every 400ms, but sometimes there are delays of more than 100ms until the next execution.

This pull request fixes the situation at trailing:true, leading:false and `trailing:true, leading:true``

There is no way of achieving a 400ms rate of execution for trailing:false, leading:true in this example I put, because by definition, is impossible to foresee the future. We need to wait for the next throttled()

I discover this lack of accuracy with this example:

$('.clickme')
    .on('click', _.throttle(function (){  
     t1 = performance.now(); 
     console.log('Time from last throttle: ', (t1 - t2).toFixed(4), 'ms ');
     t2 = performance.now();
  }, 400, {trailing:true, leading:false}))
  .on('click', function(){
    t0 = performance.now(); 
    console.log('Time from last click:', (t0-t4).toFixed(4), 'ms ');
    t4 = performance.now();
  })

image

Don't overwrite lastCalled (distorting time accuracy of throttled executions) when it has a valid value
@jdalton
Copy link
Member

jdalton commented Feb 7, 2016

Wow. Thank you for taking the time to dig into this. The throttle and debounce code is always so hard to reason about without a visual or example (timing is hard).

You're a big help here.
Have you signed our cla?

@jdalton jdalton added the bug label Feb 7, 2016
@dcorb
Copy link
Contributor Author

dcorb commented Feb 7, 2016

Thank you, It's quite tough to wrap your head around it, indeed!.
I hope this change doesn't result in any side-effect, but I'm pretty sure it's safe.
I just signed the CLA (submitted a CLA on 2016-02-07 01:05:32)

I will keep you updated when the article about debounce/throttle gets published, with a revamped visualisation tool

jdalton added a commit that referenced this pull request Feb 7, 2016
Improve accuracy of _.throttle
@jdalton jdalton merged commit 8a1028c into lodash:master Feb 7, 2016
@bman654 bman654 mentioned this pull request Mar 7, 2016
@lock
Copy link

lock bot commented Dec 27, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

2 participants