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

Rework subscription handling to batch "need to subscribe but entry exists" behavior #2593

Closed
markerikson opened this issue Aug 14, 2022 · 3 comments
Milestone

Comments

@markerikson
Copy link
Collaborator

We've had several folks report that rendering a long list of components with RTKQ hooks can be very slow. My suspicion is that it's because we dispatch separate "need to subscribe" actions for every component. The first component will fetch the data and create the cache entry. All other components will see the cache entry exists, dispatch a rejected action, and the subscription logic handles that to add another subscription.

Additionally, as components unsubscribe, there's a lot of timer cancelation and resetting going on.

I think we need to figure out how to batch the "cache entry exists but need to subscribe" behavior into some debounced batched actions to cut down on the overhead. We may also need to tweak the unsubscribe handling as well to cut down on timer manipulation.

Don't have a performance capture atm - I should set up an example.

@markerikson markerikson added this to the 1.9 milestone Aug 14, 2022
@markerikson
Copy link
Collaborator Author

Related: I just fixed keepUnusedDataFor timer counters in #2595 . Lenz also suggested:

Maybe with that knowledge, we should also allow for some kind of "keep indefinitely" option with Math.INFINITE or just -1?

I like Math.INFINITE myself. That kinda ties in here since I need to rethink the timer behavior anyway, so note to myself to add that as part of this work.

@markerikson
Copy link
Collaborator Author

Both done in #2599 .

@markerikson
Copy link
Collaborator Author

note for later reference - there's an example repo at https://github.com/nibblebot/rtkq-timer-perf :

I setup a repo here https://github.com/nibblebot/rtkq-timer-perf that reproduces the issue. It simply renders a list of 100 components with queries bound to them and a button that toggles the whole list. If you do a chrome profile and filter the events to "Install Timer" or "Remove Timer" you can see the 100 timer install/remove when you toggle the list. However, the cost of Install/Remove Timer here appears to be 0ms so the impact is not the same as my other repro case in my private code base. Testing with the 1.9.0-alpha.0 shows these events are removed from the profile event log so I'd say the new version fixes the issue.

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

No branches or pull requests

1 participant