refactor: implement MurmurHash3 internally#36
Conversation
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
| self.finish128().0 | ||
| } | ||
|
|
||
| fn write(&mut self, mut bytes: &[u8]) { |
There was a problem hiding this comment.
No need unsafe as mur3.
| /// The MurmurHash3 is a fast, non-cryptographic, 128-bit hash function that has | ||
| /// excellent avalanche and 2-way bit independence properties. | ||
| #[derive(Debug)] | ||
| pub struct MurmurHash3X64128 { |
There was a problem hiding this comment.
Naming is still hard. Also we don't need a murmurhash3_x64_128 method based on our current usage.
There was a problem hiding this comment.
Let's be explicit here. Name = Spec of the algorithm. Maybe let's add some small comment saying why we are specifically using the x64 variant.
|
|
||
| fn murmurhash3_x64_128(key: &[u8], seed: u64) -> (u64, u64) { | ||
| let mut hasher = MurmurHash3X64128::with_seed(seed); | ||
| hasher.write(key); |
There was a problem hiding this comment.
Note that this is different from key.hash(&mut hasher), which will hash the slice length first.
notfilippo
left a comment
There was a problem hiding this comment.
Looks good to me. The algorithm looks correct (I've tried to match it with some existing implementation and failed to find any discrepancies).
I'm not intimately familiar with this specific hashing function so I might have missed something, but I'm also open to correct it later.
On a separate note, re: removing dependencies. What do we feel about byteorder?
|
@freakyzoidberg / @leerho feel free to merge this :) |
I think we can have our own reiml perhaps in Let me track this task and try to implement it this week. See #41. |
This closes #34 .
cc @notfilippo @Xuanwo @leerho
I wonder whether this should be a public API, and where is the right place to put the
DEFAULT_SEED.Besides, please comment in where we can add more code comments for comprehension.