From 3d92e76ab0b734741b00a438b97977c8c22b11e0 Mon Sep 17 00:00:00 2001 From: cdellacqua Date: Fri, 7 Feb 2025 11:11:41 +0100 Subject: [PATCH 1/5] feat: add `extend_from_slice` --- benches/bench.rs | 66 ++++++++++++++++++ src/lib.rs | 137 +++++++++++++++++++++++++++++++++++++ src/ringbuffer_trait.rs | 93 +++++++++++++++++++++++++ src/with_alloc/vecdeque.rs | 7 ++ 4 files changed, 303 insertions(+) diff --git a/benches/bench.rs b/benches/bench.rs index 67f50d8..6161ca7 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -157,6 +157,44 @@ fn benchmark_copy_from_slice_vs_extend + SetLen, F: Fn() -> T group.finish(); } +fn benchmark_extend_from_slice_vs_extend, F: Fn() -> T>( + rb_size: usize, + rb_type: &str, + fn_name: &str, + c: &mut Criterion, + new: F, +) { + let mut group = c.benchmark_group(format!("{fn_name}({rb_type}, {rb_size})")); + let input = vec![9; rb_size]; + group.bench_function(format!("ExtendFromSlice({rb_type}; {rb_size})"), |b| { + let mut rb = new(); + rb.fill(9); + // making sure the read/write pointers wrap around + for _ in 0..rb_size / 2 { + let _ = rb.dequeue(); + let _ = rb.enqueue(6); + } + b.iter(|| { + rb.extend_from_slice(&input); + assert_eq!(input[input.len() / 2], 9); + }) + }); + group.bench_function(format!("ExtendFromIter({rb_type}; {rb_size})"), |b| { + let mut rb = new(); + rb.fill(9); + // making sure the read/write pointers wrap around + for _ in 0..rb_size / 2 { + let _ = rb.dequeue(); + let _ = rb.enqueue(9); + } + b.iter(|| { + rb.extend(input.iter().copied()); + assert_eq!(input[input.len() / 2], 9); + }) + }); + group.finish(); +} + macro_rules! generate_benches { (called, $c: tt, $rb: tt, $ty: tt, $fn: tt, $bmfunc: tt, $($i:tt),*) => { $( @@ -429,6 +467,34 @@ fn criterion_benchmark(c: &mut Criterion) { 1_000_000, 1_048_576 ]; + generate_benches![ + compare, + c, + AllocRingBuffer, + i32, + new, + benchmark_extend_from_slice_vs_extend, + 16, + 1024, + 4096, + 8192, + 1_000_000, + 1_048_576 + ]; + generate_benches![ + compare_typed, + c, + ConstGenericRingBuffer, + i32, + new, + benchmark_extend_from_slice_vs_extend, + 16, + 1024, + 4096, + 8192, + 1_000_000, + 1_048_576 + ]; } criterion_group!(benches, criterion_benchmark); diff --git a/src/lib.rs b/src/lib.rs index 23cafe7..a647051 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2059,4 +2059,141 @@ mod tests { rb }); } + + #[test] + fn test_extend_from_slice() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + // leave some space + rb.extend_from_slice(&[9; 3]); + assert_eq!(&rb.to_vec(), &[9; 3]); + // leave some space + rb.extend_from_slice(&[6; 5]); + assert_eq!(&rb.to_vec(), &[9, 9, 9, 6, 6, 6, 6, 6]); + // fill up remaining slots + rb.extend_from_slice(&[1; 2]); + assert_eq!(&rb.to_vec(), &[9, 9, 9, 6, 6, 6, 6, 6, 1, 1]); + assert!(&rb.is_full()); + // overwrite + rb.extend_from_slice(&[7; 2]); + assert_eq!(&rb.to_vec(), &[9, 6, 6, 6, 6, 6, 1, 1, 7, 7]); + }; + } + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(10)); + } + + #[test] + fn test_extend_from_slice_after_use() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[7, 8, 9]); + assert_eq!(&rb.to_vec(), &[4, 5, 6, 7, 8, 9]); + }; + } + + test_concrete!(|| { + let mut rb = ConstGenericRingBuffer::::new(); + let _ = rb.enqueue(4); + let _ = rb.enqueue(5); + let _ = rb.enqueue(6); + rb + }); + test_concrete!(|| { + let mut rb = AllocRingBuffer::::new(20); + let _ = rb.enqueue(4); + let _ = rb.enqueue(5); + let _ = rb.enqueue(6); + rb + }); + } + + #[test] + fn test_extend_from_slice_fill_capacity() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[9; 3]); + assert_eq!(&rb.to_vec(), &[9; 3]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(3)); + } + + #[test] + fn test_extend_from_slice_empty() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[]); + assert_eq!(&rb.to_vec(), &[0; 0]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| GrowableAllocRingBuffer::::with_capacity(1)); + test_concrete!(|| AllocRingBuffer::::new(1)); + } + + #[test] + fn test_extend_from_slice_wrap_around() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[1, 2, 3, 4]); + assert_eq!(&rb.to_vec(), &[4]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(1)); + } + + #[test] + fn test_extend_from_slice_wrap_around_overwriting() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[1, 2, 3, 4]); + rb.extend_from_slice(&[5, 6, 7, 8]); + assert_eq!(&rb.to_vec(), &[5, 6, 7, 8]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(4)); + } + + #[test] + fn test_extend_from_slice_wrap_around_partially_overwriting() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[1, 2, 3, 4]); + rb.extend_from_slice(&[5, 6]); + assert_eq!(&rb.to_vec(), &[3, 4, 5, 6]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(4)); + } + + #[test] + fn test_extend_from_slice_longer_than_capacity() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[1, 2, 3, 4, 6, 7, 8, 9, 10]); + assert_eq!(&rb.to_vec(), &[7, 8, 9, 10]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(4)); + } } diff --git a/src/ringbuffer_trait.rs b/src/ringbuffer_trait.rs index 046b845..9aab5bb 100644 --- a/src/ringbuffer_trait.rs +++ b/src/ringbuffer_trait.rs @@ -296,6 +296,45 @@ pub unsafe trait RingBuffer: { unsafe { Self::ptr_copy_from_slice(self, offset, src) } } + + /// Efficiently extend the ringbuffer with the content of a slice. + /// + /// Note: the semantics of this method are the same as `.extend`, + /// thus the slice can be longer than the capacity of the ringbuffer. + /// + /// # Safety + /// ONLY SAFE WHEN self is a *mut to to an implementor of `RingBuffer` + unsafe fn ptr_extend_from_slice(rb: *mut Self, src: &[T]) + where + T: Copy; + + /// Efficiently extend the ringbuffer with the content of a slice. + /// + /// Note: the semantics of this method are the same as `.extend`, + /// thus the slice can be longer than the capacity of the ringbuffer. + /// + /// # Examples + /// + /// ``` + /// use ringbuffer::AllocRingBuffer; + /// use crate::ringbuffer::RingBuffer; + /// + /// let mut rb = AllocRingBuffer::with_capacity(4); + /// rb.extend_from_slice(&[1, 2, 3]); + /// assert_eq!(&rb.to_vec(), &[1, 2, 3]); + /// rb.extend_from_slice(&[4, 5, 6]); + /// assert_eq!(&rb.to_vec(), &[3, 4, 5, 6]); + /// + /// let mut rb = AllocRingBuffer::with_capacity(4); + /// rb.extend_from_slice(&[1, 2, 3, 4, 5, 6, 7, 8]); + /// assert_eq!(&rb.to_vec(), &[5, 6, 7, 8]); + /// ``` + fn extend_from_slice(&mut self, src: &[T]) + where + T: Copy, + { + unsafe { Self::ptr_extend_from_slice(self, src) } + } } mod iter { @@ -689,5 +728,59 @@ macro_rules! impl_ringbuffer_ext { .copy_from_slice(&src[size - from_idx..]); } } + + unsafe fn ptr_extend_from_slice(rb: *mut Self, src: &[T]) + where + T: Copy, + { + let capacity = Self::ptr_capacity(rb); + + if src.len() == 0 { + return; + } + + let truncated_src = if src.len() > capacity { + &src[src.len() - capacity..] + } else { + src + }; + + let truncated_src_len = truncated_src.len(); + + let base: *mut T = $get_base_mut_ptr(rb); + let size = Self::ptr_buffer_size(rb); + + let from_idx = $mask(size, (*rb).$writeptr); + let to_idx = $mask(size, (*rb).$writeptr + truncated_src_len); + + if from_idx < to_idx { + unsafe { + // SAFETY: index has been modulo-ed to be within range + // to be within bounds + core::slice::from_raw_parts_mut(base.add(from_idx), truncated_src_len) + } + .copy_from_slice(truncated_src); + } else { + unsafe { + // SAFETY: index has been modulo-ed to be within range + // to be within bounds + core::slice::from_raw_parts_mut(base.add(from_idx), size - from_idx) + } + .copy_from_slice(&truncated_src[..size - from_idx]); + unsafe { + // SAFETY: index has been modulo-ed to be within range + // to be within bounds + core::slice::from_raw_parts_mut(base, to_idx) + } + .copy_from_slice(&truncated_src[size - from_idx..]); + } + + + let initially_available = capacity - Self::ptr_len(rb); + if truncated_src_len > initially_available { + (*rb).$readptr += truncated_src_len - initially_available; + } + (*rb).$writeptr += truncated_src_len; + } }; } diff --git a/src/with_alloc/vecdeque.rs b/src/with_alloc/vecdeque.rs index c195150..585ab98 100644 --- a/src/with_alloc/vecdeque.rs +++ b/src/with_alloc/vecdeque.rs @@ -317,6 +317,13 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { back[offset - first_len..].copy_from_slice(src); } } + + unsafe fn ptr_extend_from_slice(rb: *mut Self, src: &[T]) + where + T: Copy, + { + (*rb).0.extend(src.iter()); + } } impl Extend for GrowableAllocRingBuffer { From 1584c06e0dc852076c9d5d3a32c1a51e92193e87 Mon Sep 17 00:00:00 2001 From: cdellacqua Date: Fri, 7 Feb 2025 21:11:42 +0100 Subject: [PATCH 2/5] feat: add `drain_to_slice` --- benches/bench.rs | 80 ++++++++++++++++++++++++++++++++ src/lib.rs | 93 ++++++++++++++++++++++++++++++++++++-- src/ringbuffer_trait.rs | 81 +++++++++++++++++++++++++++++++++ src/with_alloc/vecdeque.rs | 18 ++++++++ 4 files changed, 269 insertions(+), 3 deletions(-) diff --git a/benches/bench.rs b/benches/bench.rs index 6161ca7..eb32ad0 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -195,6 +195,60 @@ fn benchmark_extend_from_slice_vs_extend, F: Fn() -> T>( group.finish(); } +fn benchmark_drain_to_slice_vs_drain, F: Fn() -> T>( + rb_size: usize, + rb_type: &str, + fn_name: &str, + c: &mut Criterion, + new: F, +) { + let mut group = c.benchmark_group(format!("{fn_name}({rb_type}, {rb_size})")); + let mut output = vec![0; rb_size]; + group.bench_function(&format!("DrainToSlice({rb_type}; {rb_size})"), |b| { + b.iter_batched( + || { + let mut rb = new(); + rb.fill(9); + // making sure the read/write pointers wrap around + for _ in 0..rb_size / 2 { + let _ = rb.dequeue(); + let _ = rb.enqueue(9); + } + rb + }, + |mut rb| { + rb.drain_to_slice(&mut output); + assert_eq!(output[output.len() / 2], 9); + }, + criterion::BatchSize::NumIterations(1), + ) + }); + let mut output = vec![0; rb_size]; + group.bench_function(format!("DrainIter({rb_type}; {rb_size})"), |b| { + b.iter_batched( + || { + let mut rb = new(); + rb.fill(9); + // making sure the read/write pointers wrap around + for _ in 0..rb_size / 2 { + let _ = rb.dequeue(); + let _ = rb.enqueue(9); + } + rb + }, + |mut rb| { + output + .iter_mut() + .zip(rb.drain()) + .for_each(|(dst, src)| *dst = src); + assert_eq!(output[output.len() / 2], 9); + }, + criterion::BatchSize::NumIterations(1), + ) + }); + group.finish(); +} + macro_rules! generate_benches { (called, $c: tt, $rb: tt, $ty: tt, $fn: tt, $bmfunc: tt, $($i:tt),*) => { $( @@ -495,6 +549,32 @@ fn criterion_benchmark(c: &mut Criterion) { 1_000_000, 1_048_576 ]; + generate_benches![ + compare, + c, + AllocRingBuffer, + i32, + new, + benchmark_drain_to_slice_vs_drain, + 16, + 1024, + 4096, + 8192, + 1_000_000, + 1_048_576 + ]; + generate_benches![ + compare_typed, + c, + ConstGenericRingBuffer, + i32, + new, + benchmark_drain_to_slice_vs_drain, + 16, + 1024, + 4096, + 8192 + ]; } criterion_group!(benches, criterion_benchmark); diff --git a/src/lib.rs b/src/lib.rs index a647051..9653db3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1701,7 +1701,7 @@ mod tests { macro_rules! test_concrete { ($rb_init: expr) => { let mut rb = $rb_init(); - rb.copy_from_slice(0, &[0; 0]); + rb.copy_from_slice(0, &[]); assert_eq!(rb.to_vec(), alloc::vec![]); }; } @@ -1914,7 +1914,7 @@ mod tests { let rb = $rb_init(); let mut slice = []; rb.copy_to_slice(0, &mut slice); - assert_eq!(slice.as_slice(), &[0; 0]); + assert_eq!(slice.as_slice(), &[]); }; } @@ -2130,7 +2130,7 @@ mod tests { ($rb_init: expr) => { let mut rb = $rb_init(); rb.extend_from_slice(&[]); - assert_eq!(&rb.to_vec(), &[0; 0]); + assert_eq!(&rb.to_vec(), &[]); }; } @@ -2196,4 +2196,91 @@ mod tests { test_concrete!(ConstGenericRingBuffer::::new); test_concrete!(|| AllocRingBuffer::::new(4)); } + + #[test] + fn test_drain_to_slice_fill_capacity() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[9; 3]); + let mut slice = [0; 3]; + rb.drain_to_slice(&mut slice); + assert_eq!(&slice, &[9; 3]); + rb.extend_from_slice(&[1, 2, 3]); + rb.drain_to_slice(&mut slice); + assert_eq!(&slice, &[1, 2, 3]); + assert_eq!(&rb.to_vec(), &[]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| GrowableAllocRingBuffer::::with_capacity(3)); + test_concrete!(|| AllocRingBuffer::::new(3)); + } + + #[test] + fn test_drain_to_slice_empty() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + let mut slice = []; + rb.drain_to_slice(&mut slice); + assert_eq!(&slice, &[]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| GrowableAllocRingBuffer::::with_capacity(1)); + test_concrete!(|| AllocRingBuffer::::new(1)); + } + + #[test] + fn test_drain_to_slice_wrap_around() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[1, 2, 3]); + rb.extend_from_slice(&[4, 5, 6]); + let mut slice = [0; 4]; + rb.drain_to_slice(&mut slice); + assert_eq!(&slice, &[3, 4, 5, 6]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(4)); + } + + #[test] + fn test_drain_to_slice_wrap_around_overwrite() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[1, 2, 3, 4]); + rb.extend_from_slice(&[5, 6, 7, 8]); + let mut slice = [0; 4]; + rb.drain_to_slice(&mut slice); + assert_eq!(&slice, &[5, 6, 7, 8]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(4)); + } + + #[test] + #[should_panic = "destination slice length (10) greater than buffer length (0)"] + fn test_drain_to_slice_longer_than_len() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + let mut slice = [0; 10]; + rb.drain_to_slice(&mut slice); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| GrowableAllocRingBuffer::::with_capacity(4)); + test_concrete!(|| AllocRingBuffer::::new(4)); + } } diff --git a/src/ringbuffer_trait.rs b/src/ringbuffer_trait.rs index 9aab5bb..14edb9f 100644 --- a/src/ringbuffer_trait.rs +++ b/src/ringbuffer_trait.rs @@ -335,6 +335,44 @@ pub unsafe trait RingBuffer: { unsafe { Self::ptr_extend_from_slice(self, src) } } + + /// Efficiently drain the ringbuffer, transferring items to a slice. + /// + /// # Panics + /// Panics if the `dst.len()` is greater than the buffer length. + /// + /// # Safety + /// ONLY SAFE WHEN self is a *mut to to an implementor of `RingBuffer` + unsafe fn ptr_drain_to_slice(rb: *mut Self, dst: &mut [T]) + where + T: Copy; + + /// Efficiently drain the ringbuffer, transferring items to a slice. + /// + /// # Panics + /// Panics if the `dst.len()` is greater than the buffer length. + /// + /// # Examples + /// + /// ``` + /// use ringbuffer::AllocRingBuffer; + /// use crate::ringbuffer::RingBuffer; + /// + /// let mut slice = vec![0; 3]; + /// let mut rb = AllocRingBuffer::from([1, 2, 3, 4]); + /// rb.drain_to_slice(&mut slice); + /// assert_eq!(&slice, &[1, 2, 3]); + /// assert_eq!(&rb.to_vec(), &[4]); + /// rb.drain_to_slice(&mut slice[..1]); + /// assert_eq!(&slice, &[4, 2, 3]); + /// assert_eq!(&rb.to_vec(), &[]); + /// ``` + fn drain_to_slice(&mut self, dst: &mut [T]) + where + T: Copy, + { + unsafe { Self::ptr_drain_to_slice(self, dst) } + } } mod iter { @@ -782,5 +820,48 @@ macro_rules! impl_ringbuffer_ext { } (*rb).$writeptr += truncated_src_len; } + + unsafe fn ptr_drain_to_slice(rb: *mut Self, dst: &mut [T]) + where + T: Copy, + { + let dst_len = dst.len(); + let len = Self::ptr_len(rb); + assert!( + dst_len <= len, + "destination slice length ({dst_len}) greater than buffer length ({len})" + ); + + if dst_len == 0 { + return; + } + + let base: *mut T = $get_base_mut_ptr(rb); + let size = Self::ptr_buffer_size(rb); + + let from_idx = $mask(size, (*rb).$readptr); + let to_idx = $mask(size, (*rb).$readptr + dst_len); + + if from_idx < to_idx { + dst.copy_from_slice(unsafe { + // SAFETY: index has been modulo-ed to be within range + // to be within bounds + core::slice::from_raw_parts(base.add(from_idx), dst_len) + }); + } else { + dst[..size - from_idx].copy_from_slice(unsafe { + // SAFETY: index has been modulo-ed to be within range + // to be within bounds + core::slice::from_raw_parts(base.add(from_idx), size - from_idx) + }); + dst[size - from_idx..].copy_from_slice(unsafe { + // SAFETY: index has been modulo-ed to be within range + // to be within bounds + core::slice::from_raw_parts(base, to_idx) + }); + } + + (*rb).$readptr += dst_len; + } }; } diff --git a/src/with_alloc/vecdeque.rs b/src/with_alloc/vecdeque.rs index 585ab98..3700438 100644 --- a/src/with_alloc/vecdeque.rs +++ b/src/with_alloc/vecdeque.rs @@ -324,6 +324,24 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { { (*rb).0.extend(src.iter()); } + + unsafe fn ptr_drain_to_slice(rb: *mut Self, dst: &mut [T]) + where + T: Copy, + { + let dst_len = dst.len(); + let len = Self::ptr_len(rb); + assert!( + dst_len <= len, + "destination slice length ({dst_len}) greater than buffer length ({len})" + ); + + dst.iter_mut() + .zip((*rb).0.drain(0..dst_len)) + .for_each(|(dst, src)| { + *dst = src; + }); + } } impl Extend for GrowableAllocRingBuffer { From 3c23263ec9dc7943442b91a71ba5ccb2e98a9eb1 Mon Sep 17 00:00:00 2001 From: cdellacqua Date: Tue, 12 Aug 2025 17:59:38 +0200 Subject: [PATCH 3/5] refactor: reuse copy_{from,to}_slice for {extend_from,drain_to}_slice + loosen slice size checks --- src/lib.rs | 140 ++++++++++++++++++++++++++++++++++++- src/ringbuffer_trait.rs | 103 +++++---------------------- src/with_alloc/vecdeque.rs | 22 +++--- 3 files changed, 165 insertions(+), 100 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9653db3..60c48bf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1696,6 +1696,47 @@ mod tests { }); } + #[test] + fn test_copy_from_slice_partial() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(&[3, 2, 1]); + assert_eq!(rb.capacity(), 7); + // we have some space left + assert!(rb.len() < rb.capacity()); + + // copy preserves length + rb.copy_from_slice(0, &[1, 2]); + assert_eq!(rb.to_vec(), alloc::vec![1, 2, 1]); + + let _ = rb.enqueue(4); + let _ = rb.enqueue(5); + let _ = rb.enqueue(6); + assert_eq!(rb.to_vec(), alloc::vec![1, 2, 1, 4, 5, 6]); + + // still preserving length + rb.copy_from_slice(1, &[5, 4, 3, 2, 1]); + assert_eq!(rb.to_vec(), alloc::vec![1, 5, 4, 3, 2, 1]); + }; + } + + test_concrete!(|values: &[i32]| { + let mut rb = ConstGenericRingBuffer::<_, 7>::new(); + rb.extend(values.iter().copied()); + rb + }); + test_concrete!(|values: &[i32]| { + let mut rb = GrowableAllocRingBuffer::<_>::with_capacity(7); + rb.extend(values.iter().copied()); + rb + }); + test_concrete!(|values: &[i32]| { + let mut rb = AllocRingBuffer::<_>::new(7); + rb.extend(values.iter().copied()); + rb + }); + } + #[test] fn test_copy_from_slice_empty() { macro_rules! test_concrete { @@ -1907,6 +1948,60 @@ mod tests { }); } + #[test] + fn test_copy_to_slice_partial() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(&[1, 2, 3]); + assert_eq!(rb.capacity(), 7); + // we have some space left + assert!(rb.len() < rb.capacity()); + + // copy based on length + let mut slice = [0; 2]; + rb.copy_to_slice(0, &mut slice); + assert_eq!(slice.as_slice(), &[1, 2]); + + let _ = rb.enqueue(4); + let _ = rb.enqueue(5); + let _ = rb.enqueue(6); + // still based on length + let mut slice = [0; 5]; + rb.copy_to_slice(0, &mut slice); + assert_eq!(slice.as_slice(), &[1, 2, 3, 4, 5]); + + // making sure the read/write ptrs have traversed the ring + for i in 0..6 { + let _ = rb.enqueue(i + 1); + let _ = rb.dequeue(); + } + + // sanity check + assert_eq!(rb.to_vec(), alloc::vec![1, 2, 3, 4, 5, 6]); + // copy again + let mut slice = [0; 5]; + rb.copy_to_slice(1, &mut slice); + assert_eq!(slice.as_slice(), &[2, 3, 4, 5, 6]); + }; + } + + test_concrete!(|values: &[i32]| { + let mut rb = ConstGenericRingBuffer::<_, 7>::new(); + rb.extend(values.iter().copied()); + rb + }); + test_concrete!(|values: &[i32]| { + let mut rb = GrowableAllocRingBuffer::<_>::with_capacity(7); + rb.extend(values.iter().copied()); + rb + }); + test_concrete!(|values: &[i32]| { + let mut rb = AllocRingBuffer::<_>::new(7); + rb.extend(values.iter().copied()); + rb + }); + } + #[test] fn test_copy_to_slice_empty() { macro_rules! test_concrete { @@ -2124,6 +2219,20 @@ mod tests { test_concrete!(|| AllocRingBuffer::::new(3)); } + #[test] + fn test_extend_from_slice_partial() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[9; 2]); + assert_eq!(&rb.to_vec(), &[9, 9]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| AllocRingBuffer::::new(3)); + } + #[test] fn test_extend_from_slice_empty() { macro_rules! test_concrete { @@ -2218,6 +2327,32 @@ mod tests { test_concrete!(|| AllocRingBuffer::::new(3)); } + #[test] + fn test_drain_to_slice_partial() { + macro_rules! test_concrete { + ($rb_init: expr, $growable: expr) => { + let mut rb = $rb_init(); + rb.extend_from_slice(&[9; 3]); + let mut slice = [0; 2]; + rb.drain_to_slice(&mut slice); + assert_eq!(&slice, &[9; 2]); + rb.extend_from_slice(&[1, 2, 3]); + rb.drain_to_slice(&mut slice); + if $growable { + assert_eq!(&slice, &[9, 1]); + assert_eq!(&rb.to_vec(), &[2, 3]); + } else { + assert_eq!(&slice, &[1, 2]); + assert_eq!(&rb.to_vec(), &[3]); + } + }; + } + + test_concrete!(ConstGenericRingBuffer::::new, false); + test_concrete!(|| GrowableAllocRingBuffer::::with_capacity(3), true); + test_concrete!(|| AllocRingBuffer::::new(3), false); + } + #[test] fn test_drain_to_slice_empty() { macro_rules! test_concrete { @@ -2269,13 +2404,16 @@ mod tests { } #[test] - #[should_panic = "destination slice length (10) greater than buffer length (0)"] fn test_drain_to_slice_longer_than_len() { macro_rules! test_concrete { ($rb_init: expr) => { let mut rb = $rb_init(); let mut slice = [0; 10]; rb.drain_to_slice(&mut slice); + assert_eq!(slice, [0; 10]); + rb.extend_from_slice(&[1, 2]); + rb.drain_to_slice(&mut slice); + assert_eq!(slice, [1, 2, 0, 0, 0, 0, 0, 0, 0, 0]); }; } diff --git a/src/ringbuffer_trait.rs b/src/ringbuffer_trait.rs index 14edb9f..f43c7d9 100644 --- a/src/ringbuffer_trait.rs +++ b/src/ringbuffer_trait.rs @@ -257,7 +257,7 @@ pub unsafe trait RingBuffer: /// Efficiently copy items from the ringbuffer to a target slice. /// /// # Panics - /// Panics if the buffer length minus the offset is NOT equal to `target.len()`. + /// Panics if the buffer length minus the offset is smaller than `dst.len()`. /// /// # Safety /// ONLY SAFE WHEN self is a *const to to an implementor of `RingBuffer` @@ -268,7 +268,7 @@ pub unsafe trait RingBuffer: /// Efficiently copy items from the ringbuffer to a target slice. /// /// # Panics - /// Panics if the buffer length minus the offset is NOT equal to `target.len()`. + /// Panics if the buffer length minus the offset is smaller than `dst.len()`. fn copy_to_slice(&self, offset: usize, dst: &mut [T]) where T: Copy, @@ -277,8 +277,9 @@ pub unsafe trait RingBuffer: } /// Efficiently copy items from a slice to the ringbuffer. + /// /// # Panics - /// Panics if the buffer length minus the offset is NOT equal to `source.len()`. + /// Panics if the buffer length minus the offset is smaller than `src.len()`. /// /// # Safety /// ONLY SAFE WHEN self is a *mut to to an implementor of `RingBuffer` @@ -289,7 +290,7 @@ pub unsafe trait RingBuffer: /// Efficiently copy items from a slice to the ringbuffer. /// /// # Panics - /// Panics if the buffer length minus the offset is NOT equal to `source.len()`. + /// Panics if the buffer length minus the offset is smaller than `src.len()`. fn copy_from_slice(&mut self, offset: usize, src: &[T]) where T: Copy, @@ -688,7 +689,7 @@ macro_rules! impl_ringbuffer_ext { (offset == 0 && len == 0) || offset < len, "offset ({offset}) is out of bounds for the current buffer length ({len})" ); - assert!(len - offset == dst_len, "destination slice length ({dst_len}) doesn't match buffer length ({len}) when considering the specified offset ({offset})"); + assert!(len - offset >= dst_len, "destination slice length ({dst_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); if dst_len == 0 { return; @@ -731,7 +732,7 @@ macro_rules! impl_ringbuffer_ext { (offset == 0 && len == 0) || offset < len, "offset ({offset}) is out of bounds for the current buffer length ({len})" ); - assert!(len - offset == src_len, "source slice length ({src_len}) doesn't match buffer length ({len}) when considering the specified offset ({offset})"); + assert!(len - offset >= src_len, "source slice length ({src_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); if src_len == 0 { return; @@ -771,97 +772,25 @@ macro_rules! impl_ringbuffer_ext { where T: Copy, { + let len = Self::ptr_len(rb); let capacity = Self::ptr_capacity(rb); + (*rb).$writeptr += src.len(); + (*rb).$readptr = (*rb).$writeptr - (len + src.len()).min(capacity); + let final_len = Self::ptr_len(rb); - if src.len() == 0 { - return; - } - - let truncated_src = if src.len() > capacity { - &src[src.len() - capacity..] - } else { - src - }; - - let truncated_src_len = truncated_src.len(); - - let base: *mut T = $get_base_mut_ptr(rb); - let size = Self::ptr_buffer_size(rb); - - let from_idx = $mask(size, (*rb).$writeptr); - let to_idx = $mask(size, (*rb).$writeptr + truncated_src_len); - - if from_idx < to_idx { - unsafe { - // SAFETY: index has been modulo-ed to be within range - // to be within bounds - core::slice::from_raw_parts_mut(base.add(from_idx), truncated_src_len) - } - .copy_from_slice(truncated_src); - } else { - unsafe { - // SAFETY: index has been modulo-ed to be within range - // to be within bounds - core::slice::from_raw_parts_mut(base.add(from_idx), size - from_idx) - } - .copy_from_slice(&truncated_src[..size - from_idx]); - unsafe { - // SAFETY: index has been modulo-ed to be within range - // to be within bounds - core::slice::from_raw_parts_mut(base, to_idx) - } - .copy_from_slice(&truncated_src[size - from_idx..]); - } - - - let initially_available = capacity - Self::ptr_len(rb); - if truncated_src_len > initially_available { - (*rb).$readptr += truncated_src_len - initially_available; - } - (*rb).$writeptr += truncated_src_len; + let src_offset = src.len().saturating_sub(capacity); + Self::ptr_copy_from_slice(rb, final_len - (src.len() - src_offset), &src[src_offset..]); } unsafe fn ptr_drain_to_slice(rb: *mut Self, dst: &mut [T]) where T: Copy, { - let dst_len = dst.len(); let len = Self::ptr_len(rb); - assert!( - dst_len <= len, - "destination slice length ({dst_len}) greater than buffer length ({len})" - ); - - if dst_len == 0 { - return; - } - - let base: *mut T = $get_base_mut_ptr(rb); - let size = Self::ptr_buffer_size(rb); - - let from_idx = $mask(size, (*rb).$readptr); - let to_idx = $mask(size, (*rb).$readptr + dst_len); - - if from_idx < to_idx { - dst.copy_from_slice(unsafe { - // SAFETY: index has been modulo-ed to be within range - // to be within bounds - core::slice::from_raw_parts(base.add(from_idx), dst_len) - }); - } else { - dst[..size - from_idx].copy_from_slice(unsafe { - // SAFETY: index has been modulo-ed to be within range - // to be within bounds - core::slice::from_raw_parts(base.add(from_idx), size - from_idx) - }); - dst[size - from_idx..].copy_from_slice(unsafe { - // SAFETY: index has been modulo-ed to be within range - // to be within bounds - core::slice::from_raw_parts(base, to_idx) - }); - } + let truncated_dst_len = dst.len().min(len); + Self::ptr_copy_to_slice(rb, 0, &mut dst[..truncated_dst_len]); - (*rb).$readptr += dst_len; + (*rb).$readptr += truncated_dst_len; } }; } diff --git a/src/with_alloc/vecdeque.rs b/src/with_alloc/vecdeque.rs index 3700438..ca10775 100644 --- a/src/with_alloc/vecdeque.rs +++ b/src/with_alloc/vecdeque.rs @@ -266,7 +266,7 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { (offset == 0 && len == 0) || offset < len, "offset ({offset}) is out of bounds for the current buffer length ({len})" ); - assert!(len - offset == dst_len, "destination slice length ({dst_len}) doesn't match buffer length ({len}) when considering the specified offset ({offset})"); + assert!(len - offset >= dst_len, "destination slice length ({dst_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); if dst_len == 0 { return; @@ -277,13 +277,14 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { if offset < first_len { let n_in_first = first_len - offset; - dst[..n_in_first].copy_from_slice(&front[offset..]); + dst[..n_in_first.min(dst_len)] + .copy_from_slice(&front[offset..][..n_in_first.min(dst_len)]); if n_in_first < dst_len { dst[n_in_first..].copy_from_slice(&back[..dst_len - n_in_first]); } } else { - dst.copy_from_slice(&back[offset - first_len..]); + dst.copy_from_slice(&back[offset - first_len..][..dst_len]); } } @@ -297,7 +298,7 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { (offset == 0 && len == 0) || offset < len, "offset ({offset}) is out of bounds for the current buffer length ({len})" ); - assert!(len - offset == src_len, "source slice length ({src_len}) doesn't match buffer length ({len}) when considering the specified offset ({offset})"); + assert!(len - offset >= src_len, "source slice length ({src_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); if src_len == 0 { return; @@ -308,13 +309,14 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { if offset < first_len { let n_in_first = first_len - offset; - front[offset..].copy_from_slice(&src[..n_in_first]); + front[offset..][..n_in_first.min(src_len)] + .copy_from_slice(&src[..n_in_first.min(src_len)]); if n_in_first < src_len { back[..src_len - n_in_first].copy_from_slice(&src[n_in_first..]); } } else { - back[offset - first_len..].copy_from_slice(src); + back[offset - first_len..][..src_len].copy_from_slice(src); } } @@ -329,15 +331,11 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { where T: Copy, { - let dst_len = dst.len(); let len = Self::ptr_len(rb); - assert!( - dst_len <= len, - "destination slice length ({dst_len}) greater than buffer length ({len})" - ); + let truncated_dst_len = dst.len().min(len); dst.iter_mut() - .zip((*rb).0.drain(0..dst_len)) + .zip((*rb).0.drain(0..truncated_dst_len)) .for_each(|(dst, src)| { *dst = src; }); From 783f6ddebd462b973883b91695ec09c10715d017 Mon Sep 17 00:00:00 2001 From: cdellacqua Date: Wed, 13 Aug 2025 00:33:36 +0200 Subject: [PATCH 4/5] fix: allow offset to be equal to the buffer length when the source or target slice is empty --- src/lib.rs | 33 +++++++++++++++++++++++++++++++++ src/ringbuffer_trait.rs | 4 ++-- src/with_alloc/vecdeque.rs | 4 ++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 60c48bf..c09136d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2248,6 +2248,22 @@ mod tests { test_concrete!(|| AllocRingBuffer::::new(1)); } + #[test] + fn test_extend_from_slice_empty_alt() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + let _ = rb.enqueue(1); + rb.extend_from_slice(&[]); + assert_eq!(&rb.to_vec(), &[1]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| GrowableAllocRingBuffer::::with_capacity(1)); + test_concrete!(|| AllocRingBuffer::::new(1)); + } + #[test] fn test_extend_from_slice_wrap_around() { macro_rules! test_concrete { @@ -2369,6 +2385,23 @@ mod tests { test_concrete!(|| AllocRingBuffer::::new(1)); } + #[test] + fn test_drain_to_slice_empty_alt() { + macro_rules! test_concrete { + ($rb_init: expr) => { + let mut rb = $rb_init(); + let _ = rb.enqueue(1); + let mut slice = []; + rb.drain_to_slice(&mut slice); + assert_eq!(&slice, &[]); + }; + } + + test_concrete!(ConstGenericRingBuffer::::new); + test_concrete!(|| GrowableAllocRingBuffer::::with_capacity(1)); + test_concrete!(|| AllocRingBuffer::::new(1)); + } + #[test] fn test_drain_to_slice_wrap_around() { macro_rules! test_concrete { diff --git a/src/ringbuffer_trait.rs b/src/ringbuffer_trait.rs index f43c7d9..4763546 100644 --- a/src/ringbuffer_trait.rs +++ b/src/ringbuffer_trait.rs @@ -686,7 +686,7 @@ macro_rules! impl_ringbuffer_ext { let len = Self::ptr_len(rb); let dst_len = dst.len(); assert!( - (offset == 0 && len == 0) || offset < len, + offset < len || (offset == len && dst_len == 0), "offset ({offset}) is out of bounds for the current buffer length ({len})" ); assert!(len - offset >= dst_len, "destination slice length ({dst_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); @@ -729,7 +729,7 @@ macro_rules! impl_ringbuffer_ext { let len = Self::ptr_len(rb); let src_len = src.len(); assert!( - (offset == 0 && len == 0) || offset < len, + offset < len || (offset == len && src_len == 0), "offset ({offset}) is out of bounds for the current buffer length ({len})" ); assert!(len - offset >= src_len, "source slice length ({src_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); diff --git a/src/with_alloc/vecdeque.rs b/src/with_alloc/vecdeque.rs index ca10775..f5360ff 100644 --- a/src/with_alloc/vecdeque.rs +++ b/src/with_alloc/vecdeque.rs @@ -263,7 +263,7 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { let len = Self::ptr_len(rb); let dst_len = dst.len(); assert!( - (offset == 0 && len == 0) || offset < len, + offset < len || (offset == len && dst_len == 0), "offset ({offset}) is out of bounds for the current buffer length ({len})" ); assert!(len - offset >= dst_len, "destination slice length ({dst_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); @@ -295,7 +295,7 @@ unsafe impl RingBuffer for GrowableAllocRingBuffer { let len = Self::ptr_len(rb); let src_len = src.len(); assert!( - (offset == 0 && len == 0) || offset < len, + offset < len || (offset == len && src_len == 0), "offset ({offset}) is out of bounds for the current buffer length ({len})" ); assert!(len - offset >= src_len, "source slice length ({src_len}) greater than buffer length ({len}) when considering the specified offset ({offset})"); From 134b0641a28099df1668d5faae13773e0847b2dd Mon Sep 17 00:00:00 2001 From: cdellacqua Date: Wed, 13 Aug 2025 17:13:01 +0200 Subject: [PATCH 5/5] fix: remove Panics section that no longer applies --- src/ringbuffer_trait.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/ringbuffer_trait.rs b/src/ringbuffer_trait.rs index 4763546..a173042 100644 --- a/src/ringbuffer_trait.rs +++ b/src/ringbuffer_trait.rs @@ -339,9 +339,6 @@ pub unsafe trait RingBuffer: /// Efficiently drain the ringbuffer, transferring items to a slice. /// - /// # Panics - /// Panics if the `dst.len()` is greater than the buffer length. - /// /// # Safety /// ONLY SAFE WHEN self is a *mut to to an implementor of `RingBuffer` unsafe fn ptr_drain_to_slice(rb: *mut Self, dst: &mut [T]) @@ -350,9 +347,6 @@ pub unsafe trait RingBuffer: /// Efficiently drain the ringbuffer, transferring items to a slice. /// - /// # Panics - /// Panics if the `dst.len()` is greater than the buffer length. - /// /// # Examples /// /// ```