-
Notifications
You must be signed in to change notification settings - Fork 346
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
[Issue 1021][client] FIX: use maphash instead of crypto/sha256 for hash function of hashmap in Schema.hash() #1022
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I saw the seed is random. so theres a method needed to fix the seed. |
Added a static seed with its ready from my side. |
e6fbaa3
to
2615403
Compare
…p in Schema.hash() - replace sha256 hash function with hash/maphash - use uint64 (expected from maphash) as schema cache hashmap key - init static seed as it is random generated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
To double-check the patch - it changes local access to the schema map, without any changes to the actual schema go through the wire, right?
yes, no changes to the schema itself, just the hash method to get a key for the local hashmap struct to update or access the cached schema. |
Merging... |
Thank you! |
use golang/hash/maphash instead of crypto/sha256 for hash function of hashmap in Schema.hash()
as the hash function is used for determining the hash key, it is safe to use a non-cryptographic function.
Fixes #1021
Motivation
Improve performance of the SchemaCache by replacing the cryptographic hash function.
Modifications
Replaced the crypto/sha256 with golang/hash/maphash function.
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation