From 2ca111b6b91c578c8a2b8e610471e582b6cf2c6b Mon Sep 17 00:00:00 2001 From: Alkis Evlogimenos Date: Mon, 16 Oct 2017 14:05:16 +0200 Subject: [PATCH] Improve the performance of binary_search by reducing the number of 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) ``` --- src/libcore/benches/lib.rs | 2 +- src/libcore/benches/slice.rs | 67 ++++++++++++++++++++++++++++++++++++ src/libcore/slice/mod.rs | 34 +++++++++--------- src/libcore/tests/slice.rs | 63 ++++++++++++++++++++++++++------- 4 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 src/libcore/benches/slice.rs diff --git a/src/libcore/benches/lib.rs b/src/libcore/benches/lib.rs index d2db329da7999..201064e823b1e 100644 --- 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 new file mode 100644 index 0000000000000..b2fc74544f1df --- /dev/null +++ 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 or the MIT license +// , 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(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::>(); + 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 index 5039bef631e51..e5b4df9684540 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -394,23 +394,25 @@ impl SliceExt for [T] { fn binary_search_by<'a, F>(&'a self, mut f: F) -> Result 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 index 8c31d2e83d352..7835080db1d45 100644 --- 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]