diff --git a/.gitignore b/.gitignore index 15d4356..6ea059b 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /target **/*.rs.bk Cargo.lock +benchmarks.txt # Editors .idea diff --git a/benches/bench.rs b/benches/bench.rs index aad03d3..1b13b85 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -73,7 +73,7 @@ macro_rules! generate_benches { (non_power_two, $c: tt, $rb: tt, $ty: tt, $fn: tt, $bmfunc: tt, $($i:tt),*) => { $( $c.bench_function(&format!("{} {} 1M capacity not power of two {}", stringify!($rb), stringify!($bmfunc), stringify!($i)), |b| $bmfunc(b, || { - $rb::<$ty, _>::$fn($i) + $rb::<$ty>::$fn($i) })); )* }; @@ -87,19 +87,6 @@ macro_rules! generate_benches { }; } -fn benchmark_non_power_of_two(b: &mut Bencher) { - b.iter(|| { - let mut rb = AllocRingBuffer::with_capacity_non_power_of_two(L); - - for i in 0..1_000_000 { - rb.push(i); - black_box(()); - } - - rb - }) -} - fn criterion_benchmark(c: &mut Criterion) { // TODO: Improve benchmarks // * What are representative operations @@ -111,7 +98,7 @@ fn criterion_benchmark(c: &mut Criterion) { c, AllocRingBuffer, i32, - with_capacity, + new, benchmark_push, 16, 1024, @@ -135,7 +122,7 @@ fn criterion_benchmark(c: &mut Criterion) { c, AllocRingBuffer, i32, - with_capacity, + new, benchmark_various, 16, 1024, @@ -159,7 +146,7 @@ fn criterion_benchmark(c: &mut Criterion) { c, AllocRingBuffer, i32, - with_capacity, + new, benchmark_push_dequeue, 16, 1024, @@ -183,7 +170,7 @@ fn criterion_benchmark(c: &mut Criterion) { c, AllocRingBuffer, i32, - with_capacity_non_power_of_two, + new, benchmark_various, 16, 17, diff --git a/src/lib.rs b/src/lib.rs index bbf54dd..d115bde 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,8 +26,6 @@ mod with_alloc; #[cfg(feature = "alloc")] pub use with_alloc::alloc_ringbuffer::AllocRingBuffer; #[cfg(feature = "alloc")] -pub use with_alloc::alloc_ringbuffer::{NonPowerOfTwo, PowerOfTwo, RingbufferSize}; -#[cfg(feature = "alloc")] pub use with_alloc::vecdeque::GrowableAllocRingBuffer; mod with_const_generics; @@ -1368,6 +1366,6 @@ mod tests { test_clone!(ConstGenericRingBuffer::<_, 4>::new()); test_clone!(GrowableAllocRingBuffer::<_>::new()); - test_clone!(AllocRingBuffer::<_, _>::new(4)); + test_clone!(AllocRingBuffer::<_>::new(4)); } } diff --git a/src/with_alloc/alloc_ringbuffer.rs b/src/with_alloc/alloc_ringbuffer.rs index b6fbd22..71042a0 100644 --- a/src/with_alloc/alloc_ringbuffer.rs +++ b/src/with_alloc/alloc_ringbuffer.rs @@ -7,60 +7,9 @@ use crate::ringbuffer_trait::{ extern crate alloc; // We need boxes, so depend on alloc -use crate::GrowableAllocRingBuffer; -use core::marker::PhantomData; +use crate::{mask, GrowableAllocRingBuffer}; use core::ptr; -#[derive(Debug, Copy, Clone)] -/// Represents that an alloc ringbuffer must have a size that's a power of two. -/// This means slightly more optimizations can be performed, but it is less flexible. -pub struct PowerOfTwo; - -#[derive(Debug, Copy, Clone)] -/// Represents that an alloc ringbuffer can have a size that's not a power of two. -/// This means slightly fewer optimizations can be performed, but it is more flexible. -pub struct NonPowerOfTwo; -mod private { - use crate::with_alloc::alloc_ringbuffer::{NonPowerOfTwo, PowerOfTwo}; - - pub trait Sealed {} - - impl Sealed for PowerOfTwo {} - impl Sealed for NonPowerOfTwo {} -} - -/// Sealed trait with two implementations that represent the kinds of sizes a ringbuffer can have -/// *[`NonPowerOfTwo`] -/// *[`PowerOfTwo`] -pub trait RingbufferSize: private::Sealed { - /// the mask function to use for wrapping indices in the ringbuffer - fn mask(cap: usize, index: usize) -> usize; - /// true in [`PowerOfTwo`], false in [`NonPowerOfTwo`] - fn must_be_power_of_two() -> bool; -} - -impl RingbufferSize for PowerOfTwo { - #[inline] - fn mask(cap: usize, index: usize) -> usize { - crate::mask(cap, index) - } - - fn must_be_power_of_two() -> bool { - true - } -} - -impl RingbufferSize for NonPowerOfTwo { - #[inline] - fn mask(cap: usize, index: usize) -> usize { - crate::mask_modulo(cap, index) - } - - fn must_be_power_of_two() -> bool { - false - } -} - /// The `AllocRingBuffer` is a `RingBuffer` which is based on a Vec. This means it allocates at runtime /// on the heap, and therefore needs the [`alloc`] crate. This struct and therefore the dependency on /// alloc can be disabled by disabling the `alloc` (default) feature. @@ -88,12 +37,17 @@ impl RingbufferSize for NonPowerOfTwo { /// assert_eq!(buffer.to_vec(), vec![42, 1]); /// ``` #[derive(Debug)] -pub struct AllocRingBuffer { +pub struct AllocRingBuffer { buf: *mut T, + + // the size of the allocation. Next power of 2 up from the capacity + size: usize, + // maximum number of elements actually allowed in the ringbuffer. + // Always less than or equal than the size capacity: usize, + readptr: usize, writeptr: usize, - mode: PhantomData, } // SAFETY: all methods that require mutable access take &mut, @@ -101,15 +55,15 @@ pub struct AllocRingBuffer { unsafe impl Sync for AllocRingBuffer {} unsafe impl Send for AllocRingBuffer {} -impl From<[T; N]> for AllocRingBuffer { +impl From<[T; N]> for AllocRingBuffer { fn from(value: [T; N]) -> Self { - let mut rb = Self::with_capacity_non_power_of_two(value.len()); + let mut rb = Self::new(value.len()); rb.extend(value); rb } } -impl From<&[T; N]> for AllocRingBuffer { +impl From<&[T; N]> for AllocRingBuffer { // the cast here is actually not trivial #[allow(trivial_casts)] fn from(value: &[T; N]) -> Self { @@ -117,109 +71,104 @@ impl From<&[T; N]> for AllocRingBuffer From<&[T]> for AllocRingBuffer { +impl From<&[T]> for AllocRingBuffer { fn from(value: &[T]) -> Self { - let mut rb = Self::with_capacity_non_power_of_two(value.len()); + let mut rb = Self::new(value.len()); rb.extend(value.iter().cloned()); rb } } -impl From> for AllocRingBuffer { - fn from(mut v: GrowableAllocRingBuffer) -> AllocRingBuffer { - let mut rb = AllocRingBuffer::with_capacity_non_power_of_two(v.len()); +impl From> for AllocRingBuffer { + fn from(mut v: GrowableAllocRingBuffer) -> AllocRingBuffer { + let mut rb = AllocRingBuffer::new(v.len()); rb.extend(v.drain()); rb } } -impl From<&mut [T]> for AllocRingBuffer { +impl From<&mut [T]> for AllocRingBuffer { fn from(value: &mut [T]) -> Self { Self::from(&*value) } } -impl From<&mut [T; CAP]> for AllocRingBuffer { +impl From<&mut [T; CAP]> for AllocRingBuffer { fn from(value: &mut [T; CAP]) -> Self { Self::from(value.clone()) } } -impl From> for AllocRingBuffer { +impl From> for AllocRingBuffer { fn from(value: alloc::vec::Vec) -> Self { - let mut res = AllocRingBuffer::with_capacity_non_power_of_two(value.len()); + let mut res = AllocRingBuffer::new(value.len()); res.extend(value); res } } -impl From> for AllocRingBuffer { +impl From> for AllocRingBuffer { fn from(value: alloc::collections::VecDeque) -> Self { - let mut res = AllocRingBuffer::with_capacity_non_power_of_two(value.len()); + let mut res = AllocRingBuffer::new(value.len()); res.extend(value); res } } -impl From> for AllocRingBuffer { +impl From> for AllocRingBuffer { fn from(value: alloc::collections::LinkedList) -> Self { - let mut res = AllocRingBuffer::with_capacity_non_power_of_two(value.len()); + let mut res = AllocRingBuffer::new(value.len()); res.extend(value); res } } -impl From for AllocRingBuffer { +impl From for AllocRingBuffer { fn from(value: alloc::string::String) -> Self { - let mut res = AllocRingBuffer::with_capacity_non_power_of_two(value.len()); + let mut res = AllocRingBuffer::new(value.len()); res.extend(value.chars()); res } } -impl From<&str> for AllocRingBuffer { +impl From<&str> for AllocRingBuffer { fn from(value: &str) -> Self { - let mut res = AllocRingBuffer::with_capacity_non_power_of_two(value.len()); + let mut res = AllocRingBuffer::new(value.len()); res.extend(value.chars()); res } } -impl From> - for AllocRingBuffer -{ +impl From> for AllocRingBuffer { fn from(mut value: crate::ConstGenericRingBuffer) -> Self { - let mut res = AllocRingBuffer::with_capacity_non_power_of_two(value.len()); + let mut res = AllocRingBuffer::new(value.len()); res.extend(value.drain()); res } } -impl Drop for AllocRingBuffer { +impl Drop for AllocRingBuffer { fn drop(&mut self) { self.drain().for_each(drop); - let layout = alloc::alloc::Layout::array::(self.capacity).unwrap(); + let layout = alloc::alloc::Layout::array::(self.size).unwrap(); unsafe { alloc::alloc::dealloc(self.buf as *mut u8, layout); } } } -impl Clone for AllocRingBuffer { +impl Clone for AllocRingBuffer { fn clone(&self) -> Self { debug_assert_ne!(self.capacity, 0); - debug_assert!(!SIZE::must_be_power_of_two() || self.capacity.is_power_of_two()); - // whatever the previous capacity was, we can just use the same one again. - // It should be valid. - let mut new = unsafe { Self::with_capacity_unchecked(self.capacity) }; + let mut new = Self::new(self.capacity); self.iter().cloned().for_each(|i| new.push(i)); new } } -impl PartialEq for AllocRingBuffer { +impl PartialEq for AllocRingBuffer { fn eq(&self, other: &Self) -> bool { self.capacity == other.capacity && self.len() == other.len() @@ -227,9 +176,9 @@ impl PartialEq for AllocRingBuffer } } -impl Eq for AllocRingBuffer {} +impl Eq for AllocRingBuffer {} -impl IntoIterator for AllocRingBuffer { +impl IntoIterator for AllocRingBuffer { type Item = T; type IntoIter = RingBufferIntoIterator; @@ -238,25 +187,25 @@ impl IntoIterator for AllocRingBuffer { } } -impl<'a, T, SIZE: RingbufferSize> IntoIterator for &'a AllocRingBuffer { +impl<'a, T> IntoIterator for &'a AllocRingBuffer { type Item = &'a T; - type IntoIter = RingBufferIterator<'a, T, AllocRingBuffer>; + type IntoIter = RingBufferIterator<'a, T, AllocRingBuffer>; fn into_iter(self) -> Self::IntoIter { self.iter() } } -impl<'a, T, SIZE: RingbufferSize> IntoIterator for &'a mut AllocRingBuffer { +impl<'a, T> IntoIterator for &'a mut AllocRingBuffer { type Item = &'a mut T; - type IntoIter = RingBufferMutIterator<'a, T, AllocRingBuffer>; + type IntoIter = RingBufferMutIterator<'a, T, AllocRingBuffer>; fn into_iter(self) -> Self::IntoIter { self.iter_mut() } } -impl Extend for AllocRingBuffer { +impl Extend for AllocRingBuffer { fn extend>(&mut self, iter: A) { let iter = iter.into_iter(); @@ -266,7 +215,7 @@ impl Extend for AllocRingBuffer { } } -unsafe impl RingBuffer for AllocRingBuffer { +unsafe impl RingBuffer for AllocRingBuffer { #[inline] unsafe fn ptr_capacity(rb: *const Self) -> usize { (*rb).capacity @@ -277,12 +226,8 @@ unsafe impl RingBuffer for AllocRingBuffer #[inline] fn push(&mut self, value: T) { if self.is_full() { - let previous_value = unsafe { - ptr::read(get_unchecked_mut( - self, - SIZE::mask(self.capacity, self.readptr), - )) - }; + let previous_value = + unsafe { ptr::read(get_unchecked_mut(self, mask(self.size, self.readptr))) }; // make sure we drop whatever is being overwritten // SAFETY: the buffer is full, so this must be initialized @@ -295,7 +240,7 @@ unsafe impl RingBuffer for AllocRingBuffer self.readptr += 1; } - let index = SIZE::mask(self.capacity, self.writeptr); + let index = mask(self.size, self.writeptr); unsafe { ptr::write(get_unchecked_mut(self, index), value); @@ -308,7 +253,7 @@ unsafe impl RingBuffer for AllocRingBuffer if self.is_empty() { None } else { - let index = SIZE::mask(self.capacity, self.readptr); + let index = mask(self.size, self.readptr); let res = unsafe { get_unchecked_mut(self, index) }; self.readptr += 1; @@ -319,13 +264,7 @@ unsafe impl RingBuffer for AllocRingBuffer } } - impl_ringbuffer_ext!( - get_unchecked, - get_unchecked_mut, - readptr, - writeptr, - SIZE::mask - ); + impl_ringbuffer_ext!(get_unchecked, get_unchecked_mut, readptr, writeptr, mask); #[inline] fn fill_with T>(&mut self, mut f: F) { @@ -340,59 +279,13 @@ unsafe impl RingBuffer for AllocRingBuffer } } -impl AllocRingBuffer { - /// Creates a `AllocRingBuffer` with a certain capacity. This capacity is fixed. - /// for this ringbuffer to work, cap must be a power of two and greater than zero. - /// - /// # Safety - /// Only safe if the capacity is greater than zero, and a power of two. - /// Only if `MODE` == [`NonPowerOfTwo`](NonPowerOfTwo) can the capacity be not a power of two, in which case this function is also safe. - #[inline] - unsafe fn with_capacity_unchecked(cap: usize) -> Self { - let layout = alloc::alloc::Layout::array::(cap).unwrap(); - let buf = unsafe { alloc::alloc::alloc(layout) as *mut T }; - - Self { - buf, - capacity: cap, - readptr: 0, - writeptr: 0, - mode: PhantomData, - } - } -} - -impl AllocRingBuffer { - /// Creates a `AllocRingBuffer` with a certain capacity. This capacity is fixed. - /// for this ringbuffer to work, and must not be zero. - /// - /// Note, that not using a power of two means some operations can't be optimized as well. - /// For example, bitwise ands might become modulos. - /// - /// For example, on push operations, benchmarks have shown that a ringbuffer with a power-of-two - /// capacity constructed with `with_capacity_non_power_of_two` (so which don't get the same optimization - /// as the ones constructed with `with_capacity`) can be up to 3x slower - /// - /// # Panics - /// if the capacity is zero - #[inline] - #[must_use] - pub fn with_capacity_non_power_of_two(cap: usize) -> Self { - assert_ne!(cap, 0, "Capacity must be greater than 0"); - - // Safety: Mode is NonPowerOfTwo and we checked above that the capacity isn't zero - unsafe { Self::with_capacity_unchecked(cap) } - } -} - -impl AllocRingBuffer { +impl AllocRingBuffer { /// Creates a `AllocRingBuffer` with a certain capacity. The actual capacity is the input to the /// function raised to the power of two (effectively the input is the log2 of the actual capacity) #[inline] #[must_use] pub fn with_capacity_power_of_2(cap_power_of_two: usize) -> Self { - // Safety: 1 << n is always a power of two, and nonzero - unsafe { Self::with_capacity_unchecked(1 << cap_power_of_two) } + Self::new(1 << cap_power_of_two) } #[inline] @@ -403,27 +296,32 @@ impl AllocRingBuffer { Self::new(cap) } - /// Creates a `AllocRingBuffer` with a certain capacity. The capacity must be a power of two. + /// Creates a `AllocRingBuffer` with a certain capacity. The capacity must not be zero. + /// /// # Panics - /// Panics when capacity is zero or not a power of two + /// Panics when capacity is zero #[inline] #[must_use] - pub fn new(cap: usize) -> Self { - assert_ne!(cap, 0, "Capacity must be greater than 0"); - assert!(cap.is_power_of_two(), "Capacity must be a power of two"); - - // Safety: assertions check that cap is a power of two and nonzero - unsafe { Self::with_capacity_unchecked(cap) } + pub fn new(capacity: usize) -> Self { + assert_ne!(capacity, 0, "Capacity must be greater than 0"); + let size = capacity.next_power_of_two(); + let layout = alloc::alloc::Layout::array::(size).unwrap(); + let buf = unsafe { alloc::alloc::alloc(layout) as *mut T }; + Self { + buf, + size, + capacity, + readptr: 0, + writeptr: 0, + } } } /// Get a reference from the buffer without checking it is initialized. +/// /// Caller must be sure the index is in bounds, or this will panic. #[inline] -unsafe fn get_unchecked<'a, T, SIZE: RingbufferSize>( - rb: *const AllocRingBuffer, - index: usize, -) -> &'a T { +unsafe fn get_unchecked<'a, T>(rb: *const AllocRingBuffer, index: usize) -> &'a T { let p = (*rb).buf.add(index); // Safety: caller makes sure the index is in bounds for the ringbuffer. // All in bounds values in the ringbuffer are initialized @@ -431,12 +329,10 @@ unsafe fn get_unchecked<'a, T, SIZE: RingbufferSize>( } /// Get a mut reference from the buffer without checking it is initialized. +/// /// Caller must be sure the index is in bounds, or this will panic. #[inline] -unsafe fn get_unchecked_mut( - rb: *mut AllocRingBuffer, - index: usize, -) -> *mut T { +unsafe fn get_unchecked_mut(rb: *mut AllocRingBuffer, index: usize) -> *mut T { let p = (*rb).buf.add(index); // Safety: caller makes sure the index is in bounds for the ringbuffer. @@ -444,7 +340,7 @@ unsafe fn get_unchecked_mut( p.cast() } -impl Index for AllocRingBuffer { +impl Index for AllocRingBuffer { type Output = T; fn index(&self, index: isize) -> &Self::Output { @@ -452,7 +348,7 @@ impl Index for AllocRingBuffer { } } -impl IndexMut for AllocRingBuffer { +impl IndexMut for AllocRingBuffer { fn index_mut(&mut self, index: isize) -> &mut Self::Output { self.get_mut(index).expect("index out of bounds") } @@ -460,25 +356,22 @@ impl IndexMut for AllocRingBuffer { #[cfg(test)] mod tests { - use crate::with_alloc::alloc_ringbuffer::RingbufferSize; use crate::{AllocRingBuffer, RingBuffer}; // just test that this compiles #[test] fn test_generic_clone() { - fn helper( - a: &AllocRingBuffer, - ) -> AllocRingBuffer { + fn helper(a: &AllocRingBuffer) -> AllocRingBuffer { a.clone() } _ = helper(&AllocRingBuffer::new(2)); - _ = helper(&AllocRingBuffer::with_capacity_non_power_of_two(5)); + _ = helper(&AllocRingBuffer::new(5)); } #[test] fn test_not_power_of_two() { - let mut rb = AllocRingBuffer::with_capacity_non_power_of_two(10); + let mut rb = AllocRingBuffer::new(10); const NUM_VALS: usize = 1000; // recycle the ringbuffer a bunch of time to see if noneof the logic @@ -503,12 +396,6 @@ mod tests { assert_eq!(b.capacity, 4); } - #[test] - #[should_panic] - fn test_with_capacity_no_power_of_two() { - let _ = AllocRingBuffer::::new(10); - } - #[test] #[should_panic] fn test_index_zero_length() { diff --git a/src/with_alloc/vecdeque.rs b/src/with_alloc/vecdeque.rs index 5c9a312..8dbd1f1 100644 --- a/src/with_alloc/vecdeque.rs +++ b/src/with_alloc/vecdeque.rs @@ -1,5 +1,4 @@ use crate::ringbuffer_trait::{RingBufferIntoIterator, RingBufferIterator, RingBufferMutIterator}; -use crate::with_alloc::alloc_ringbuffer::RingbufferSize; use crate::{AllocRingBuffer, RingBuffer}; use alloc::collections::VecDeque; use core::ops::{Deref, DerefMut, Index, IndexMut}; @@ -40,8 +39,8 @@ impl From<&[T]> for GrowableAllocRingBuffer { } } -impl From> for GrowableAllocRingBuffer { - fn from(mut v: AllocRingBuffer) -> GrowableAllocRingBuffer { +impl From> for GrowableAllocRingBuffer { + fn from(mut v: AllocRingBuffer) -> GrowableAllocRingBuffer { let mut rb = GrowableAllocRingBuffer::new(); rb.extend(v.drain()); rb diff --git a/src/with_const_generics.rs b/src/with_const_generics.rs index f9ea31a..e7eabd9 100644 --- a/src/with_const_generics.rs +++ b/src/with_const_generics.rs @@ -5,9 +5,6 @@ use core::mem; use core::mem::MaybeUninit; use core::ops::{Index, IndexMut}; -#[cfg(feature = "alloc")] -use crate::with_alloc::alloc_ringbuffer::RingbufferSize; - /// The `ConstGenericRingBuffer` struct is a `RingBuffer` implementation which does not require `alloc` but /// uses const generics instead. /// @@ -126,10 +123,8 @@ impl From> } #[cfg(feature = "alloc")] -impl From> - for ConstGenericRingBuffer -{ - fn from(mut value: crate::AllocRingBuffer) -> Self { +impl From> for ConstGenericRingBuffer { + fn from(mut value: crate::AllocRingBuffer) -> Self { value.drain().collect() } } diff --git a/tests/compile-fail/test_const_generic_array_zero_length_new.rs b/tests/compile-fail/test_const_generic_array_zero_length_new.rs index 24a3494..b080de1 100644 --- a/tests/compile-fail/test_const_generic_array_zero_length_new.rs +++ b/tests/compile-fail/test_const_generic_array_zero_length_new.rs @@ -1,6 +1,6 @@ extern crate ringbuffer; -use ringbuffer::{ConstGenericRingBuffer, RingBufferWrite}; +use ringbuffer::{ConstGenericRingBuffer, RingBuffer}; fn main() { let mut buf = ConstGenericRingBuffer::new::<0>(); diff --git a/tests/conversions.rs b/tests/conversions.rs index 54fba3d..e3c13a2 100644 --- a/tests/conversions.rs +++ b/tests/conversions.rs @@ -45,7 +45,7 @@ convert_tests!( alloc_from_cgrb: {let a = ConstGenericRingBuffer::from(['1', '2']); a}, alloc_from_garb: {let a = GrowableAllocRingBuffer::from(['1', '2']); a}, - ] => AllocRingBuffer::<_, _> + ] => AllocRingBuffer::<_> ); convert_tests!(