Improve the performance of binary_search by reducing the number of

Alkis Evlogimenos · alkis · commit 2ca111b6b91c · 2017-11-11T16:00:26.000+01:00
unpredictable conditional branches in the loop. In addition improve the
benchmarks to test performance in l1, l2 and l3 caches on sorted arrays
with or without dups.

Before:

```
test slice::binary_search_l1                               ... bench:  48 ns/iter (+/- 1)
test slice::binary_search_l2                               ... bench:  63 ns/iter (+/- 0)
test slice::binary_search_l3                               ... bench: 152 ns/iter (+/- 12)
test slice::binary_search_l1_with_dups                     ... bench:  36 ns/iter (+/- 0)
test slice::binary_search_l2_with_dups                     ... bench:  64 ns/iter (+/- 1)
test slice::binary_search_l3_with_dups                     ... bench: 153 ns/iter (+/- 6)
```

After:

```
test slice::binary_search_l1                               ... bench:  15 ns/iter (+/- 0)
test slice::binary_search_l2                               ... bench:  23 ns/iter (+/- 0)
test slice::binary_search_l3                               ... bench: 100 ns/iter (+/- 17)
test slice::binary_search_l1_with_dups                     ... bench:  15 ns/iter (+/- 0)
test slice::binary_search_l2_with_dups                     ... bench:  23 ns/iter (+/- 0)
test slice::binary_search_l3_with_dups                     ... bench:  98 ns/iter (+/- 14)
```
diff --git a/src/libcore/benches/lib.rs b/src/libcore/benches/lib.rs
@@ -20,6 +20,6 @@ extern crate test;
 mod any;
 mod hash;
 mod iter;
-mod mem;
 mod num;
 mod ops;
