-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[draft] Cache collection #92
base: main
Are you sure you want to change the base?
Conversation
This is probably a little different than what's trying to be achieved with a LRU, but I really like how react-query manages stale data with a duration. Demo: I wanted to share this in case it could be useful in coming up with a design for a flexible caching system. |
@rostero1 it is a LRU (least recently used) model, but size based (not duration based). The reason is that I think this implementation should be more efficient after reading the implementation of these libs: The goal of this lib is to be as lightweight and fast as possible, and like I mentioned in this PR, most users don't even need the LRU:
And since hooks are being called very frequently in the UI, cache collection especially a duration based cache collection process will slow down the rendering a lot. That's why I kept this PR on hold and want to maximize its performance before landing it. So I'm curious about the real world use case of managing stale data with a duration. Because after a long time, what you should do for the cache is revalidation, not expiration. |
Thanks, @quietshu. I'm not sure if this conversation should live somewhere else, but I have some data that appears as a dropdown list in multiple areas; for example, in the left side bar, and then in the right sidebar, deeply down in a few tabs. Note that because of privacy I cannot use any HTTP caching for my REST APIs. After thinking about this some more, I find that I'm using
For the app I'm working on I was trying to come up with a scenario where I actually need to refetch the data after a |
@quietshu have you thought more about this? I think it could be implemented manually per project using subscribe and custom hooks but maybe it could be integrated directly in SWR, do you mind if I work on this? |
I'd like to add some more clarification on the duration-based cache expiration in React Query for you guys to make a better decision here. Hopefully it helps. Duration based expiration on it's own is useless, since you would never want to expire something in the cache if it was currently in use, and likewise, if you know that a query result is never going to be used again or very infrequently, you don't want to keep it around forever. However, duration based caching is actually a very good approach to UI and how users interact with it. Users are visiting different parts of your app more than other parts and those frequencies may vary drastically in timing, but have a very low cardinality (like you said, many apps don't have a lot of unique query cache items). So to combat this in React Query, I track the "active"-ness of a query by how many hooks instances at any given time are using a query cache value. That cache can still become stale (staleTime), and get revalidated, but the moment that there are 0 active instances on the page for a given query cache item, it gets marked for garbage collection after With that explained, you can see how a duration based caching system could easily get out of hand if you were firing off thousands upon thousands of unique queries with different query keys and they took a while to expire, BUT, like you said, most apps (even larger enterprise-sized apps) don't use that many unique queries (maybe in the hundreds over a long period of time). I researched this quite a bit when I was building React Query and IMO, you guys should go the direction of a duration based cache system, not an LRU for similar reasons. But hey, that's just my two cents! Good luck guys! |
Thank you @tannerlinsley for your great suggestion!
First of all this LRU implementation tracks the active hook instances as well :D For sure I can add a timestamp to each item in this LRU cache, to make it duration based. It would be just 2 or 3 lines of code, but my biggest concern is still the performance. Like I commented a while ago:
I'll do some benchmark and test it with real world scenarios to see:
Thanks again! Cheers! |
"requestIdleCallback" method is not supported in Safari. Is it okay to use it? Does SWR core use that method? If so, was it used with a polyfill? |
SWR used to use |
One thing that holds SWR from moving forward is that the cache doesn't have a timestamp that tells when it's saved. Without it, some feature like Let me know if there is some workaround. @shuding |
This PR adds inactive (LRU based) cache collection implementation.
By default <=600 inactive keys will be stored.
However, there're a couple of things we need to explore further before landing this feature.
Performance
For most (~99%) websites, the number of requests per page won't be more than 200. [1]
So this feature will increase the unnecessary workload by a lot.
A solution can be just skip handling cache collection until its size is near ~3/4 of the maximum allowed cache size.
API
How does it work with:
How to customize the cache size?