Skip to content

Commit

Permalink
Implement binary_search for collections::Vec (#836)
Browse files Browse the repository at this point in the history
* [storage] Implement binary_search

* [storage] Add binary_search testcase for missing smaller element

* [storage] use indexing instead of Vec::get

Co-authored-by: Robin Freyler <robbepop@web.de>

* [storage] Port binary_search tests from core

* [storage] Format tests

* [storage] Format tests

* Remove old comment

Co-authored-by: Robin Freyler <robbepop@web.de>

* [storage] Incorporate review feedback

* [CI] add comparator to spellcheck

Porting binary_search to the storage Vec, including the documentation found in core, led to a failure in spellcheck. Since core is considered to be correct, it is updated on spellcheck.

* incorporate review feedback

* add permalink to stdlib source

* Split long line

* fix examples

* RustFmt the doc examples

Co-authored-by: Robin Freyler <robbepop@web.de>
Co-authored-by: Hernando Castano <hernando@hcastano.com>
  • Loading branch information
3 people authored Jul 9, 2021
1 parent 1747b34 commit 36bbdec
Show file tree
Hide file tree
Showing 3 changed files with 258 additions and 4 deletions.
1 change: 1 addition & 0 deletions .config/cargo_spellcheck.dic
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ bitvector
bitwise
callee
codegen
comparator
const
crypto
cryptographic
Expand Down
176 changes: 176 additions & 0 deletions crates/storage/src/collections/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,182 @@ where
*self.len += 1;
self.elems.put(last_index, Some(value));
}

/// Binary searches this sorted vector for a given element.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
/// one of the matches could be returned. If the value is not found then
/// [`Result::Err`] is returned, containing the index where a matching
/// element could be inserted while maintaining sorted order.
///
/// See also [`binary_search_by`], [`binary_search_by_key`].
///
/// [`binary_search_by`]: Vec::binary_search_by
/// [`binary_search_by_key`]: Vec::binary_search_by_key
///
/// # Examples
///
/// Looks up a series of four elements. The first is found, with a
/// uniquely determined position; the second and third are not
/// found; the fourth could match any position in `[1, 4]`.
///
/// ```
/// use ink_storage::Vec as StorageVec;
///
/// let s: StorageVec<i32> = [0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55]
/// .into_iter()
/// .copied()
/// .collect();
///
/// assert_eq!(s.binary_search(&13), Ok(9));
/// assert_eq!(s.binary_search(&4), Err(7));
/// assert_eq!(s.binary_search(&100), Err(13));
/// let r = s.binary_search(&1);
/// assert!(match r { Ok(1..=4) => true, _ => false, });
/// ```
#[inline]
pub fn binary_search(&self, x: &T) -> Result<u32, u32>
where
T: Ord,
{
self.binary_search_by(|p| p.cmp(x))
}

/// Binary searches this sorted vector with a comparator function.
///
/// The comparator function should implement an order consistent
/// with the sort order of the underlying vector, returning an
/// order code that indicates whether its argument is `Less`,
/// `Equal` or `Greater` the desired target.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
/// one of the matches could be returned. If the value is not found then
/// [`Result::Err`] is returned, containing the index where a matching
/// element could be inserted while maintaining sorted order.
///
/// See also [`binary_search`], [`binary_search_by_key`].
///
/// [`binary_search`]: Vec::binary_search
/// [`binary_search_by_key`]: Vec::binary_search_by_key
///
/// # Examples
///
/// Looks up a series of four elements. The first is found, with a
/// uniquely determined position; the second and third are not
/// found; the fourth could match any position in `[1, 4]`.
///
/// ```
/// use ink_storage::Vec as StorageVec;
///
/// let s: StorageVec<i32> = [0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55]
/// .into_iter()
/// .copied()
/// .collect();
///
/// let seek = 13;
/// assert_eq!(s.binary_search_by(|probe| probe.cmp(&seek)), Ok(9));
/// let seek = 4;
/// assert_eq!(s.binary_search_by(|probe| probe.cmp(&seek)), Err(7));
/// let seek = 100;
/// assert_eq!(s.binary_search_by(|probe| probe.cmp(&seek)), Err(13));
/// let seek = 1;
/// let r = s.binary_search_by(|probe| probe.cmp(&seek));
/// assert!(match r { Ok(1..=4) => true, _ => false, });
/// ```
// The binary_search implementation is ported from
// /~https://github.com/rust-lang/rust/blob/c5e344f7747dbd7e7d4b209e3c480deb5979a56f/library/core/src/slice/mod.rs#L2191
// and attempts to remain as close to the source as possible.
#[inline]
pub fn binary_search_by<'a, F>(&'a self, mut f: F) -> Result<u32, u32>
where
F: FnMut(&'a T) -> core::cmp::Ordering,
{
use core::cmp::Ordering::*;

let mut size = self.len();
let mut left = 0;
let mut right = size;
while left < right {
let mid = left + size / 2;

// the call is made safe by the following invariants:
// - `mid >= 0`
// - `mid < size`: `mid` is limited by `[left; right)` bound.
let cmp = f(&self[mid]);

// The reason why we use if/else control flow rather than match
// is because match reorders comparison operations, which is perf sensitive.
if cmp == Less {
left = mid + 1;
} else if cmp == Greater {
right = mid;
} else {
return Ok(mid)
}

size = right - left;
}
Err(left)
}

/// Binary searches this sorted vector with a key extraction function.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
/// one of the matches could be returned. If the value is not found then
/// [`Result::Err`] is returned, containing the index where a matching
/// element could be inserted while maintaining sorted order.
///
/// See also [`binary_search`], [`binary_search_by`].
///
/// [`binary_search`]: Vec::binary_search
/// [`binary_search_by`]: Vec::binary_search_by
///
/// # Examples
///
/// Looks up a series of four elements in a vector of pairs sorted by
/// their second elements. The first is found, with a uniquely
/// determined position; the second and third are not found; the
/// fourth could match any position in `[1, 4]`.
///
/// ```
/// use ink_storage::Vec as StorageVec;
///
/// let s: StorageVec<(i32, i32)> = [
/// (0, 0),
/// (2, 1),
/// (4, 1),
/// (5, 1),
/// (3, 1),
/// (1, 2),
/// (2, 3),
/// (4, 5),
/// (5, 8),
/// (3, 13),
/// (1, 21),
/// (2, 34),
/// (4, 55),
/// ]
/// .into_iter()
/// .copied()
/// .collect();
///
/// assert_eq!(s.binary_search_by_key(&13, |&(a, b)| b), Ok(9));
/// assert_eq!(s.binary_search_by_key(&4, |&(a, b)| b), Err(7));
/// assert_eq!(s.binary_search_by_key(&100, |&(a, b)| b), Err(13));
/// let r = s.binary_search_by_key(&1, |&(a, b)| b);
/// assert!(match r { Ok(1..=4) => true, _ => false, });
/// ```
#[inline]
pub fn binary_search_by_key<'a, B, F>(&'a self, b: &B, mut f: F) -> Result<u32, u32>
where
F: FnMut(&'a T) -> B,
B: Ord,
{
self.binary_search_by(|k| f(k).cmp(b))
}
}