+mod slice;
diff --git a/src/libcore/benches/slice.rs b/src/libcore/benches/slice.rs
@@ -0,0 +1,67 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use test::black_box;
+use test::Bencher;
+
+enum Cache {
+    L1,
+    L2,
+    L3,
+}
+
+fn binary_search<F>(b: &mut Bencher, cache: Cache, mapper: F)
+    where F: Fn(usize) -> usize
+{
+    let size = match cache {
+        Cache::L1 => 1000, // 8kb
+        Cache::L2 => 10_000, // 80kb
+        Cache::L3 => 1_000_000, // 8Mb
+    };
+    let v = (0..size).map(&mapper).collect::<Vec<_>>();
+    let mut r = 0usize;
+    b.iter(move || {
+        // LCG constants from https://en.wikipedia.org/wiki/Numerical_Recipes.
+        r = r.wrapping_mul(1664525).wrapping_add(1013904223);
+        // Lookup the whole range to get 50% hits and 50% misses.
+        let i = mapper(r % size);
+        black_box(v.binary_search(&i).is_ok());
+    })
+}
+
+#[bench]
+fn binary_search_l1(b: &mut Bencher) {
+    binary_search(b, Cache::L1, |i| i * 2);
+}
+
+#[bench]
+fn binary_search_l2(b: &mut Bencher) {
+    binary_search(b, Cache::L2, |i| i * 2);
+}
+
+#[bench]
+fn binary_search_l3(b: &mut Bencher) {
+    binary_search(b, Cache::L3, |i| i * 2);
+}
+
+#[bench]
+fn binary_search_l1_with_dups(b: &mut Bencher) {
+    binary_search(b, Cache::L1, |i| i / 16 * 16);
+}
+
+#[bench]
+fn binary_search_l2_with_dups(b: &mut Bencher) {
+    binary_search(b, Cache::L2, |i| i / 16 * 16);
+}
+
+#[bench]
+fn binary_search_l3_with_dups(b: &mut Bencher) {
+    binary_search(b, Cache::L3, |i| i / 16 * 16);
+}
diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs
@@ -394,23 +394,25 @@ impl<T> SliceExt for [T] {
     fn binary_search_by<'a, F>(&'a self, mut f: F) -> Result<usize, usize>
         where F: FnMut(&'a T) -> Ordering
     {
-        let mut base = 0usize;
-        let mut s = self;
-
-        loop {
-            let (head, tail) = s.split_at(s.len() >> 1);
-            if tail.is_empty() {
-                return Err(base)
-            }
-            match f(&tail[0]) {
-                Less => {
-                    base += head.len() + 1;
-                    s = &tail[1..];
-                }
-                Greater => s = head,
-                Equal => return Ok(base + head.len()),
-            }
+        let s = self;
+        let mut size = s.len();
+        if size == 0 {
+            return Err(0);
         }
+        let mut base = 0usize;
+        while size > 1 {
+            let half = size / 2;
+            let mid = base + half;
+            // mid is always in [0, size).
+            // mid >= 0: by definition
+            // mid < size: mid = size / 2 + size / 4 + size / 8 ...
+            let cmp = f(unsafe { s.get_unchecked(mid) });
+            base = if cmp == Greater { base } else { mid };
+            size -= half;
+        }
+        // base is always in [0, size) because base <= mid.
+        let cmp = f(unsafe { s.get_unchecked(base) });
+        if cmp == Equal { Ok(base) } else { Err(base + (cmp == Less) as usize) }
     }
 
     #[inline]
diff --git a/src/libcore/tests/slice.rs b/src/libcore/tests/slice.rs
@@ -15,22 +15,59 @@ use rand::{Rng, XorShiftRng};
 
 #[test]
 fn test_binary_search() {
+    let b: [i32; 0] = [];
+    assert_eq!(b.binary_search(&5), Err(0));
+
+    let b = [4];
+    assert_eq!(b.binary_search(&3), Err(0));
+    assert_eq!(b.binary_search(&4), Ok(0));
+    assert_eq!(b.binary_search(&5), Err(1));
+
     let b = [1, 2, 4, 6, 8, 9];
-    assert!(b.binary_search_by(|v| v.cmp(&6)) == Ok(3));
-    assert!(b.binary_search_by(|v| v.cmp(&5)) == Err(3));
-    let b = [1, 2, 4, 6, 7, 8, 9];
-    assert!(b.binary_search_by(|v| v.cmp(&6)) == Ok(3));
-    assert!(b.binary_search_by(|v| v.cmp(&5)) == Err(3));
-    let b = [1, 2, 4, 6, 8, 9];
-    assert!(b.binary_search_by(|v| v.cmp(&8)) == Ok(4));
-    assert!(b.binary_search_by(|v| v.cmp(&7)) == Err(4));
+    assert_eq!(b.binary_search(&5), Err(3));
+    assert_eq!(b.binary_search(&6), Ok(3));
+    assert_eq!(b.binary_search(&7), Err(4));
+    assert_eq!(b.binary_search(&8), Ok(4));
+
+    let b = [1, 2, 4, 5, 6, 8];
+    assert_eq!(b.binary_search(&9), Err(6));
+
     let b = [1, 2, 4, 6, 7, 8, 9];
-    assert!(b.binary_search_by(|v| v.cmp(&8)) == Ok(5));
+    assert_eq!(b.binary_search(&6), Ok(3));
+    assert_eq!(b.binary_search(&5), Err(3));
+    assert_eq!(b.binary_search(&8), Ok(5));
+
     let b = [1, 2, 4, 5, 6, 8, 9];
-    assert!(b.binary_search_by(|v| v.cmp(&7)) == Err(5));
-    assert!(b.binary_search_by(|v| v.cmp(&0)) == Err(0));
-    let b = [1, 2, 4, 5, 6, 8];
-    assert!(b.binary_search_by(|v| v.cmp(&9)) == Err(6));
+    assert_eq!(b.binary_search(&7), Err(5));
+    assert_eq!(b.binary_search(&0), Err(0));
+
+    let b = [1, 3, 3, 3, 7];
+    assert_eq!(b.binary_search(&0), Err(0));
+    assert_eq!(b.binary_search(&1), Ok(0));
+    assert_eq!(b.binary_search(&2), Err(1));
+    assert!(match b.binary_search(&3) { Ok(1...3) => true, _ => false });
+    assert!(match b.binary_search(&3) { Ok(1...3) => true, _ => false });
+    assert_eq!(b.binary_search(&4), Err(4));
+    assert_eq!(b.binary_search(&5), Err(4));
+    assert_eq!(b.binary_search(&6), Err(4));
+    assert_eq!(b.binary_search(&7), Ok(4));
+    assert_eq!(b.binary_search(&8), Err(5));
+}
+
+#[test]
+// Test implementation specific behavior when finding equivalent elements.
+// It is ok to break this test but when you do a crater run is highly advisable.
+fn test_binary_search_implementation_details() {
+    let b = [1, 1, 2, 2, 3, 3, 3];
+    assert_eq!(b.binary_search(&1), Ok(1));
+    assert_eq!(b.binary_search(&2), Ok(3));
+    assert_eq!(b.binary_search(&3), Ok(6));
+    let b = [1, 1, 1, 1, 1, 3, 3, 3, 3];
+    assert_eq!(b.binary_search(&1), Ok(4));
+    assert_eq!(b.binary_search(&3), Ok(8));
+    let b = [1, 1, 1, 1, 3, 3, 3, 3, 3];
+    assert_eq!(b.binary_search(&1), Ok(3));
+    assert_eq!(b.binary_search(&3), Ok(8));
 }
 
 #[test]
