Performance issue in @tanstack/query-async-storage-persister (and possibly #updateStaleTimeout)

[Jack-Works]

Describe the bug

It looks like delayFunc in the packages/query-async-storage-persister/src/asyncThrottle.ts has some performance problem.

  const delayFunc = async () => {
    clearTimeout(timeout)
    timeout = setTimeout(() => {
      if (running) {
        delayFunc() // Will come here when 'func' execution time is greater than the interval.
      } else {
        execFunc()
      }
    }, interval)
  }

this is the 4x slow-down profile, each refetch call causes persistQueryClientSave to be called, and then delayFunc is the slowest function in the tree. All small yellow bars are setTimeout or clearTimeout calls.

My throttle timeout is the default (1e3), therefore it cannot be caused by execFunc (all those delayFunc calls happen within 1 second).

I try to patch my node_modules and replace the asyncThrottle with this:

function asyncThrottle2(func, { interval = 1e3, onError = noop } = {}) {
    if (typeof func !== 'function') throw new Error('argument is not function.')
    let currentArgs = null
    setInterval(() => {
        if (!currentArgs) return
        Promise.resolve(func(...currentArgs)).catch(onError)
        currentArgs = null
    }, interval)
    return (...args) => {
        currentArgs = args
    }
}
export { asyncThrottle2 as asyncThrottle }

This implementation is incorrect but fast.

After the patch, persistQueryClientSave now only costs 1 ms instead of 57 ms.

I think it looks like calling setTimeout or clearTimeout is costly. I think a better async throttle implementation can avoid calling them so often.

btw I also observed that #updateStaleTimeout method has a similar behavior.

also, this method behaves like delayFunc, simple but costs a lot of time (cancel & create timer).

Your minimal, reproducible example

n/a

Steps to reproduce

Sorry I didn't provide one, but my patch works so I think my analysis of the performance problem is correct.

I'm happy to try any new asyncThrottle implementation (by patch node_modules, so maintainers don't have to release a new version for this) in our project to see if that fixes the performance problem. Thanks!

Expected behavior

No performance problem

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Chrome 119

@tanstack/query-async-storage-persister 5.8.7

Tanstack Query adapter

None

TanStack Query version

5.8.7

TypeScript version

No response

Additional context

No response

[TkDodo]

I'm happy to try any new asyncThrottle implementation

if you have a better implementation that is also correct, please do contribute 🙏. The asyncThrottle function does have a decent test coverage.

Maybe we can also get inspired by other, already existing implementations?

• https://github.com/sindresorhus/p-throttle

[TkDodo]

@Jack-Works would you consider contributing your patch back to the persister plugin?

• https://github.com/DimensionDev/Maskbook/pull/11227/files

[Jack-Works]

@Jack-Works would you consider contributing your patch back to the persister plugin?

* [DimensionDev/Maskbook#11227 (files)](https://github.com/DimensionDev/Maskbook/pull/11227/files)

Hi! I think our patch is obviously imperfect (it is over simplified and maybe even incorrect). I hope your team can find another throttle implementation and I'd like to try it.

[jhnns]

Maybe we can also get inspired by other, already existing implementations?

[sindresorhus/p-throttle](https://github.com/sindresorhus/p-throttle)

@TkDodo Genuine question: why don't you use p-throttle? :) I'm just trying to understand what makes asyncThrottle special.

[webNeat]

Hello @jhnns,

I was also wondering why not just use p-throttle, so I looked into the two implementations and they don't actually behave the same.

Take the following example:

const start = performance.now();
const fn = (a, b) => {
  const time = Math.floor(performance.now() - start);
  const result = a + b;
  console.log({ time, result });
};

const throttledFn = asyncThrottle(fn, { interval: 500 });

throttledFn(1, 2); // prints: { time: 0, result: 3 }
throttledFn(1, 3); // ignored
throttledFn(1, 4); // ignored
throttledFn(1, 5); // ignored
throttledFn(1, 6); // prints: { time: 500, result: 7 }

So to my understanding, asyncThrottle only keeps the last call waiting and ignores all the other calls.

While p-throttle keeps all the calls waiting and executes them in order:

import pThrottle from "p-throttle";

const start = performance.now();
const fn = (a, b) => {
  const time = Math.floor(performance.now() - start);
  const result = a + b;
  console.log({ time, result });
};
const throttle = pThrottle({
  limit: 1,
  interval: 500,
});
const throttledFn = throttle(fn, { interval: 500 });

throttledFn(1, 2); // prints: { time: 0, result: 3 }
throttledFn(1, 3); // prints: { time: 500, result: 4 }
throttledFn(1, 4); // prints: { time: 1000, result: 5 }
throttledFn(1, 5); // prints: { time: 1500, result: 6 }
throttledFn(1, 6); // prints: { time: 2000, result: 7 }

I didn't find a way to tell p-throttle to only keep the last call, so I don't think it can be used as a drop-in replacement for asyncThrottle.

[webNeat]

@Jack-Works I made the PR https://github.com/TanStack/query/pull/7224 that suggests a new implementation. Can you please try it on your project and let us know if it fixes the issue?

[TkDodo]

fixed by:

• #7224