Make bloom filters simpler #1021
Conversation
| } | ||
|
|
||
| // fmt.Println(f) |
|
if we are going to go with a single hash function, i prefer not to use fnv, it doesnt have very good properties for this application (as described here: http://bretmulvey.com/hash/6.html) |
|
there probably are some fast hash function impls in the golang.org/x packages. google's been leading the development of fastest non-crypto hashes. Anyone recall why we needed multiple hash functions in the first place? @whyrusleeping ( i mean flexibility is always good if it can be afforded, just wondering if there was a specific reason we needed it ) |
|
my only reason for multiple hash functions was the flexibility to allow the user to use whatever they wanted |
|
yeahh that's nice. :/ your call @whyrusleeping |
|
eh, if we want it later its an easy change |
|
Pushed a fixed up version, using jenkins hash for now, as recommended by http://bretmulvey.com/hash/7.html (thx @whyrusleeping) Also added test coverage for merging filters together. |
|
if you are ok with random-ish github dependencies that is (MIT license), otherwise i could look around. |
These did not work before, and had some unnecessary complexity. Now the filters use only one hashing function, no bignum arithmetic, and gets the additional bit positions by repeatedly hashing the result of prior hash. Since we're not concerned about crypto hashing here, this should be a win. External interfaces unchanged.
| "ImportPath": "github.com/whyrusleeping/go-metrics", | ||
| "Rev": "1cd8009604ec2238b5a71305a0ecd974066e0e16" | ||
| }, | ||
| { |
|
LGTM! |
|
(i'll merge when tests green) |
|
@krl 👍 Looks good :) |
These did not work before, and had some unnecessary complexity.
Now the filters use only one hashing function, no bignum arithmetic, and gets the additional bit positions by repeatedly hashing the result of prior hash.
Since we're not concerned about crypto hashing here, this should be a win.
External interfaces unchanged.