-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
useQueries have quadratic performance in relation to the number of queries #8604
Comments
The problem is the If you have N queries managed by I could be wrong, of course, but I think reusing function getOptimisticResult(queries, combine) {
const matches = this.#findMatchingObservers(queries)
const result = matches.map(match => ...)
return [
result,
(r) => {
return this.#combineResult(r ?? result, combine)
},
() => {
- // Avoid calling #trackResult which calls findMatchingObservers again
- // Instead, directly reuse the 'matches'
+ return matches.map((match, index) => {
+ const observerResult = result[index]
+ // ...
+ })
},
]
}
|
I don't think there is any issue with getOptimisticResult. It'll only be called once in my scenario. It then doing N operations is fine as it only leads to O(N) performance. Maybe it could be even better with better bookkeeping but that is not what my issue is about. The issue is with |
Describe the bug
This PR #8295 introduced a significant performance regression for us. A commenter on that PR seem to have had a similar issue #8295 (comment).
We use the data loader pattern together with useQueries to give individual results from a batch fetch individual cache entries in react-query. Something like this:
Looking at a performance profile i noticed this will result in one call to #findMatchingObservers in QueriesObserver for every query when the fetch resolve. #findMatchingObservers will then loop over all observers for for all queries internally making the performance quadratic O(n^2).
In our case we were loading way too many ids at once for this to be reasonable so the performance was already bad before the change but became browser freezing bad after the change.
Seems like the problem however should be avoidable. #onUpdate know what observer was updated so should not have to find it. This might however require some reorganization of the code and how observers are stored in QueriesObserver.
The indexOf in #onUpdate is also a source of quadratic performance that should be avoidable however indexOf is a way faster operation than what happens in #findMatchingObservers so it might not be an issue until even more queries are fetched at the same time.
Your minimal, reproducible example
https://codesandbox.io/p/sandbox/boring-https-dxlhnv?file=%2Fsrc%2FApp.js%3A17%2C62
Steps to reproduce
Click on the numeric buttons to try loading different amount of queries and click on the "useQueries" and "multiple useQuery" buttons to switch between using a single useQueries or multiple useQuery and observer the performance difference.
Expected behavior
I expect useQueries to have similar or faster performance than multiple useQuery.
How often does this bug happen?
Every time
Screenshots or Videos
No response
Platform
Tanstack Query adapter
react-query
TanStack Query version
5.64.2
TypeScript version
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: