diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 79edf65..6ba504a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -44,6 +44,9 @@ jobs: - name: Cargo test w/ malloc_size_of run: cargo test --verbose --features malloc_size_of + - name: Cargo fmt + run: cargo --all -- --check + - name: Cargo check w/o default features if: matrix.toolchain == 'nightly' run: cargo check --verbose --no-default-features @@ -92,4 +95,3 @@ jobs: - name: Mark the job as unsuccessful run: exit 1 if: "!success()" - diff --git a/Cargo.toml b/Cargo.toml index aac85c7..1deb55b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,13 @@ malloc_size_of = { version = "0.1.1", optional = true, default-features = false [dev-dependencies] bincode = "1.0.1" +criterion = "0.8" [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] + +[[bench]] +name = "bench" +path = "benches/bench.rs" +harness = false diff --git a/benches/bench.rs b/benches/bench.rs index 2386400..bded165 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -1,10 +1,8 @@ -#![feature(test)] #![allow(deprecated)] -extern crate test; - +use criterion::{criterion_group, criterion_main, Bencher, Criterion}; use smallvec::{smallvec, SmallVec}; -use test::Bencher; +use std::hint::black_box; const VEC_SIZE: usize = 16; const SPILLED_SIZE: usize = 100; @@ -18,87 +16,90 @@ trait Vector: for<'a> From<&'a [T]> + Extend { fn from_elem(val: T, n: usize) -> Self; fn from_elems(val: &[T]) -> Self; fn extend_from_slice(&mut self, other: &[T]); + fn retain_mut(&mut self, f: F) + where + F: FnMut(&mut T) -> bool; } impl Vector for Vec { fn new() -> Self { Self::with_capacity(VEC_SIZE) } - fn push(&mut self, val: T) { self.push(val) } - fn pop(&mut self) -> Option { self.pop() } - fn remove(&mut self, p: usize) -> T { self.remove(p) } - fn insert(&mut self, n: usize, val: T) { self.insert(n, val) } - fn from_elem(val: T, n: usize) -> Self { vec![val; n] } - fn from_elems(val: &[T]) -> Self { val.to_owned() } - fn extend_from_slice(&mut self, other: &[T]) { Vec::extend_from_slice(self, other) } + fn retain_mut(&mut self, f: F) + where + F: FnMut(&mut T) -> bool, + { + self.retain_mut(f) + } } impl Vector for SmallVec { fn new() -> Self { Self::new() } - fn push(&mut self, val: T) { self.push(val) } - fn pop(&mut self) -> Option { self.pop() } - fn remove(&mut self, p: usize) -> T { self.remove(p) } - fn insert(&mut self, n: usize, val: T) { self.insert(n, val) } - fn from_elem(val: T, n: usize) -> Self { smallvec![val; n] } - fn from_elems(val: &[T]) -> Self { SmallVec::from(val) } - fn extend_from_slice(&mut self, other: &[T]) { SmallVec::extend_from_slice(self, other) } + fn retain_mut(&mut self, f: F) + where + F: FnMut(&mut T) -> bool, + { + self.retain_mut(f) + } } macro_rules! make_benches { ($typ:ty { $($b_name:ident => $g_name:ident($($args:expr),*),)* }) => { $( - #[bench] - fn $b_name(b: &mut Bencher) { - $g_name::<$typ>($($args,)* b) + fn $b_name(c: &mut Criterion) { + c.bench_function(stringify!($b_name), |b: &mut Bencher| { + $g_name::<$typ>($($args,)* b) + }); } )* } } +/* ---------- Bench generation (same list, just using the new macro) ---------- */ make_benches! { SmallVec { bench_push => gen_push(SPILLED_SIZE as _), @@ -122,6 +123,12 @@ make_benches! { bench_macro_from_elem => gen_from_elem(SPILLED_SIZE as _), bench_macro_from_elem_small => gen_from_elem(VEC_SIZE as _), bench_pushpop => gen_pushpop(), + bench_retain_mut_half => gen_retain_mut_half(SPILLED_SIZE as _), + bench_retain_mut_half_small => gen_retain_mut_half(VEC_SIZE as _), + bench_retain_mut_all => gen_retain_mut_all(SPILLED_SIZE as _), + bench_retain_mut_all_small => gen_retain_mut_all(VEC_SIZE as _), + bench_retain_mut_none => gen_retain_mut_none(SPILLED_SIZE as _), + bench_retain_mut_none_small => gen_retain_mut_none(VEC_SIZE as _), } } @@ -148,156 +155,257 @@ make_benches! { bench_macro_from_elem_vec => gen_from_elem(SPILLED_SIZE as _), bench_macro_from_elem_vec_small => gen_from_elem(VEC_SIZE as _), bench_pushpop_vec => gen_pushpop(), + bench_retain_mut_vec_half => gen_retain_mut_half(SPILLED_SIZE as _), + bench_retain_mut_vec_half_small => gen_retain_mut_half(VEC_SIZE as _), + bench_retain_mut_vec_all => gen_retain_mut_all(SPILLED_SIZE as _), + bench_retain_mut_vec_all_small => gen_retain_mut_all(VEC_SIZE as _), + bench_retain_mut_vec_none => gen_retain_mut_none(SPILLED_SIZE as _), + bench_retain_mut_vec_none_small => gen_retain_mut_none(VEC_SIZE as _), } } +/* ---------- Benchmark helpers – unchanged except for Bencher type ---------- */ fn gen_push>(n: u64, b: &mut Bencher) { #[inline(never)] fn push_noinline>(vec: &mut V, x: u64) { - vec.push(x); + vec.push(black_box(x)); } b.iter(|| { + let n = black_box(n); let mut vec = V::new(); for x in 0..n { push_noinline(&mut vec, x); } - vec + black_box(vec) }); } fn gen_insert_push>(n: u64, b: &mut Bencher) { #[inline(never)] fn insert_push_noinline>(vec: &mut V, x: u64) { - vec.insert(x as usize, x); + vec.insert(black_box(x) as usize, black_box(x)); } b.iter(|| { + let n = black_box(n); let mut vec = V::new(); for x in 0..n { insert_push_noinline(&mut vec, x); } - vec + black_box(vec) }); } fn gen_insert>(n: u64, b: &mut Bencher) { #[inline(never)] fn insert_noinline>(vec: &mut V, p: usize, x: u64) { - vec.insert(p, x) + vec.insert(black_box(p), black_box(x)) } b.iter(|| { + let n = black_box(n); let mut vec = V::new(); - // Always insert at position 0 so that we are subject to shifts of - // many different lengths. vec.push(0); for x in 0..n { insert_noinline(&mut vec, 0, x); } - vec + black_box(vec) }); } fn gen_remove>(n: usize, b: &mut Bencher) { #[inline(never)] fn remove_noinline>(vec: &mut V, p: usize) -> u64 { - vec.remove(p) + vec.remove(black_box(p)) } b.iter(|| { + let n = black_box(n); let mut vec = V::from_elem(0, n as _); - for _ in 0..n { - remove_noinline(&mut vec, 0); + black_box(remove_noinline(&mut vec, 0)); } + black_box(vec) }); } fn gen_extend>(n: u64, b: &mut Bencher) { b.iter(|| { + let n = black_box(n); let mut vec = V::new(); vec.extend(0..n); - vec + black_box(vec) }); } fn gen_extend_filtered>(n: u64, b: &mut Bencher) { b.iter(|| { let mut vec = V::new(); - vec.extend((0..n).filter(|i| i % 2 == 0)); - vec + vec.extend((0..black_box(n)).filter(|i| black_box(*i) % 2 == 0)); + black_box(vec) }); } fn gen_from_iter>(n: u64, b: &mut Bencher) { - let v: Vec = (0..n).collect(); + let v: Vec = (0..black_box(n)).collect(); b.iter(|| { - let vec = V::from(&v); - vec + let vec = V::from(black_box(&v)); + black_box(vec) }); } fn gen_from_slice>(n: u64, b: &mut Bencher) { - let v: Vec = (0..n).collect(); + let v: Vec = (0..black_box(n)).collect(); b.iter(|| { - let vec = V::from_elems(&v); - vec + let vec = V::from_elems(black_box(&v)); + black_box(vec) }); } fn gen_extend_from_slice>(n: u64, b: &mut Bencher) { - let v: Vec = (0..n).collect(); + let v: Vec = (0..black_box(n)).collect(); b.iter(|| { let mut vec = V::new(); - vec.extend_from_slice(&v); - vec + vec.extend_from_slice(black_box(&v)); + black_box(vec) }); } fn gen_pushpop>(b: &mut Bencher) { #[inline(never)] fn pushpop_noinline>(vec: &mut V, x: u64) -> Option { - vec.push(x); + vec.push(black_box(x)); vec.pop() } b.iter(|| { let mut vec = V::new(); for x in 0..SPILLED_SIZE as _ { - pushpop_noinline(&mut vec, x); + black_box(pushpop_noinline(&mut vec, x)); } - vec + black_box(vec) }); } fn gen_from_elem>(n: usize, b: &mut Bencher) { b.iter(|| { - let vec = V::from_elem(42, n); - vec + let n = black_box(n); + let vec = V::from_elem(black_box(42), n); + black_box(vec) + }); +} + +fn gen_retain_mut_half>(n: usize, b: &mut Bencher) { + b.iter(|| { + let n = black_box(n); + let mut vec = V::from_elem(16, n); + vec.retain_mut(|x| black_box(*x) % 2 == 0); + black_box(vec) }); } -#[bench] -fn bench_macro_from_list(b: &mut Bencher) { +fn gen_retain_mut_all>(n: usize, b: &mut Bencher) { b.iter(|| { - let vec: SmallVec = smallvec![ - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20, 24, 32, 36, 0x40, 0x80, - 0x100, 0x200, 0x400, 0x800, 0x1000, 0x2000, 0x4000, 0x8000, 0x10000, 0x20000, 0x40000, - 0x80000, 0x100000, - ]; - vec + let n = black_box(n); + let mut vec = V::from_elem(16, n); + vec.retain_mut(|_| true); + black_box(vec) }); } -#[bench] -fn bench_macro_from_list_vec(b: &mut Bencher) { +fn gen_retain_mut_none>(n: usize, b: &mut Bencher) { b.iter(|| { - let vec: Vec = vec![ - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20, 24, 32, 36, 0x40, 0x80, - 0x100, 0x200, 0x400, 0x800, 0x1000, 0x2000, 0x4000, 0x8000, 0x10000, 0x20000, 0x40000, - 0x80000, 0x100000, - ]; - vec + let n = black_box(n); + let mut vec = V::from_elem(16, n); + vec.retain_mut(|_| false); + black_box(vec) }); } + +fn bench_macro_from_list(c: &mut Criterion) { + c.bench_function("bench_macro_from_list", |b| { + b.iter(|| { + let vec: SmallVec = smallvec![ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20, 24, 32, 36, 0x40, + 0x80, 0x100, 0x200, 0x400, 0x800, 0x1000, 0x2000, 0x4000, 0x8000, 0x10000, 0x20000, + 0x40000, 0x80000, 0x100000, + ]; + vec + }) + }); +} + +fn bench_macro_from_list_vec(c: &mut Criterion) { + c.bench_function("bench_macro_from_list_vec", |b| { + b.iter(|| { + let vec: Vec = vec![ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20, 24, 32, 36, 0x40, + 0x80, 0x100, 0x200, 0x400, 0x800, 0x1000, 0x2000, 0x4000, 0x8000, 0x10000, 0x20000, + 0x40000, 0x80000, 0x100000, + ]; + vec + }) + }); +} + +criterion_group!( + benches, + bench_push, + bench_push_small, + bench_insert_push, + bench_insert_push_small, + bench_insert, + bench_insert_small, + bench_remove, + bench_remove_small, + bench_extend, + bench_extend_small, + bench_extend_filtered, + bench_extend_filtered_small, + bench_from_iter, + bench_from_iter_small, + bench_from_slice, + bench_from_slice_small, + bench_extend_from_slice, + bench_extend_from_slice_small, + bench_macro_from_elem, + bench_macro_from_elem_small, + bench_pushpop, + bench_retain_mut_half, + bench_retain_mut_half_small, + bench_retain_mut_all, + bench_retain_mut_all_small, + bench_retain_mut_none, + bench_retain_mut_none_small, + bench_push_vec, + bench_push_vec_small, + bench_insert_push_vec, + bench_insert_push_vec_small, + bench_insert_vec, + bench_insert_vec_small, + bench_remove_vec, + bench_remove_vec_small, + bench_extend_vec, + bench_extend_vec_small, + bench_extend_vec_filtered, + bench_extend_vec_filtered_small, + bench_from_iter_vec, + bench_from_iter_vec_small, + bench_from_slice_vec, + bench_from_slice_vec_small, + bench_extend_from_slice_vec, + bench_extend_from_slice_vec_small, + bench_macro_from_elem_vec, + bench_macro_from_elem_vec_small, + bench_pushpop_vec, + bench_retain_mut_vec_half, + bench_retain_mut_vec_half_small, + bench_retain_mut_vec_all, + bench_retain_mut_vec_all_small, + bench_retain_mut_vec_none, + bench_retain_mut_vec_none_small, + bench_macro_from_list, + bench_macro_from_list_vec +); +criterion_main!(benches); diff --git a/src/lib.rs b/src/lib.rs index 135d7b9..eb5eb36 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -490,7 +490,10 @@ impl Drain<'_, T, N> { let range_start = vec.len(); let range_end = self.tail_start; let range_slice = unsafe { - core::slice::from_raw_parts_mut(vec.as_mut_ptr().add(range_start), range_end - range_start) + core::slice::from_raw_parts_mut( + vec.as_mut_ptr().add(range_start), + range_end - range_start, + ) }; for place in range_slice { @@ -690,7 +693,11 @@ impl Drop for Splice<'_, I, N> { } // Collect any remaining elements. - let mut collected = self.replace_with.by_ref().collect::>().into_iter(); + let mut collected = self + .replace_with + .by_ref() + .collect::>() + .into_iter(); // Now we have an exact count. if collected.len() > 0 { self.drain.move_tail(collected.len()); @@ -847,7 +854,9 @@ impl SmallVec { #[inline] pub const fn from_buf(elements: [T; S]) -> Self { - const { assert!(S <= N); } + const { + assert!(S <= N); + } // Althought we create a new buffer, since S and N are known at compile time, // even with `-C opt-level=1`, it gets optimized as best as it could be. (Checked with ) @@ -1207,7 +1216,10 @@ impl SmallVec { R: core::ops::RangeBounds, I: IntoIterator, { - Splice { drain: self.drain(range), replace_with: replace_with.into_iter() } + Splice { + drain: self.drain(range), + replace_with: replace_with.into_iter(), + } } #[inline] @@ -1226,15 +1238,16 @@ impl SmallVec { #[inline] pub fn pop(&mut self) -> Option { - if self.is_empty() { + let len = self.len(); + if len == 0 { None } else { - let len = self.len() - 1; - // SAFETY: len < old_len since this can't overflow, because the old length is non zero - unsafe { self.set_len(len) }; + let new_len = len - 1; + // SAFETY: new_len < len since len is non-zero + unsafe { self.set_len(new_len) }; // SAFETY: this element was initialized and we just gave up ownership of it, so we can // give it away - let value = unsafe { self.as_mut_ptr().add(len).read() }; + let value = unsafe { self.as_mut_ptr().add(new_len).read() }; Some(value) } } @@ -1242,7 +1255,11 @@ impl SmallVec { #[inline] pub fn pop_if(&mut self, predicate: impl FnOnce(&mut T) -> bool) -> Option { let last = self.last_mut()?; - if predicate(last) { self.pop() } else { None } + if predicate(last) { + self.pop() + } else { + None + } } #[inline] @@ -1311,16 +1328,26 @@ impl SmallVec { } #[inline] + #[track_caller] pub fn reserve(&mut self, additional: usize) { - // can't overflow since len <= capacity - if additional > self.capacity() - self.len() { + // Callers expect this function to be very cheap when there is already sufficient capacity. + // Therefore, we move all the resizing and error-handling logic behind a call, while making + // sure that this function is likely to be inlined as just a comparison and a call if the + // comparison fails. + #[cold] + fn do_reserve_and_handle(slf: &mut SmallVec, additional: usize) { let new_capacity = infallible( - self.len() + slf.len() .checked_add(additional) .and_then(usize::checked_next_power_of_two) .ok_or(CollectionAllocErr::CapacityOverflow), ); - self.grow(new_capacity); + slf.grow(new_capacity); + } + + // can't overflow since len <= capacity + if additional > self.capacity() - self.len() { + do_reserve_and_handle(self, additional); } } @@ -1332,22 +1359,30 @@ impl SmallVec { .checked_add(additional) .and_then(usize::checked_next_power_of_two) .ok_or(CollectionAllocErr::CapacityOverflow)?; - self.try_grow(new_capacity) - } else { - Ok(()) + self.try_grow(new_capacity)?; } + Ok(()) } #[inline] + #[track_caller] pub fn reserve_exact(&mut self, additional: usize) { - // can't overflow since len <= capacity - if additional > self.capacity() - self.len() { + #[cold] + fn do_reserve_exact_and_handle( + slf: &mut SmallVec, + additional: usize, + ) { let new_capacity = infallible( - self.len() + slf.len() .checked_add(additional) .ok_or(CollectionAllocErr::CapacityOverflow), ); - self.grow(new_capacity); + slf.grow(new_capacity); + } + + // can't overflow since len <= capacity + if additional > self.capacity() - self.len() { + do_reserve_exact_and_handle(self, additional); } } @@ -1358,10 +1393,9 @@ impl SmallVec { .len() .checked_add(additional) .ok_or(CollectionAllocErr::CapacityOverflow)?; - self.try_grow(new_capacity) - } else { - Ok(()) + self.try_grow(new_capacity)?; } + Ok(()) } #[inline] @@ -1407,7 +1441,10 @@ impl SmallVec { self.set_inline(); alloc::alloc::dealloc( ptr.cast().as_ptr(), - Layout::from_size_align_unchecked(capacity * size_of::(), align_of::()), + Layout::from_size_align_unchecked( + capacity * size_of::(), + align_of::(), + ), ); } } else if target < self.capacity() { @@ -1438,7 +1475,10 @@ impl SmallVec { #[inline] pub fn swap_remove(&mut self, index: usize) -> T { let len = self.len(); - assert!(index < len, "swap_remove index (is {index}) should be < len (is {len})"); + assert!( + index < len, + "swap_remove index (is {index}) should be < len (is {len})" + ); // This can't overflow since `len > index >= 0` let new_len = len - 1; unsafe { @@ -1470,7 +1510,10 @@ impl SmallVec { #[inline] pub fn remove(&mut self, index: usize) -> T { let len = self.len(); - assert!(index < len, "removal index (is {index}) should be < len (is {len})"); + assert!( + index < len, + "removal index (is {index}) should be < len (is {len})" + ); let new_len = len - 1; unsafe { // SAFETY: new_len < len @@ -1487,7 +1530,10 @@ impl SmallVec { #[inline] pub fn insert(&mut self, index: usize, value: T) { let len = self.len(); - assert!(index <= len, "insertion index (is {index}) should be <= len (is {len})"); + assert!( + index <= len, + "insertion index (is {index}) should be <= len (is {len})" + ); self.reserve(1); let ptr = self.as_mut_ptr(); unsafe { @@ -1598,21 +1644,28 @@ impl SmallVec { #[inline] pub fn retain_mut bool>(&mut self, mut f: F) { - let mut del = 0; let len = self.len(); + + if len == 0 { + // return early as hint to llvm, like what std does + return; + } + let ptr = self.as_mut_ptr(); - for i in 0..len { + let mut write_idx = 0; + + for read_idx in 0..len { // SAFETY: all the pointers are in bounds - // `i - del` never overflows since `del <= i` is a maintained invariant unsafe { - if !f(&mut *ptr.add(i)) { - del += 1; - } else if del > 0 { - core::ptr::swap(ptr.add(i), ptr.add(i - del)); + if f(&mut *ptr.add(read_idx)) { + if write_idx < read_idx { + core::ptr::swap(ptr.add(read_idx), ptr.add(write_idx)); + } + write_idx += 1; } } } - self.truncate(len - del); + self.truncate(write_idx); } #[inline] @@ -1683,7 +1736,9 @@ impl SmallVec { pub fn leak<'a>(self) -> &'a mut [T] { if !self.spilled() { - panic!("SmallVec::leak() called on inline (stack) SmallVec, which cannot be safely leaked"); + panic!( + "SmallVec::leak() called on inline (stack) SmallVec, which cannot be safely leaked" + ); } let mut me = ManuallyDrop::new(self); unsafe { core::slice::from_raw_parts_mut(me.as_mut_ptr(), me.len()) } @@ -1816,12 +1871,11 @@ impl SmallVec { #[inline] pub fn extend_from_slice_copy(&mut self, other: &[T]) where - T: Copy + T: Copy, { - let len = other.len(); let src = other.as_ptr(); - + let l = self.len(); self.reserve(len); @@ -1837,7 +1891,7 @@ impl SmallVec { pub fn extend_from_within_copy(&mut self, src: R) where R: core::ops::RangeBounds, - T: Copy + T: Copy, { let src = slice_range(src, ..self.len()); let core::ops::Range { start, end } = src; @@ -1856,7 +1910,7 @@ impl SmallVec { pub fn insert_from_slice_copy(&mut self, index: usize, other: &[T]) where - T: Copy + T: Copy, { let l = self.len(); let len = other.len(); @@ -1880,7 +1934,7 @@ impl SmallVec { /// for types with the [`Copy`] trait. pub fn from_slice_copy(slice: &[T]) -> Self where - T: Copy + T: Copy, { let src = slice.as_ptr(); let len = slice.len(); @@ -2594,7 +2648,7 @@ impl Clone for SmallVec { #[cfg(not(feature = "specialization"))] { - self.clone_from_fallback(&*source); + self.clone_from_fallback(source); } } } diff --git a/src/tests.rs b/src/tests.rs index 2082f96..989f9fb 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -587,10 +587,7 @@ fn test_from() { #[test] fn test_from_slice() { assert_eq!(&SmallVec::::from(&[1][..])[..], [1]); - assert_eq!( - &SmallVec::::from(&[1, 2, 3][..])[..], - [1, 2, 3] - ); + assert_eq!(&SmallVec::::from(&[1, 2, 3][..])[..], [1, 2, 3]); } #[test] @@ -990,7 +987,11 @@ fn collect_from_iter() { // A length of 3 is fine to trigger this bug under valgrind, but making the vector 1 million // elements makes it crash - which is much easier to detect. - let iter = IterNoHint(std::iter::repeat(1u8).take(1_000_000)); + #[cfg(miri)] + const ELEMENTS: usize = 1000; + #[cfg(not(miri))] + const ELEMENTS: usize = 1_000_000; + let iter = IterNoHint(std::iter::repeat(1u8).take(ELEMENTS)); let _y: SmallVec = SmallVec::from_iter(iter); }