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

Make the cache size configurable #135

Closed
dadrus opened this issue May 2, 2024 · 9 comments · Fixed by #152
Closed

Make the cache size configurable #135

dadrus opened this issue May 2, 2024 · 9 comments · Fixed by #152

Comments

@dadrus
Copy link
Contributor

dadrus commented May 2, 2024

First of all, thank you very much for the great project!

Currently, there is an option to configure the capacity of the cache in sense of the amount of items it should store via WithCapacity option. It does however not take the size of the items into account (by size I mean the memory used by the Item objects). In my application the size of the stored items can vary pretty much.

Typically, there is a strong need to configure the limits of a container, incl how much memory it should be able to use. Unfortunately, the above said option does not allow tuning the memory, the application making use of ttlcache, may use for cache purpose. And that makes the configuration of container limits pretty hard.

Do you see this as a viable option to add?

Thank you in advance.

p.s. The background for this FR is described in dadrus/heimdall#1250

@davseby
Copy link
Contributor

davseby commented May 2, 2024

Hi, an interesting idea. I'm not sure if we can compute the size of items ourselves without creating a fancy bytes computation method that may not work for everyone. Using unsafe's Sizeof doesn't seem to meet the requirements as well.

However, I think we could introduce a weight system. For example, when creating a cache a new option would be introduced.

func WithMaximumWeight[K comparable, V any](weight uint64, weightFn func(V) uint64) Option[K, V]

The user would be responsible for creating his own weight computation function which could be as flexible as one would want (e.g from computing the actual size in bytes to computing the weight using some sort of attribute, etc...).

@swithek thoughts?

@swithek
Copy link
Contributor

swithek commented May 2, 2024

However, I think we could introduce a weight system.

Yes, I agree. Besides, we might tackle more than one problem/use case with this kind of feature. However, one big issue with both of these approaches is that the values might grow in size/weight when no set/update/etc. operations are performed. I guess we would have to handle this with some kind of pub/sub (or notification) system, or just update the eviction queues when a new value is inserted/updated and not care about the value sizes at other times.

@dadrus
Copy link
Contributor Author

dadrus commented May 2, 2024

Maybe one could do something similar rueidis (a redis client) is doing here: https://github.com/redis/rueidis/blob/cf567f7740252126f7932bbdbb4c6be1613e0cfb/lru.go#L235. In their case, the value type is actually known in advance.

@swithek
Copy link
Contributor

swithek commented May 11, 2024

@dadrus thanks for the example. I just wonder how we should approach the problem I outlined above, i.e., should we track dynamic size updates (the value can be modified by the user if they still have a reference to it) and make changes/evictions based on that or not?

@dadrus
Copy link
Contributor Author

dadrus commented May 12, 2024

FMPOV, the implementation should only care about the values it has stored and not about changes done outside of it. So, my vote goes to "update the eviction queues when a new value is inserted/updated and not care about the value sizes at other times".

@dadrus
Copy link
Contributor Author

dadrus commented Aug 3, 2024

@swithek, @davseby: Are there any chances to have this FR addressed in the near future?

@swithek
Copy link
Contributor

swithek commented Aug 4, 2024

I think it's possible that this will be included in one of the next releases, but it's still not clear when they will be shipped out. Sorry for not being too specific, but if you want to speed things up, we always welcome new PRs.

@dadrus
Copy link
Contributor Author

dadrus commented Sep 24, 2024

If you don't mind, I would love to contribute the corresponding functionality (as I would like to have the corresponding feature in the upcoming version of my project). To be able to proceed, I would like to know whether you agree with the approach I've written in #135 (comment), or whether you have a different vision?

@swithek
Copy link
Contributor

swithek commented Sep 25, 2024

Of course, go for it!

Just to make it clear for everyone:

  • We need to check the size of the cache/items only on Set, which means evictions based on size should only happen on Set call as well.
  • The max size should be configurable via a function similar to WithCapacity.
  • If the max size is not set, no restrictions should be applied.

A few tips:

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

Successfully merging a pull request may close this issue.

3 participants