-
Notifications
You must be signed in to change notification settings - Fork 601
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: Support uint64 keys without converting to string then back again #380
Comments
Hey, could post a sample code. I don't get the question? |
@janisz Sure thing. We're already hashing package main
import (
"context"
"fmt"
"strconv"
"time"
"github.com/allegro/bigcache/v3"
pb "github.com/myco/protos/foobarbaz"
)
func main() {
cache := NewBigCache()
obj := getObject()
key := hashObject(obj)
// Because cache.Set() takes a string, now I have to convert
cache.Set(strconv.FormatUint(key, 10), obj)
}
func hashObject(obj *pb.Object) uint64 {
// Internal xxh3 sync.Pool for buffer reuse
hasher := Hasher()
defer ReleaseHasher(hasher)
hashString(hasher, obj.Foo)
hashBar(hasher, obj.Bar)
return hasher.Sum64()
}
func hashBar(hasher Hasher, bar *pb.Bar) {
// Long switch-statement to deal with one-of values
}
func NewBigCache() *bigcache.BigCache {
cache, err := bigcache.New(context.TODO(), bigcache.Config{
Shards: 1024,
HardMaxCacheSize: 1024,
LifeWindow: time.Minute * 5,
CleanWindow: time.Second * 150,
MaxEntriesInWindow: 150000,
MaxEntrySize: 2048,
// Relevant bit
Hasher: dumbHasher{},
})
if err != nil {
panic(err)
}
return cache
}
type dumbHasher struct{}
// Sum64 assumes that 's' is a string-formatted uint64 and panics if not
func (p dumbHasher) Sum64(s string) uint64 {
u, err := strconv.ParseUint(s, 10, 64)
if err != nil {
panic(fmt.Sprintf("invalid argument: cannot convert %q to a uint64", s))
}
return u
} |
Thank you. Now I understand. We use string as a key just for as it was our historical use case. Now, Go supports generics so maybe we can make it more liberal in what we accept (e.g. accept |
But it's Either way, @janisz, it sounds great to me. :) |
Correct and we should keep it that way. So the only change will be in public methods and hasher. So hasher need to convert I'm not sure if we then can provide defulat hasher easliy. Maybe we need to cut v4 and have |
Or have a different package so that the name isn't compromised. We did this with
|
Hey folks!
I have a use-case where we already use
xxh3
for hashing various things, including the key for a previous cache's key. We're usingbigcache
now, but it's a bit awkward doingstrconv.FormatUint
and then configuring a dummyHasher
to dostrconv.ParseUint
.Is there a reason this is omitted?
The text was updated successfully, but these errors were encountered: