grouping_map: Allow to create GroupingMap with a custom BuildHasher#1057
grouping_map: Allow to create GroupingMap with a custom BuildHasher#1057vadorovsky wants to merge 2 commits intorust-itertools:masterfrom
GroupingMap with a custom BuildHasher#1057Conversation
Currently, `GroupingMap` can create a `HashMap` with the default `RandomState`. For some kind of keys, using different `BuildHasher` implementation can result in better performance. Add a possibility to create a `GroupingMap` with a custom hasher builder, which then produces a `HashMap` with it.
|
Thanks for this. Closing this for now, because - if we do this - we won't restrict ourselves to custom |
|
We've been blocked for ages on the problem of abstracting over container type. Abstracting over hasher is a much simpler problem and, with type defaults, perhaps even a non-breaking change. I think we should consider this PR. |
|
Re-opening as per @jswrenn's comment. |
phimuemue
left a comment
phimuemue
left a comment
There was a problem hiding this comment.
Ok, this looks all reasonable to me.
The only nit I have is that I'd ideally like to have less builder functions. Maybe all ::new-functions could call the respective ::with_hasher-functions, and we could get rid of grouping_map::new?
@jswrenn I'm no expert in the not-a-breaking-change business. I guess this one could be ok, because we "only" add a S: BuildHasher here and there, but existing callers may be unaffected. Could you offer a second look at this, and if you, too, say, this is fine, I'd merge it.
| { | ||
| GroupingMap { iter } | ||
| let hash_builder = RandomState::new(); | ||
| GroupingMap { iter, hash_builder } |
There was a problem hiding this comment.
It's a nit, but please forward this to with_hasher(iter, RandomState::new()).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1057 +/- ##
==========================================
- Coverage 94.38% 93.35% -1.03%
==========================================
Files 48 50 +2
Lines 6665 6355 -310
==========================================
- Hits 6291 5933 -358
- Misses 374 422 +48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Currently,
GroupingMapcan create aHashMapwith the defaultRandomState.For some kind of keys, using different
BuildHasherimplementation can result in better performance. Add a possibility to create aGroupingMapwith a custom hasher builder, which then produces aHashMapwith it.