arp242
Multi Get and Multi Set
Is there any reason in particular you used two slices in MultiGet() rather than a slice with a struct? It seems to me that's quite a bit more convenient, and this can also work for setting multiple keys:
func (c *cache[K, V]) MultiSet(vals ...struct {
k K
v V
}) {
c.mu.Lock()
for _, v := range vals {
c.set(v.k, v.v, c.defaultExpiration)
}
c.mu.Unlock()
}
i personaly found the resuling code, that uses the function more readable. But this is personal preference. I can rework this with structs if you like
It seems to me that juggling around two variables for one thing is rather awkward. Then again I also don't think my version is super-great either (it was just the quickest to write – I mostly wanted to compare performance to see if it's worth it after our Twitter chat).
Since I really want to avoid introducing incompatible changes in the future I'd like to get this right – I need to play around and think a bit to see what works, so let me get back to you.
Also: there's quite a few other operations where it makes sense to operate on more than one key. Why do Get() and Set() have "Multi versions" but not Add() or Delete() or Touch() or GetWithExpire() or any of the others?
Adding Multi-variants for each one would be a bit much, but I'm wonder if there's nice way to make a more generic API that extends to more operations – for example maybe something like (and just thinking out loud here):
// Returns []struct{..} or two slices.
vals := mycache.Multi(key1, key2).Get()
mycache.Multi(key1, key2).Delete()
mycache.Multi(key1, key2).Set("val1", "val2")
mycache.Multi(key1, key2).SetWithExpire(expire, "val1", "val2")
mycache.Multi(key1, key2).Rename("newname1", "newname2")
Implementation is more work, but I don't mind doing the work on that. Getting the API right is what I'm most concerned with.
That might be a good idea. I tested 2 return types: ([]V, []bool), []struct{v V, exists bool} in benchmarks. I modified your benchmark. The return the get-method is iterated over and if a value doesn't exists time.Sleep(time.Nanosecond) is used to simulate loading the value and the value is set to "".
goos: windows
goarch: amd64
pkg: zgo.at/zcache/v2
cpu: AMD Ryzen 7 5700U with Radeon Graphics
BenchmarkGetMulti/get
BenchmarkGetMulti/get-16 2 6196558400 ns/op
BenchmarkGetMulti/mget-16 2 6155529550 ns/op
BenchmarkGetMulti/mgetStruct-16 4 2641001275 ns/op
PASS
As you can see, my idea with 2 slices completely ruins the performance, therefore the struct version should be the right return type.
Hm, I get quite different benchmark results?
BenchmarkGetMulti/get-8 62580 17229 ns/op 0 B/op 0 allocs/op
BenchmarkGetMulti/mget-8 112527 12081 ns/op 16384 B/op 1 allocs/op
BenchmarkGetMulti/mget2-8 114786 10299 ns/op 10368 B/op 2 allocs/op
I put the benchmark in https://github.com/arp242/zcache/tree/mget
My benchmark is a lot simpler though.
Your Benchmark only tests the method and misses the access of the method retrieved. I find this important since you normally iterate through the information and load information the cache missed. My Tests simulates the load through time.sleep. Depending on the result time, the access of the data differs. My results might also be influenced by my cpu since it's a 15w Laptop CPU, which ran on battery.
I will further test tomorrow
It's fine, don't bother – it's not hugely important.
This seems to work reasonably well: https://github.com/arp242/zcache/pull/5 – what do you think?
Keyset.Set() is up to ~60% faster, and Keyset.Get() up to ~40%.
Need to finish with some extra tests and documentation, and maybe also some extra methods.
Also not entirely sure about the "Keyset" name; but I couldn't think of anything better.