Skip to content

Faster midpoint calculation for binary_search_by #128236

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

Closed
wants to merge 1 commit into from

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Jul 26, 2024

This takes advantage of the fact that slices can only have a maximum of isize::MAX elements, which means that the midpoint can be calculated using (left + right) / 2 instead of left + (right - left) / 2.

The old calculation is kept only for ZST slices which can have up to usize::MAX elements.

Comparison with previous version: https://rust.godbolt.org/z/YccscWoqv

This takes advantage of the fact that slices can only have a maximum of
`isize::MAX` elements, which means that the midpoint can be calculated
using `(left + right) / 2` instead of `left + (right - left) / 2`.

The old calculation is kept only for ZST slices which can have up to
`usize::MAX` elements.

Comparison with previous version: https://rust.godbolt.org/z/YccscWoqv
@rustbot
Copy link
Collaborator

rustbot commented Jul 26, 2024

r? @Nilstrieb

rustbot has assigned @Nilstrieb.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 26, 2024
@DaniPopes
Copy link
Contributor

CC similar attempt #127007

@Amanieu
Copy link
Member Author

Amanieu commented Jul 26, 2024

Actually, I've been doing more tests and I get much better results by just reverting to the original implementation from #45333 + forcing LLVM to emit CMOVs.

@Amanieu Amanieu closed this Jul 26, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants