From 2a176317a18ab2d8e13fbcfe361d14a4cd9f1097 Mon Sep 17 00:00:00 2001 From: Barak Ugav Date: Mon, 6 Jul 2026 20:42:43 +0300 Subject: [PATCH 1/5] Add ArrayVec::clone() benchmark --- Cargo.toml | 4 ++++ benches/clone.rs | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 benches/clone.rs diff --git a/Cargo.toml b/Cargo.toml index 8471a259..3496e76d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,10 @@ harness = false name = "arraystring" harness = false +[[bench]] +name = "clone" +harness = false + [features] default = ["std"] std = [] diff --git a/benches/clone.rs b/benches/clone.rs new file mode 100644 index 00000000..f380c447 --- /dev/null +++ b/benches/clone.rs @@ -0,0 +1,43 @@ + +extern crate arrayvec; +#[macro_use] extern crate bencher; + +use arrayvec::ArrayVec; + +use bencher::Bencher; +use bencher::black_box; + +fn clone_x(b: &mut Bencher) { + let mut v = ArrayVec::::new(); + v.extend((0..N).map(|x| x as u8)); + b.iter(|| { + black_box(v.clone()) + }); + b.bytes = v.len() as u64; +} + +fn clone_8(b: &mut Bencher) { + clone_x::<8>(b); +} +fn clone_16(b: &mut Bencher) { + clone_x::<16>(b); +} +fn clone_32(b: &mut Bencher) { + clone_x::<32>(b); +} +fn clone_64(b: &mut Bencher) { + clone_x::<64>(b); +} +fn clone_512(b: &mut Bencher) { + clone_x::<512>(b); +} + +benchmark_group!(benches, + clone_8, + clone_16, + clone_32, + clone_64, + clone_512 +); + +benchmark_main!(benches); From 8a29e20a8f4cfddcac3ffc4dadc8c675e03c965c Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Thu, 9 Jul 2026 13:50:15 +0200 Subject: [PATCH 2/5] tests: Add test for .clone() --- tests/tests.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index 32bdd23a..309ceb8b 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -27,6 +27,27 @@ fn test_simple() { assert_eq!(sum_len, 8); } +#[test] +fn test_clone_simple() { + let data = [1, 2, 3, 4, 5, 6, 7]; + let mut vec = ArrayVec::from(data); + vec.pop(); + + let u = vec.clone(); + assert_eq!(vec, u); + assert_eq!(&vec, &data[..data.len() - 1]); +} + +#[test] +fn test_clone_complex() { + let mut vec: ArrayVec, 3> = ArrayVec::new(); + + vec.push(vec![1, 2, 3, 4]); + vec.push(vec![10]); + let u = vec.clone(); + assert_eq!(vec, u); +} + #[test] fn test_capacity_left() { let mut vec: ArrayVec = ArrayVec::new(); From 16189be706a398076157541b24a83e667d4a7875 Mon Sep 17 00:00:00 2001 From: Barak Ugav Date: Wed, 8 Jul 2026 09:32:57 +0300 Subject: [PATCH 3/5] Copy past len in Clone for simpler asm --- src/arrayvec.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/arrayvec.rs b/src/arrayvec.rs index eec77328..ef46ca0c 100644 --- a/src/arrayvec.rs +++ b/src/arrayvec.rs @@ -1193,7 +1193,62 @@ impl Clone for ArrayVec where T: Clone { fn clone(&self) -> Self { - self.iter().cloned().collect() + let mut array: ArrayVec = ArrayVec::new(); + { + let mut guard = ScopeExitGuard { + value: &mut array.len, + data: 0 as LenUint, + f: move |&len, self_len| { + **self_len = len; + }, + }; + + for i in 0..CAP { + if i < self.len() { + let val = unsafe { &*self.xs[i].as_ptr() }.clone(); + if mem::size_of::() != 0 { + unsafe { array.xs[i].as_mut_ptr().write(val) }; + } else { + // The ZST element has logically been moved into the vector. + // There is no memory to write, but dropping `elt` here would + // drop it once now and once again when the vector is dropped. + mem::forget(val); + } + guard.data += 1; + } else { + // we are done copying all the elements. + // we continue to copy uninitialized elements past len up to CAP. This seems + // weird and counter intuitive, but when T is trivial (like i8/i32), copying the + // whole array is faster as the compiler doesnt have to loop up to len, and the + // generated code is branchless copy of the whole struct (like Copy). + // + // We only do it if; + // - T does not requires drop, which we take as an indication that T::clone() + // is not trivial and therefore the loop will not be optimized away by the + // compiler. + // - size_of > 0, to avoid reading self.xs[i].as_ptr() + // - the total array is less or equal to 128 bytes. An arbitrary threshold to + // avoid unnecessary copying of large arrays. + if mem::needs_drop::() + || mem::size_of::() == 0 + || CAP > 128 / mem::size_of::() + { + break; + } + // Safety: copy of MaybeUninit to MaybeUninit + unsafe { ptr::copy_nonoverlapping(&self.xs[i], &mut array.xs[i], 1) }; + } + } + } + + // This assignment seems redundant as guard.data is already equal to len and will set the + // array len on drop, but setting it here explicitly helps the compiler understand it + // can just copy the len instead of accumulating it in the guard. This is especially + // useful for T::clone() that can not panic. + debug_assert_eq!(array.len, self.len); + array.len = self.len; + + array } fn clone_from(&mut self, rhs: &Self) { From 4a7fb2d75980cd54578588602868e85a0ca57a5e Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Thu, 9 Jul 2026 14:36:43 +0200 Subject: [PATCH 4/5] clone: Simplify optimization Desired feature is that clone of ArrayVec should look like a simple struct copy (regardless of how full the arrayvec is). Simplify the clone implementation while preserving this. Scope guard for length not found to be necessary. Write element and length possible but it is equivalent to push_unchecked which also worked here. --- src/arrayvec.rs | 85 ++++++++++++++++++++----------------------------- 1 file changed, 35 insertions(+), 50 deletions(-) diff --git a/src/arrayvec.rs b/src/arrayvec.rs index ef46ca0c..40e97340 100644 --- a/src/arrayvec.rs +++ b/src/arrayvec.rs @@ -1193,61 +1193,46 @@ impl Clone for ArrayVec where T: Clone { fn clone(&self) -> Self { - let mut array: ArrayVec = ArrayVec::new(); - { - let mut guard = ScopeExitGuard { - value: &mut array.len, - data: 0 as LenUint, - f: move |&len, self_len| { - **self_len = len; - }, - }; + let mut array = Self::new(); - for i in 0..CAP { - if i < self.len() { - let val = unsafe { &*self.xs[i].as_ptr() }.clone(); - if mem::size_of::() != 0 { - unsafe { array.xs[i].as_mut_ptr().write(val) }; - } else { - // The ZST element has logically been moved into the vector. - // There is no memory to write, but dropping `elt` here would - // drop it once now and once again when the vector is dropped. - mem::forget(val); - } - guard.data += 1; - } else { - // we are done copying all the elements. - // we continue to copy uninitialized elements past len up to CAP. This seems - // weird and counter intuitive, but when T is trivial (like i8/i32), copying the - // whole array is faster as the compiler doesnt have to loop up to len, and the - // generated code is branchless copy of the whole struct (like Copy). - // - // We only do it if; - // - T does not requires drop, which we take as an indication that T::clone() - // is not trivial and therefore the loop will not be optimized away by the - // compiler. - // - size_of > 0, to avoid reading self.xs[i].as_ptr() - // - the total array is less or equal to 128 bytes. An arbitrary threshold to - // avoid unnecessary copying of large arrays. - if mem::needs_drop::() - || mem::size_of::() == 0 - || CAP > 128 / mem::size_of::() - { - break; - } - // Safety: copy of MaybeUninit to MaybeUninit - unsafe { ptr::copy_nonoverlapping(&self.xs[i], &mut array.xs[i], 1) }; + for i in 0..CAP { + if i < self.len() { + // Safety: known to not exceed capacity + unsafe { + array.push_unchecked(self[i].clone()); } - } + } else { + // we are done copying all the elements. + // we continue to copy uninitialized elements past len up to CAP. This seems + // weird and counter intuitive, but when T is trivial (like i8/i32), copying the + // whole array is faster as the compiler doesnt have to loop up to len, and the + // generated code is branchless copy of the whole struct (like Copy). + // + // We only do it if: + // - T does not require drop, which we take as an indication that T::clone() + // is not trivial and therefore the loop will not be optimized away by the + // compiler. + // - size_of > 0, to avoid reading self.xs[i].as_ptr() + // - the total array is less or equal to 128 bytes. An arbitrary threshold to + // avoid unnecessary copying of large arrays. + if mem::needs_drop::() + || mem::size_of::() == 0 + || CAP > 128 / mem::size_of::() + { + break; + } + + // Safety: copy as MaybeUninit + unsafe { + ptr::copy_nonoverlapping(&self.xs[i], &mut array.xs[i], 1); + } + }; } - // This assignment seems redundant as guard.data is already equal to len and will set the - // array len on drop, but setting it here explicitly helps the compiler understand it - // can just copy the len instead of accumulating it in the guard. This is especially - // useful for T::clone() that can not panic. - debug_assert_eq!(array.len, self.len); + // This assignment helps the compiler understand it can just copy the len instead + // of using the accumulated value. + // This is especially useful for T::clone() that can not panic. array.len = self.len; - array } From fed684172b6d6b55fca92ab24039641cb9975cc0 Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Thu, 9 Jul 2026 14:36:43 +0200 Subject: [PATCH 5/5] tests: Add codegen test for clone --- tests/codegen/clone_copy.rs | 8 ++++++ tests/codegen/justfile | 51 +++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 tests/codegen/clone_copy.rs create mode 100644 tests/codegen/justfile diff --git a/tests/codegen/clone_copy.rs b/tests/codegen/clone_copy.rs new file mode 100644 index 00000000..c330397f --- /dev/null +++ b/tests/codegen/clone_copy.rs @@ -0,0 +1,8 @@ +extern crate arrayvec; +use arrayvec::ArrayVec; + +#[unsafe(no_mangle)] +#[inline(never)] +pub fn test_subject(array: &ArrayVec) -> ArrayVec { + array.clone() +} diff --git a/tests/codegen/justfile b/tests/codegen/justfile new file mode 100644 index 00000000..af0b4f0b --- /dev/null +++ b/tests/codegen/justfile @@ -0,0 +1,51 @@ +root := "../.." + +default: check + +build-deps: + cd {{root}} && cargo build --release + +# Compile and inspect the asm for .clone() testcase +# Require that it's very short (should be like a short struct copy) +check: build-deps + #!/usr/bin/env bash + set -euo pipefail + + ROOT={{root}} + asm=$(rustc --edition 2024 -C opt-level=3 --crate-type lib \ + clone_copy.rs -L "$ROOT/target/release/deps" \ + --emit=asm -o -) + + fail= + + FUNC=test_subject + MAXBODY=20 + + # Sanity check + echo "$asm" | grep -q "$FUNC.*:" || { echo "FAIL: $FUNC label missing"; fail=1; } + + # Assertions + echo "$asm" | grep -q -E 'memcpy|memmove' && { echo "FAIL: references memcpy"; fail=1; } + echo "$asm" | grep -q -E 'panic' && { echo "FAIL: references panic"; fail=1; } + + # Extract the function body + body=$(echo "$asm" | sed -n "/$FUNC.*:$/,/\.cfi_endproc/p") + nlines=$(echo "$body" | wc -l | tr -d ' ') + if [ "$nlines" -gt "$MAXBODY" ]; then + echo "FAIL: body too long" + fail=1 + fi + + + if [ "${fail:-0}" = 1 ]; then + echo "--- output (full) ---" + echo "$asm" + echo "--- end ---" + exit 1 + else + echo "--- output (body) ---" + echo "$body" + echo "--- end ---" + fi + + echo "PASS: function $FUNC"