Skip to content

Commit d000d5f

Browse files
committed
Rewrite binary search implementation
This restores the original binary search implementation from rust-lang#45333 which has the nice property of having a loop count that only depends on the size of the slice. This, along with explicit conditional moves from rust-lang#128250, means that the entire binary search loop can be perfectly predicted by the branch predictor. Additionally, LLVM is able to unroll the loop when the slice length is known at compile-time. This results in a very compact code sequence of 3-4 instructions per binary search step and zero branches. Fixes rust-lang#53823
1 parent 472058a commit d000d5f

File tree

1 file changed

+31
-26
lines changed

1 file changed

+31
-26
lines changed

library/core/src/slice/mod.rs

+31-26
Original file line numberDiff line numberDiff line change
@@ -2787,40 +2787,45 @@ impl<T> [T] {
27872787
where
27882788
F: FnMut(&'a T) -> Ordering,
27892789
{
2790-
// INVARIANTS:
2791-
// - 0 <= left <= left + size = right <= self.len()
2792-
// - f returns Less for everything in self[..left]
2793-
// - f returns Greater for everything in self[right..]
27942790
let mut size = self.len();
2795-
let mut left = 0;
2796-
let mut right = size;
2797-
while left < right {
2798-
let mid = left + size / 2;
2799-
2800-
// SAFETY: the while condition means `size` is strictly positive, so
2801-
// `size/2 < size`. Thus `left + size/2 < left + size`, which
2802-
// coupled with the `left + size <= self.len()` invariant means
2803-
// we have `left + size/2 < self.len()`, and this is in-bounds.
2791+
if size == 0 {
2792+
return Err(0);
2793+
}
2794+
let mut base = 0usize;
2795+
2796+
// This loop intentionally doesn't have an early exit if the comparison
2797+
// returns Equal. We want the number of loop iterations to depend *only*
2798+
// on the size of the input slice so that the CPU can reliably predict
2799+
// the loop count.
2800+
while size > 1 {
2801+
let half = size / 2;
2802+
let mid = base + half;
2803+
2804+
// SAFETY: the call is made safe by the following inconstants:
2805+
// - `mid >= 0`: by definition
2806+
// - `mid < size`: `mid = size / 2 + size / 4 + size / 8 ...`
28042807
let cmp = f(unsafe { self.get_unchecked(mid) });
28052808

28062809
// Binary search interacts poorly with branch prediction, so force
28072810
// the compiler to use conditional moves if supported by the target
28082811
// architecture.
2809-
left = select_unpredictable(cmp == Less, mid + 1, left);
2810-
right = select_unpredictable(cmp == Greater, mid, right);
2811-
if cmp == Equal {
2812-
// SAFETY: same as the `get_unchecked` above
2813-
unsafe { hint::assert_unchecked(mid < self.len()) };
2814-
return Ok(mid);
2815-
}
2816-
2817-
size = right - left;
2812+
base = select_unpredictable(cmp == Greater, base, mid);
2813+
size -= half;
28182814
}
28192815

2820-
// SAFETY: directly true from the overall invariant.
2821-
// Note that this is `<=`, unlike the assume in the `Ok` path.
2822-
unsafe { hint::assert_unchecked(left <= self.len()) };
2823-
Err(left)
2816+
// SAFETY: base is always in [0, size) because base <= mid.
2817+
let cmp = f(unsafe { self.get_unchecked(base) });
2818+
if cmp == Equal {
2819+
// SAFETY: same as the `get_unchecked` above.
2820+
unsafe { hint::assert_unchecked(base < self.len()) };
2821+
Ok(base)
2822+
} else {
2823+
let result = base + (cmp == Less) as usize;
2824+
// SAFETY: same as the `get_unchecked` above.
2825+
// Note that this is `<=`, unlike the assume in the `Ok` path.
2826+
unsafe { hint::assert_unchecked(result <= self.len()) };
2827+
Err(result)
2828+
}
28242829
}
28252830

28262831
/// Binary searches this slice with a key extraction function.

0 commit comments

Comments
 (0)