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

Extend SimpleCache to allow for unbounded growth #38

Merged
merged 5 commits into from
Oct 10, 2017
Merged

Extend SimpleCache to allow for unbounded growth #38

merged 5 commits into from
Oct 10, 2017

Conversation

erwanor
Copy link
Contributor

@erwanor erwanor commented Jul 6, 2017

In this PR, we allow users to create a (SimpleCache) cache with no artificial upper bound on the number of elements that it can host. To achieve that, the user will specify a size <= 0 before building the cache.

simple.go Outdated
@@ -18,7 +18,11 @@ func newSimpleCache(cb *CacheBuilder) *SimpleCache {
}

func (c *SimpleCache) init() {
c.items = make(map[interface{}]*simpleItem, c.size)
if c.size == -1 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want size==0 to means unbounded, so could you change to c.size <= 0 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

simple.go Outdated
@@ -58,7 +62,7 @@ func (c *SimpleCache) set(key, value interface{}) (interface{}, error) {
item.value = value
} else {
// Verify size not exceeded
if len(c.items) >= c.size {
if (len(c.items) >= c.size) && c.size != -1 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaronwinter
ref: https://github.com/bluele/gcache/pull/38/files#r127405430

Could you change to if c.size > 0 && len(c.items) >= c.size ?

simple.go Outdated
@@ -18,7 +18,11 @@ func newSimpleCache(cb *CacheBuilder) *SimpleCache {
}

func (c *SimpleCache) init() {
c.items = make(map[interface{}]*simpleItem, c.size)
if c.size == -1 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
When do you guys expect this to be merged?
This is exactly the change I've been waiting for...

Copy link
Contributor Author

@erwanor erwanor Oct 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kogan69 Hi, my bad, I sort of forgot about it. I will complete the PR by Wednesday this week.

@kogan69
Copy link

kogan69 commented Oct 1, 2017 via email

@erwanor
Copy link
Contributor Author

erwanor commented Oct 5, 2017

@kogan69 I fixed the PR. Let me add a test suite for it and then it should be good to go for review and merge.

@erwanor erwanor changed the title Unbounded SimpleCache - WIP Extend SimpleCache to allow for unbounded growth Oct 5, 2017
@erwanor
Copy link
Contributor Author

erwanor commented Oct 5, 2017

Added the test case, I'd like to add another to test for SetWithExpire but turns out the fake clock object we introduced is only package-exposed. I'll do another PR once we get that figured out.

This one is ready for review @bluele

@bluele
Copy link
Owner

bluele commented Oct 10, 2017

Thanks, @aaronwinter !
LGTM.

@bluele bluele merged commit 4726142 into bluele:master Oct 10, 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

Successfully merging this pull request may close these issues.

3 participants