aio-libs / aiocache

Asyncio cache manager for redis, memcached and memory
http://aiocache.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.13k stars 154 forks source link

SimpleMemoryBackend - set max size #498

Open rspadim opened 4 years ago

rspadim commented 4 years ago

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 commented 4 years ago

This could help:

https://cachetools.readthedocs.io/en/stable/

https://github.com/hephex/asyncache/blob/master/asyncache/__init__.py

prochanev-thw commented 3 years ago

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

sergioave commented 1 year ago

Any progress or implementation schedule? Thank you

Dreamsorcerer commented 1 year ago

Feel free to make a PR implementing it.

Yiling-J commented 1 year ago

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

Dreamsorcerer commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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: https://github.com/aio-libs/aiocache/issues/495#issuecomment-1368573174)

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 commented 5 months ago

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 commented 5 months ago

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.