Use FNV hasher #276
Use FNV hasher #276
Conversation
|
To back up the claim that it is faster for small keys: on my MacBook the benchmark I added runs at 26,115 ns/iter (+/- 2,329) on the default SipHash |
Accidentally reformatted lib.rs using rustfmt. This undoes that.
|
Hi, thanks for this thoughtful improvement. We will not add more dependencies right now; itertools has a bunch of different functionality, and for most itertools users this will be a dependency that they don't have any benefit from. For that reason, I'll decline this PR. |
| @@ -1901,7 +1902,7 @@ pub trait Itertools : Iterator { | |||
| /// assert_eq!(lookup[&3], vec![13, 33]); | |||
| /// ``` | |||
| #[cfg(feature = "use_std")] | |||
| fn into_group_map<K, V>(self) -> HashMap<K, Vec<V>> | |||
| fn into_group_map<K, V>(self) -> FnvHashMap<K, Vec<V>> | |||
There was a problem hiding this comment.
Now it won't matter in this PR, but we'd usually be mindful here - the function signature changes, so it's a breaking change.
There was a problem hiding this comment.
Thanks for the review! I completely understand the aversion to adding more dependencies. I would like to note that FnvHashMap is just a type alias, so the change would technically be from HashMap<K, Vec<V>> to HashMap<K, Vec<V>, FnvBuildHasher>, so still a signature change but the hasher is more of an implementation detail.
ghost
deleted the
The default
HashMapuses SipHash, which is great for collision resistance, but can be slow in general cases where this isn't necessary. Specifically, it is much faster for small keys. This switches to the FNV hasher (using the very smallfnvcrate) for theHashMaps in.unique()and.into_group_map(). This results in faster Iterators with very little work.