impl<T> Vec<T>
Expand Down
85 changes: 81 additions & 4 deletions crates/storage/src/collections/vec/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use super::Vec as StorageVec;
use crate::{
collections::vec::IndexOutOfBounds,
traits::{
KeyPtr,
PackedLayout,
SpreadLayout,
},
Lazy,
};
use core::cmp::Ordering;
use ink_primitives::Key;

#[test]
Expand Down Expand Up @@ -276,8 +277,8 @@ fn assert_eq_slice(vec: &StorageVec<u8>, slice: &[u8]) {
}

/// Creates a storage vector from the given slice.
fn vec_from_slice(slice: &[u8]) -> StorageVec<u8> {
slice.iter().copied().collect::<StorageVec<u8>>()
fn vec_from_slice<T: Copy + PackedLayout>(slice: &[T]) -> StorageVec<T> {
slice.iter().copied().collect::<StorageVec<T>>()
}

#[test]
Expand Down Expand Up @@ -422,11 +423,87 @@ fn clear_works_on_filled_vec() {

#[test]
fn clear_works_on_empty_vec() {
let mut vec = vec_from_slice(&[]);
let mut vec: StorageVec<()> = vec_from_slice(&[]);
vec.clear();
assert!(vec.is_empty());
}

#[test]
fn test_binary_search() {
let b: StorageVec<i32> = StorageVec::new();
assert_eq!(b.binary_search(&5), Err(0));

let b = vec_from_slice(&[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 = vec_from_slice(&[1, 2, 4, 6, 8, 9]);
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 = vec_from_slice(&[1, 2, 4, 5, 6, 8]);
assert_eq!(b.binary_search(&9), Err(6));

let b = vec_from_slice(&[1, 2, 4, 6, 7, 8, 9]);
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 = vec_from_slice(&[1, 2, 4, 5, 6, 8, 9]);
assert_eq!(b.binary_search(&7), Err(5));
assert_eq!(b.binary_search(&0), Err(0));

let b = vec_from_slice(&[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));

let b = vec_from_slice(&[(); u8::MAX as usize]);
assert_eq!(b.binary_search(&()), Ok(u8::MAX as u32 / 2));
}

#[test]
fn test_binary_search_by_overflow() {
let b = vec_from_slice(&[(); u8::MAX as usize]);
assert_eq!(
b.binary_search_by(|_| Ordering::Equal),
Ok(u8::MAX as u32 / 2)
);
assert_eq!(b.binary_search_by(|_| Ordering::Greater), Err(0));
assert_eq!(b.binary_search_by(|_| Ordering::Less), Err(u8::MAX as u32));
}

#[test]
// Test implementation specific behavior when finding equivalent elements.
fn test_binary_search_implementation_details() {
let b = vec_from_slice(&[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(5));
let b = vec_from_slice(&[1, 1, 1, 1, 1, 3, 3, 3, 3]);
assert_eq!(b.binary_search(&1), Ok(4));
assert_eq!(b.binary_search(&3), Ok(7));
let b = vec_from_slice(&[1, 1, 1, 1, 3, 3, 3, 3, 3]);
assert_eq!(b.binary_search(&1), Ok(2));
assert_eq!(b.binary_search(&3), Ok(4));
}

#[test]
#[should_panic(expected = "encountered empty storage cell")]
#[cfg(not(feature = "ink-experimental-engine"))]
Expand Down

0 comments on commit 36bbdec

Please sign in to comment.