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

Feature Request: Allowing for unbounded cache size #34

Closed
pcman312 opened this issue Jun 8, 2017 · 7 comments
Closed

Feature Request: Allowing for unbounded cache size #34

pcman312 opened this issue Jun 8, 2017 · 7 comments

Comments

@pcman312
Copy link
Contributor

pcman312 commented Jun 8, 2017

Being forced to provide a cache size during construction of the cache doesn't work well for our particular use case. The size of our cache is bounded in other more real-world ways and we don't need to worry about the cache growing out of control. To that end, I'd like to request a new function on the cache builder to allow for no max cache size. The New() method could be changed such that it allows for <=0 size that is interpreted as unbounded.

@erwanor
Copy link
Contributor

erwanor commented Jun 12, 2017

Agreed. This is something I have been wondering as well. I am not too sure what purpose SimpleCache serves as it is. Especially since a gorountine-safe map is coming soon (see: golang/go#18177).

If @bluele is ok with this, I'd like to modify SimpleCache to be unbounded by default.

@bluele
Copy link
Owner

bluele commented Jun 13, 2017

It's a nice feature.
Let's add this feature to all types, not just simple cache type.

@erwanor
Copy link
Contributor

erwanor commented Jun 13, 2017

@bluele Why though? Does it make sense to have allow an unbounded LRU/LFU/ARC cache. Eviction will just never happen. In that scenario, it is more than likely that this is an error that should be reported.

@pcman312
Copy link
Contributor Author

I agree with @aaronwinter. If you are never evicting from the cache, the method of evicting (LRU, LFU, ARC) doesn't matter.

@bluele
Copy link
Owner

bluele commented Jun 13, 2017

Thanks @aaronwinter and @pcman312 .
That's true. There seems to be no case to use evicting with expiration only, so it should only be implemented to Simple cache.

@aaronwinter
If there is no other problem, could you implement this feature and its test?

@erwanor
Copy link
Contributor

erwanor commented Jun 14, 2017

@bluele Cool, thanks for the quick reply. I will work on this (along other features on my backlog) this week-end.

@erwanor
Copy link
Contributor

erwanor commented Oct 11, 2017

This feature request can be closed. Addressed in #38

@bluele bluele closed this as completed Oct 11, 2017
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

3 participants