From 7ff9b854469ef2c1262c021b40af9f38ab27c7db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Bernier?= Date: Wed, 16 Sep 2026 18:11:25 -0400 Subject: [PATCH 1/2] Optimize VecDeque clone and clone_from Reuse existing element buffers and copy trivial elements in bulk. --- .../alloc/src/collections/vec_deque/mod.rs | 67 +++- library/alloctests/benches/vec_deque.rs | 135 +++++++++ library/alloctests/tests/lib.rs | 1 + library/alloctests/tests/vec_deque.rs | 286 +++++++++++++++++- 4 files changed, 479 insertions(+), 10 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs index 5bd43806b1a09..2baa329ec308c 100644 --- a/library/alloc/src/collections/vec_deque/mod.rs +++ b/library/alloc/src/collections/vec_deque/mod.rs @@ -119,17 +119,78 @@ pub struct VecDeque< impl Clone for VecDeque { fn clone(&self) -> Self { let mut deq = Self::with_capacity_in(self.len(), self.allocator().clone()); - deq.extend(self.iter().cloned()); + deq.spec_clone_from(self); deq } /// Overwrites the contents of `self` with a clone of the contents of `source`. /// /// This method is preferred over simply assigning `source.clone()` to `self`, - /// as it avoids reallocation if possible. + /// as it avoids reallocation if possible. Additionally, if the element type + /// `T` overrides `clone_from()`, this will reuse the resources of `self`'s + /// elements as well. fn clone_from(&mut self, source: &Self) { + self.spec_clone_from(source); + } +} + +// The trait is required to prevent internal details of the implementation leaking in rustdoc. +trait SpecCloneFrom { + fn spec_clone_from(&mut self, source: &Self); +} + +impl SpecCloneFrom for VecDeque { + default fn spec_clone_from(&mut self, source: &Self) { + self.truncate(source.len()); + + // We need to clone the overlapping elements in chunks given the deques may wrap at + // different points. + let (dst_front, dst_back) = self.as_mut_slices(); + let (mut src_front, mut src_back) = source.as_slices(); + for mut destination in [dst_front, dst_back] { + while !destination.is_empty() { + if src_front.is_empty() { + src_front = src_back; + src_back = &[]; + } + let len = cmp::min(destination.len(), src_front.len()); + debug_assert!(len > 0); + let (dst, dst_rest) = destination.split_at_mut(len); + let (src, src_rest) = src_front.split_at(len); + dst.clone_from_slice(src); + destination = dst_rest; + src_front = src_rest; + } + } + + self.extend(src_front.iter().chain(src_back).cloned()); + } +} + +impl SpecCloneFrom for VecDeque { + fn spec_clone_from(&mut self, source: &Self) { self.clear(); - self.extend(source.iter().cloned()); + self.reserve(source.len()); + + let (front, back) = source.as_slices(); + // SAFETY: + // - `TrivialClone` allows cloning by copying the bits. + // - `clear` dropped all destination elements, leaving no live values to overwrite. + // - The source slices are initialized, and all pointers are properly aligned for `T`. + // - `reserve` ensures capacity for `front.len() + back.len() == source.len()` + // elements, so both destination ranges and `dst.add(front.len())` are in bounds. + // - For non-ZSTs, the deques own distinct allocations, so the copied ranges do not + // overlap. For ZSTs, the copies and pointer offset have size zero in bytes. + unsafe { + let dst = self.ptr(); + ptr::copy_nonoverlapping(front.as_ptr(), dst, front.len()); + if !back.is_empty() { + ptr::copy_nonoverlapping(back.as_ptr(), dst.add(front.len()), back.len()); + } + } + // SAFETY: The copies initialized `source.len()` elements starting at index zero. + self.head = WrappedIndex::zero(); + self.len = source.len(); } } diff --git a/library/alloctests/benches/vec_deque.rs b/library/alloctests/benches/vec_deque.rs index a56f8496963bc..bbde9aeab94b0 100644 --- a/library/alloctests/benches/vec_deque.rs +++ b/library/alloctests/benches/vec_deque.rs @@ -22,6 +22,141 @@ fn bench_grow_1025(b: &mut Bencher) { }) } +/// first_len is the length of the first slice returned by as_slices +fn clone_fixture(value: &T, (len, first_len): (usize, usize)) -> VecDeque { + let mut deque = VecDeque::with_capacity(len); + let push_front = if first_len == len { 0 } else { first_len }; + deque.resize_with(len - push_front, || value.clone()); + for _ in 0..push_front { + deque.push_front(value.clone()); + } + let (first, second) = deque.as_slices(); + assert_eq!((first.len(), second.len()), (first_len, len - first_len)); + deque +} + +/// times allocation and drop as well as cloning +fn do_bench_clone(b: &mut Bencher, value: T, layout: (usize, usize)) { + let src = clone_fixture(&value, layout); + + b.iter(|| black_box(black_box(&src).clone())); +} + +fn do_bench_clone_batch_32(b: &mut Bencher, value: T, layout: (usize, usize)) { + let src = clone_fixture(&value, layout); + + b.iter(|| { + // keep all clones alive so the allocator can't reuse the same buffer for each clone + let clones: [VecDeque; 32] = std::array::from_fn(|_| black_box(&src).clone()); + black_box(&clones); + }); +} + +/// clone_from may make the destination contiguous even when the source is wrapped +fn do_bench_clone_from(b: &mut Bencher, value: T, layout: (usize, usize)) { + let src = clone_fixture(&value, layout); + let mut dst = clone_fixture(&value, layout); + + b.iter(|| { + dst.clone_from(black_box(&src)); + black_box(&dst); + }); +} + +/// shrink and grow through clone_from so the timing doesn't include a separate reset +fn do_bench_clone_from_alternating( + b: &mut Bencher, + value: T, + long: (usize, usize), + short: (usize, usize), +) { + let long_src = clone_fixture(&value, long); + let short_src = clone_fixture(&value, short); + let mut dst = clone_fixture(&value, long); + + b.iter(|| { + dst.clone_from(black_box(&short_src)); + dst.clone_from(black_box(&long_src)); + black_box(&dst); + }); +} + +fn do_bench_clone_from_empty(b: &mut Bencher, value: T, layout: (usize, usize)) { + let src = clone_fixture(&value, layout); + + b.iter(|| { + let mut dst = VecDeque::new(); + dst.clone_from(black_box(&src)); + black_box(dst); + }); +} + +macro_rules! clone_benches { + ($($name:ident, $value:expr, $layout:expr;)*) => { + $( + #[bench] + fn ${concat(bench_clone_, $name)}(b: &mut Bencher) { + do_bench_clone(b, $value, $layout); + } + + #[bench] + fn ${concat(bench_clone_from_, $name)}(b: &mut Bencher) { + do_bench_clone_from(b, $value, $layout); + } + )* + }; +} + +clone_benches! { + u64_empty, 42u64, (0, 0); + u64_one, 42u64, (1, 1); + u64_small, 42u64, (16, 16); + u64_small_wrapped, 42u64, (16, 5); + u64_contiguous, 42u64, (1024, 1024); + u64_wrapped, 42u64, (1024, 384); + string_contiguous, "abcdefgh".repeat(15), (1024, 1024); + string_wrapped, "abcdefgh".repeat(15), (1024, 384); + zst, (), (1024, 1024); +} + +#[bench] +fn bench_clone_u64_small_batch_32(b: &mut Bencher) { + do_bench_clone_batch_32(b, 42u64, (16, 16)); +} + +#[bench] +fn bench_clone_u64_small_wrapped_batch_32(b: &mut Bencher) { + do_bench_clone_batch_32(b, 42u64, (16, 5)); +} + +macro_rules! clone_from_benches { + ($($name:ident, $value:expr, $long:expr, $short:expr;)*) => { + $( + #[bench] + fn ${concat(bench_clone_from_, $name)}(b: &mut Bencher) { + do_bench_clone_from_alternating(b, $value, $long, $short); + } + )* + }; +} + +clone_from_benches! { + u64_alternating_contiguous, 42u64, (1024, 1024), (512, 512); + u64_alternating_wrapped, 42u64, (1024, 384), (512, 192); + string_alternating_contiguous, "abcdefgh".repeat(15), (1024, 1024), (512, 512); + string_alternating_wrapped, "abcdefgh".repeat(15), (1024, 384), (512, 192); +} + +#[bench] +fn bench_clone_from_u64_from_empty(b: &mut Bencher) { + do_bench_clone_from_empty(b, 42u64, (1024, 1024)); +} + +#[bench] +fn bench_clone_from_string_from_empty(b: &mut Bencher) { + do_bench_clone_from_empty(b, "abcdefgh".repeat(15), (1024, 1024)); +} + #[bench] fn bench_iter_1000(b: &mut Bencher) { let ring: VecDeque<_> = (0..1000).collect(); diff --git a/library/alloctests/tests/lib.rs b/library/alloctests/tests/lib.rs index b2a3e8f8f8e2a..fbb77a014b126 100644 --- a/library/alloctests/tests/lib.rs +++ b/library/alloctests/tests/lib.rs @@ -53,6 +53,7 @@ #![feature(test)] #![feature(thin_box)] #![feature(titlecase)] +#![feature(trivial_clone)] #![feature(trusted_len)] #![feature(try_reserve_kind)] #![feature(try_with_capacity)] diff --git a/library/alloctests/tests/vec_deque.rs b/library/alloctests/tests/vec_deque.rs index 00b2c2e34d569..2ac0e7b8bfda8 100644 --- a/library/alloctests/tests/vec_deque.rs +++ b/library/alloctests/tests/vec_deque.rs @@ -574,6 +574,25 @@ fn test_from_iter() { assert_eq!(deq.len(), 256); } +/// pushing everything to the front leaves a nonzero start index when there's spare capacity +fn clone_test_fixture( + len: usize, + push_front_count: usize, + capacity: usize, + mut element: impl FnMut(usize) -> T, +) -> VecDeque { + let mut deque = VecDeque::with_capacity(capacity); + deque.extend((push_front_count..len).map(&mut element)); + // pushing at both ends gives the same split even if capacity is rounded up + for i in (0..push_front_count).rev() { + deque.push_front(element(i)); + } + let first_len = if push_front_count == 0 { len } else { push_front_count }; + let (first, second) = deque.as_slices(); + assert_eq!((first.len(), second.len()), (first_len, len - first_len)); + deque +} + #[test] fn test_clone() { let mut d = VecDeque::new(); @@ -581,14 +600,267 @@ fn test_clone() { d.push_front(42); d.push_back(137); d.push_back(137); - assert_eq!(d.len(), 4); - let mut e = d.clone(); - assert_eq!(e.len(), 4); - while !d.is_empty() { - assert_eq!(d.pop_back(), e.pop_back()); + + let e = d.clone(); + assert_eq!(e, d); + // they should be disjoint in memory + assert!(d.as_slices().0.as_ptr() != e.as_slices().0.as_ptr()); +} + +#[test] +fn test_clone_from_reuses_element_allocations() { + for (case, (source_len, source_front), (destination_len, destination_front, capacity)) in [ + ("empty destination", (5, 2), (0, 0, 0)), + ("contiguous source", (5, 0), (5, 2, 8)), + ("contiguous destination", (5, 2), (5, 0, 8)), + ("source wraps first", (5, 1), (5, 3, 8)), + ("destination wraps first", (5, 3), (5, 1, 8)), + ("append across source wrap", (8, 3), (2, 1, 8)), + ("append within source back slice", (8, 2), (4, 1, 8)), + ("append after growing the destination", (8, 3), (2, 1, 2)), + ("truncate destination", (3, 1), (6, 4, 8)), + ] { + let source = clone_test_fixture(source_len, source_front, 8, |i| i.to_string()); + let mut destination = + clone_test_fixture(destination_len, destination_front, capacity, |i| { + String::with_capacity(32 + i) + }); + let capacities: Vec<_> = + destination.iter().take(source_len).map(String::capacity).collect(); + + destination.clone_from(&source); + + assert_eq!(destination, source, "{case}"); + // existing strings should keep their buffers, even if the deque has to grow + for (value, capacity) in destination.iter().zip(capacities) { + assert_eq!(value.capacity(), capacity, "{case}"); + } } - assert_eq!(d.len(), 0); - assert_eq!(e.len(), 0); +} + +#[test] +fn test_clone_trivial() { + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + #[repr(align(32))] + struct Elem([u64; 4]); + + for (case, source_len, source_front, source_capacity) in [ + ("empty", 0, 0, 0), + ("contiguous at buffer start", 5, 0, 8), + ("contiguous at buffer end", 5, 5, 8), + ("wrapped", 5, 2, 8), + ] { + let source = clone_test_fixture(source_len, source_front, source_capacity, |i| { + Elem([i as u64 + 1; 4]) + }); + assert_eq!(source.clone(), source, "{case}"); + + let mut destination = clone_test_fixture(4, 2, 8, |i| Elem([i as u64 + 100; 4])); + let capacity = destination.capacity(); + + destination.clone_from(&source); + + assert_eq!(destination, source, "{case}"); + assert_eq!(destination.capacity(), capacity, "{case}"); + } + + // force reallocation with a wrapped source + let mut destination = clone_test_fixture(2, 1, 2, |i| Elem([i as u64 + 100; 4])); + let source_len = destination.capacity() + 1; + let source = clone_test_fixture(source_len, 1, source_len, |i| Elem([i as u64 + 1; 4])); + + destination.clone_from(&source); + + assert_eq!(destination, source); +} + +#[test] +fn test_clone_trivial_drops_elements() { + #[derive(Clone)] + struct Elem<'a>(&'a Cell); + + // SAFETY: the derived Clone only copies a shared reference + unsafe impl std::clone::TrivialClone for Elem<'_> {} + + impl Drop for Elem<'_> { + fn drop(&mut self) { + self.0.update(|count| count + 1); + } + } + + let source_drops = [const { Cell::new(0) }; 3]; + let destination_drops = [const { Cell::new(0) }; 4]; + let source = clone_test_fixture(3, 1, 3, |id| Elem(&source_drops[id])); + let cloned = source.clone(); + assert_eq!(source_drops.each_ref().map(Cell::get), [0; 3]); + drop(cloned); + assert_eq!(source_drops.each_ref().map(Cell::get), [1; 3]); + + let mut destination = clone_test_fixture(4, 2, 4, |id| Elem(&destination_drops[id])); + + destination.clone_from(&source); + + assert_eq!(destination_drops.each_ref().map(Cell::get), [1; 4]); + assert_eq!(source_drops.each_ref().map(Cell::get), [1; 3]); + + drop(destination); + assert_eq!(source_drops.each_ref().map(Cell::get), [2; 3]); + drop(source); + assert_eq!(source_drops.each_ref().map(Cell::get), [3; 3]); +} + +#[test] +fn test_clone_copy_with_custom_clone() { + #[derive(Copy)] + struct Elem<'a> { + id: usize, + clones: &'a Cell, + } + + impl Clone for Elem<'_> { + fn clone(&self) -> Self { + self.clones.update(|count| count + 1); + *self + } + } + + let clones = Cell::new(0); + let element = |id| Elem { id, clones: &clones }; + let source = clone_test_fixture(5, 2, 5, &element); + + let cloned = source.clone(); + assert_eq!(cloned.iter().map(|element| element.id).collect::>(), [0, 1, 2, 3, 4]); + assert_eq!(clones.get(), 5); + + clones.set(0); + let mut destination = clone_test_fixture(3, 1, 3, |id| element(5 + id)); + destination.clone_from(&source); + assert_eq!(destination.iter().map(|element| element.id).collect::>(), [0, 1, 2, 3, 4]); + assert_eq!(clones.get(), 5); +} + +#[test] +#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] +fn test_clone_panic_drops_elements() { + // use None for clone and Some(len) for clone_from with an existing destination + for (case, destination_len, panic_index) in [ + ("new destination", None, 5), + ("overwrite after truncating", Some(12), 2), + ("append after overlap", Some(3), 5), + ] { + let clones = [const { Cell::new(0) }; 20]; + let drops = [const { Cell::new(0) }; 20]; + let element = |id| CloneTracker { + id, + clone: Some(&clones[id]), + drop: Some(&drops[id]), + panic: false, + }; + let mut source = clone_test_fixture(8, 3, 8, &element); + let mut destination = destination_len + .map(|len| clone_test_fixture(len, len - len / 2, len, |id| element(8 + id))); + let old_len = destination_len.unwrap_or(0); + source[panic_index].panic = true; + + catch_unwind(AssertUnwindSafe(|| match &mut destination { + Some(destination) => destination.clone_from(&source), + None => drop(source.clone()), + })) + .expect_err(case); + + // check for leaks and double drops before retrying the clone + let mut live = [0; 20]; + if let Some(destination) = &destination { + for element in destination { + live[element.id] += 1; + } + } + // the original destination elements weren't created by clone + for id in 0..8 + old_len { + assert_eq!( + drops[id].get() + live[id], + clones[id].get() + u32::from(id >= 8), + "{case}: element {id}" + ); + } + + // check that cloning still works after the panic + source[panic_index].panic = false; + let destination = match destination { + Some(mut destination) => { + destination.clone_from(&source); + destination + } + None => source.clone(), + }; + assert_eq!( + destination.iter().map(|element| element.id).collect::>(), + source.iter().map(|element| element.id).collect::>(), + "{case}" + ); + + drop(destination); + drop(source); + for id in 0..8 + old_len { + assert_eq!(drops[id].get(), 1 + clones[id].get(), "{case}: element {id}"); + } + } +} + +#[test] +fn test_clone_zero_sized() { + static CLONES: AtomicUsize = AtomicUsize::new(0); + static DROPS: AtomicUsize = AtomicUsize::new(0); + + // a custom Clone checks that zero-sized elements still get cloned individually + struct Counted; + + impl Clone for Counted { + fn clone(&self) -> Self { + CLONES.fetch_add(1, Ordering::Relaxed); + Self + } + } + + impl Drop for Counted { + fn drop(&mut self) { + DROPS.fetch_add(1, Ordering::Relaxed); + } + } + + #[derive(Clone, Copy)] + #[repr(align(32))] + struct Zst; + + let source = clone_test_fixture(3, 1, 3, |_| Zst); + let mut destination = source.clone(); + assert_eq!(destination.len(), 3); + destination.clone_from(&VecDeque::new()); + assert_eq!(destination.len(), 0); + destination.clone_from(&source); + assert_eq!(destination.len(), 3); + + let source: VecDeque<_> = (0..3).map(|_| Counted).collect(); + let mut destination = source.clone(); + assert_eq!(destination.len(), 3); + assert_eq!(CLONES.load(Ordering::Relaxed), 3); + assert_eq!(DROPS.load(Ordering::Relaxed), 0); + + destination.push_back(Counted); + destination.clone_from(&source); + assert_eq!(destination.len(), 3); + assert_eq!(CLONES.load(Ordering::Relaxed), 6); + // one truncated element and three replaced elements were dropped + assert_eq!(DROPS.load(Ordering::Relaxed), 4); + + destination.clone_from(&VecDeque::new()); + assert_eq!(destination.len(), 0); + assert_eq!(CLONES.load(Ordering::Relaxed), 6); + assert_eq!(DROPS.load(Ordering::Relaxed), 7); + + drop(destination); + drop(source); + assert_eq!(DROPS.load(Ordering::Relaxed), 10); } #[test] From f29ba157be130adf9bae370377c9aad500a38f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Bernier?= Date: Fri, 18 Sep 2026 21:17:01 -0400 Subject: [PATCH 2/2] Bring back altered tests --- library/alloctests/tests/vec_deque.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/library/alloctests/tests/vec_deque.rs b/library/alloctests/tests/vec_deque.rs index 2ac0e7b8bfda8..7a1d13d3be1f3 100644 --- a/library/alloctests/tests/vec_deque.rs +++ b/library/alloctests/tests/vec_deque.rs @@ -600,11 +600,14 @@ fn test_clone() { d.push_front(42); d.push_back(137); d.push_back(137); - - let e = d.clone(); - assert_eq!(e, d); - // they should be disjoint in memory - assert!(d.as_slices().0.as_ptr() != e.as_slices().0.as_ptr()); + assert_eq!(d.len(), 4); + let mut e = d.clone(); + assert_eq!(e.len(), 4); + while !d.is_empty() { + assert_eq!(d.pop_back(), e.pop_back()); + } + assert_eq!(d.len(), 0); + assert_eq!(e.len(), 0); } #[test] @@ -653,7 +656,12 @@ fn test_clone_trivial() { let source = clone_test_fixture(source_len, source_front, source_capacity, |i| { Elem([i as u64 + 1; 4]) }); - assert_eq!(source.clone(), source, "{case}"); + let mut cloned = source.clone(); + assert_eq!(cloned, source, "{case}"); + if source_len != 0 { + cloned[0] = Elem([u64::MAX; 4]); + assert_eq!(source[0], Elem([1; 4]), "{case}"); + } let mut destination = clone_test_fixture(4, 2, 8, |i| Elem([i as u64 + 100; 4])); let capacity = destination.capacity();