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

Generic go-cache #149

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Generic go-cache #149

wants to merge 6 commits into from

Conversation

sschiz
Copy link

@sschiz sschiz commented Mar 18, 2022

I have rewritten go-cache to generic one.
Originally Cache has incrementing and decrementing methods, but any constraint doesn't allow types to be added and subtracted. I decided to implement the original cache using any restriction, while removing these methods, and also to implement extended Cache named OrderedCache that has incrementing method.
I didn't really touch the sharded cache. I made the cached value comparable. The key wasn't touched. Internally the sharded cache has OrderedCache.
Last but not least, I initiated the go.mod file because a dependency of golang.org/x/exp was required.

@muety
Copy link

muety commented Mar 20, 2022

I'd love to have this merged soon! 🙌

@arp242
Copy link

arp242 commented Mar 26, 2022

This package hasn't seen any updates for a number of years, so it seems unmaintained.

I'll probably add this, or a version of this, to the fork I use. This is an incompatible change though, so it needs to be as a v2.

@muety
Copy link

muety commented Mar 26, 2022

Keep me posted once version 2 is out, then I'll probably switch to zcache.

@sschiz
Copy link
Author

sschiz commented Mar 26, 2022

@arp242 If there's anything I can do to help, let me know.

@arp242
Copy link

arp242 commented Mar 27, 2022

Working on adding this, as I figured it would be a good project to get up to speed with generics. I wonder if there's a more convenient way to do this?

var zeroValue V
return zeroValue, false

It's just a bit of needless indirection; I'd rather do something like return V{}, false.

For the time being I added:

func (c *cache[K, V]) zero() V {
    var zeroValue V
    return zeroValue
}

So you can do return c.zero(), false. But meh.

@sschiz
Copy link
Author

sschiz commented Mar 27, 2022

You could create function like this instead of method
func zero(v V) V

@arp242
Copy link

arp242 commented Mar 27, 2022

I put a v2 branch over here, with some other incompatible changes I made last year too: https://github.com/arp242/zcache/tree/v2

I'm not entirely convinced about using a OrderedCache for Increment and Decrement operations; I removed these methods for now (still work-in-progress). Need to look a bit to alternatives.

@arp242
Copy link

arp242 commented Mar 28, 2022

I think I'll leave out Increment and Decrement; the Modify that I added works for this as well, and the performance is roughly equal:

cache := zcache.New[string, int](zcache.DefaultExpiration, 0)
cache.Set("one", 1)

newValue, err := cache.Modify("one", func(v int) int { return v + 2 })

It's a bit more typing, bit I think that's okay.

Any feedback @muety or @sschiz? I migrated my own application to v2 and it all seems to work grand, but a second pair of eyes never hurts.

@sschiz
Copy link
Author

sschiz commented Mar 28, 2022

@arp242 what is the difference between these:

    1. Get
    2. Do changes
    3. Set
    1. Modify with function

In fact, it's the same thing, and Modify method is boilerplate.
I think, you should leave modifications of value to user, because in development it is unlikely that anyone will use such a thing.
The purpose of Increment or Decrement is shortcutting like ++ and --. Modify is useless because it's shortcutting nothing.

@arp242
Copy link

arp242 commented Mar 28, 2022

The advantage of Modify() is that it's thread-safe; if you have multiple goroutines working on the same key, then in-between the "get" and "set" some other goroutine can also increment it, and the final value will be wrong. So you end up with:

goroutine 1: get value 1
goroutine 2: get value 1
goroutine 1: add 1 to value
goroutine 2: add 1 to value
goroutine 1: set value to 2
goroutine 2: set value to 2

But what really should happen is:

goroutine 1: get value 1
goroutine 1: add 1 to value
goroutine 1: set value to 2

goroutine 2: get value 2
goroutine 2: add 1 to value
goroutine 2: set value to 3

Because Modify() locks from "get value" to "set value" you'll get the correct behaviour: the second goroutine will block until the first one is done, and will use the correct value.

This is/was also the advantage of having Increment.

@sschiz
Copy link
Author

sschiz commented Mar 28, 2022

Initially go-cache is not thread-safe like built-in map. It's not a problem. Anyway there's sync.Mutex.

@arp242
Copy link

arp242 commented Mar 28, 2022

I'm not sure if I follow? All operations should be thread-safe (in go-cache, and zcache); this is why the cache object already has a mutex. Much of the value of this comes from being a thread-safe map implementation (and the other value is the cache expiration).

@sschiz
Copy link
Author

sschiz commented Mar 28, 2022

I'm sorry. It was misunderstanding.
You're right. Modify is a great agreement.

@arp242
Copy link

arp242 commented Mar 28, 2022

Haha, okay; no worries! I updated the documentation a bit as a result because I realized it wasn't all that clear, so it was useful regardless :-)

@willboland
Copy link

Here you go: https://github.com/willboland/simcache

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.

None yet

5 participants