-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Refactor binary_search_by
to use conditional moves
#117722
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Refactor the if/else checking on cmp::Ordering variants to a "branchless" reassignment of left and right. This change results in fewer branches and instructions.
r? @thomcc (rustbot has picked a reviewer for you, use r? to override) |
@bors r+ rollup=never |
☀️ Test successful - checks-actions |
Finished benchmarking commit (8abf920): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 676.001s -> 675.075s (-0.14%) |
@rustbot label: +perf-regression-triaged |
Refactor the if/else checking on
cmp::Ordering
variants to a "branchless" reassignment of left and right.This change results in fewer branches and instructions.
https://rust.godbolt.org/z/698eYffTx
I saw consistent benchmark improvements locally. Performance of worst case seems about the same, maybe slightly faster for the L3 test.
Current
This PR