Iterator::{min,max}(_by_key) should use overridden min/max/lt - #160203
Iterator::{min,max}(_by_key) should use overridden min/max/lt#160203scottmcm wants to merge 1 commit into
Iterator::{min,max}(_by_key) should use overridden min/max/lt#160203Conversation
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
`Iterator::{min,max}(_by_key)` should use overridden `min`/`max`/`lt`
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (e9abde2): comparison URL. Overall result: ❌✅ regressions and improvements - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.1%, secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.1%, secondary -0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 490.333s -> 489.5s (-0.17%) |
Two related changes to provided Iterator implementations:
Iterator::minandIterator::maxare currently implemented viamin_byandmax_by, which means they don't useOrd::{min,max}despite those being overridable to do something more efficient. Move these to just being.reduce(Ord::min)and.reduce(Ord::max)to take advantage of potential overrides.Iterator::min_by_keyandIterator::max_by_keyare implemented by mapping to a tuple then usingmin_by/max_bywith a comparator that only looks at one field in the tuple. That means they end up doing things likea.cmp(b).is_le(), which is wasteful if there an overloaded-> boolmethod it could use instead. So rephrase these two to work as.map(…).min()/.map(…).max()by mapping to a type that's not a tuple and which can thus override more things instead of just passing aFn(…) -> Ordering.