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

SimpleMemoryBackend - set max size #498

Open
rspadim opened this issue Jun 15, 2020 · 15 comments
Open

SimpleMemoryBackend - set max size #498

rspadim opened this issue Jun 15, 2020 · 15 comments

Comments

@rspadim
Copy link

rspadim commented Jun 15, 2020

When using SimpleMemoryBackend, should be important set the max size of internal dict

an idea is use an algorithm to select what to do when dict is "full", remove last element, first, random, rlu ...

@rspadim
Copy link
Author

rspadim commented Jun 18, 2020

@argaen argaen added the feature label Oct 15, 2020
@argaen argaen added this to the 0.12.0 milestone Oct 15, 2020
@prochanev-thw
Copy link

We have any problems with alru_cache while this feature not implemented?

@Dreamsorcerer Dreamsorcerer removed this from the 0.12.0 milestone Jan 2, 2023
@sergioave
Copy link

Any progress or implementation schedule?
Thank you

@Dreamsorcerer
Copy link
Member

Feel free to make a PR implementing it.

@Yiling-J
Copy link

Yiling-J commented Mar 5, 2023

Consider using Theine as the backend when you decide to implement it. Which is much faster than cachetools.

@Dreamsorcerer
Copy link
Member

Probably better if someone can create an external backend for theine (or even include it in theine itself, if they're intersted), rather than adding more dependencies here.

We can link to it in the README as an alternative backend if someone sorts that out.

@mirandadam
Copy link

Hi @Dreamsorcerer, I have submitted an incomplete PR to address this. Proper unit tests are needed and I have no idea how to implement them. Please advise.

@Yiling-J
Copy link

Actually I'm thinking, async-lru is also an aio-libs project, maybe it's possible to combine these two and use async-lru as the SimpleMemoryBackend? Theine is a little heavy because it has a Rust core(and maybe not that simple).

@Dreamsorcerer
Copy link
Member

I didn't even know we had that library. :P
If it's easy to drop-in, then I think that looks like a good solution. Ideally, we could make it an optional dependency.

@JPena-code
Copy link

Hi, I see that this issue is still open and I wonder. Do you think this feature is still required? Or is preferred to implement the external backend or the max size feature

@Dreamsorcerer
Copy link
Member

If it's easy to make async-lru an optional dependency, then I'm happy to put that in. If you want to use theine (which maybe has better performance?), then it should be implemented as an external backend (again, happy to maintain a list to external backends in the README).

@JPena-code
Copy link

Hi@Dreamsorcerer, I ran some benchmarks to test the performance of both packages, using the three cache policies that theine supports, against the lru cache provied by async-lru and the sample data were random values from a zipf and uniform distribution, Fell free to give me any advice.

image

@Dreamsorcerer
Copy link
Member

Still the same response. The only reason I think we should consider supporting async-lru as a built-in is because it's a library we also maintain, so any issue we can fix ourselves.

So, I think for any other backends, they should be implemented as additional libraries (or a class included within the related library) and maintenance to keep the backend working correctly can be handled there. We can then maintain a list of external backends from here. (I've made this suggestion before for other backends: #495 (comment))

It's up to you which option you'd like to work on, a new library with theine (or see if they'll take a PR to add to their adapters: https://github.com/Yiling-J/theine/tree/main/theine/adapters), or add async-lru here.

@mcpate
Copy link

mcpate commented May 8, 2024

If it's easy to make async-lru an optional dependency, then I'm happy to put that in.

@Dreamsorcerer, could you elaborate a little more on the above? What, roughly, would making async-lru an optional dependency look like? "Max size" is a feature I would love to see as well, and I would be happy to work on this (my thought was doing this directly in the SimpleMemoryBackend).

@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented May 10, 2024

Well, some approach that will keep all the current features available without installing async-lru. I haven't really thought about it, but you could have the parameter check if the library is available and raise a RuntimeError if not, for example.

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

No branches or pull requests

9 participants