-
Notifications
You must be signed in to change notification settings - Fork 8
feat(bloomfilter): add salt #742
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
base: main
Are you sure you want to change the base?
Conversation
559e85f
to
08445e8
Compare
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.
This is looking good so far.
A few minor suggestions:
- arg order
- do we really need a class for hashing with salt, it's just used internally
- where the Salt type lives
And then we can move on to the uses in lsm-tree.
update s (hashSalt64 salt x) | ||
|
||
instance (Hashable a, Hashable b) => Hashable (a, b) where | ||
hashSalt64 salt (x, y) = incrementalHash salt $ \s -> do | ||
update s (hash64 x) | ||
update s (hash64 y) | ||
update s (hashSalt64 salt x) | ||
update s (hashSalt64 salt y) |
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.
You don't need to change these ones that use incrementalHash
, because incrementalHash
already uses the salt to initialise the (mutable) hash state.
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.
Yes, I was wondering about that.
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.
Does it hurt, though?
a0d8b27
to
caeb000
Compare
4752a95
to
35e8db0
Compare
2fc1622
to
f13d95b
Compare
This PR adds salt to the Bloom filter API.
I expect the CI to break, since I have not modified the usage of the bloomfilter package in lsm-tree.