Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faster midpoint calculation for binary_search_by #128236

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2788,19 +2788,30 @@ impl<T> [T] {
F: FnMut(&'a T) -> Ordering,
{
// INVARIANTS:
// - 0 <= left <= left + size = right <= self.len()
// - 0 <= left <= mid <= right <= self.len()
// - f returns Less for everything in self[..left]
// - f returns Greater for everything in self[right..]
let mut size = self.len();
let mut left = 0;
let mut right = size;
while left < right {
let mid = left + size / 2;

// SAFETY: the while condition means `size` is strictly positive, so
// `size/2 < size`. Thus `left + size/2 < left + size`, which
// coupled with the `left + size <= self.len()` invariant means
// we have `left + size/2 < self.len()`, and this is in-bounds.
let mut right = self.len();
while left != right {
// These 2 expressions are arithmetically equivalent, except if
// `left + right` overflows. This can't happen for non-ZST slices
// since such allocations cannot exceed `isize::MAX` bytes.
let mid = if T::IS_ZST {
let size = right - left;
left + size / 2
} else {
(left + right) / 2
};

// SAFETY:
// - the calculation of `mid` guarantees `left <= mid <= right`.
// - `left` only gets bigger and can't get larger than `right`.
// - `right` only gets smaller and can't get smaller than `left`.
// - `left` and `right` can't move "past" each other because only
// one of them is updated on each loop iteration.
// - This means that `left`, `mid` and `right` are always between
// `0` and `self.len()`, which means the access is in-bounds.
let cmp = f(unsafe { self.get_unchecked(mid) });

// This control flow produces conditional moves, which results in
Expand All @@ -2814,8 +2825,6 @@ impl<T> [T] {
unsafe { hint::assert_unchecked(mid < self.len()) };
return Ok(mid);
}

size = right - left;
}

// SAFETY: directly true from the overall invariant.
Expand Down
Loading