-
Notifications
You must be signed in to change notification settings - Fork 17
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
StringKey for cache key? #35
Comments
As mentioned in #33, StringKey is a temporary solution for structs containing strings. Its purpose is to address the lack of an official Go mechanism for customizing hashing. If Go introduces such a solution in the future, StringKey will likely be deprecated and removed. On the other hand, loader is just a function. You can use parts of the input parameters to construct a new key within the function itself. Additionally, since loader is generic and the input parameter type matches the key type, changing it while maintaining backwards compatibility might be challenging. |
Thanks for the response. Agreed that ideally go should have a mechanism for customizing hashing. I'm not sure I understand how the loader could customize the key? I think the generic FWIW the change I quickly spiked doesn't look like it needed to break backwards compatibility, as it is just changing the internal hashmap implementation. It does require passing the stringKey around a bit more, though I wasn't able to detect any real changes running the benchmark. |
I understand your point. Currently, the string key function is only used to select a shard, and the Go HashMap within the shard still relies on the original key struct. Your patch seems functional, but directly converting the key to a string might be a littile over-engineered . In languages like Rust, it's common practice to implement Hash and Equal traits for your struct, allowing the HashMap to utilize them directly. This way, the key within the map remains the original struct, not a newly generated string. Additionally, consider the Range API. It iterates through key-value pairs, and the key type within this API will change. Potentially causing unexpected behavior. |
With #33 adding
StringKey()
for sharding, would it make sense to have a way to use the value ofStringKey()
for cache key comparison? I was attempting to use a loader but don't want all of the arguments passed to the loader as part of the cache key.The text was updated successfully, but these errors were encountered: