close
Skip to content

Commit 2464b9b

Browse files
committed
extend: Simplify while preserving optimizations
Keeping an eye on the benchmarks, simplify the optimization in extend_from_iter; use a simpler scope guard. Use a counted index. Previously there was a ZST special case which is not needed because: `ptr` is a valid pointer for writing and `ptr.add(index)` still too; `.write(elt)` consumes the element. (Previous implementation: ptr was an out of bounds pointer for ZST.) Benchmarks found to be sensitive to the order of predicates in `if index == end && CHECK` which shouldn't be the case, but it was. extend_from_iter: use only I: Iterator - reduce generics already in `extend`.
1 parent 6228c4d commit 2464b9b

1 file changed

Lines changed: 35 additions & 57 deletions

File tree

‎src/arrayvec.rs‎

Lines changed: 35 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,19 +1064,20 @@ impl<'a, T: 'a, const CAP: usize> Drop for Drain<'a, T, CAP> {
10641064
}
10651065
}
10661066

1067-
struct ScopeExitGuard<T, Data, F>
1068-
where F: FnMut(&Data, &mut T)
1069-
{
1070-
value: T,
1071-
data: Data,
1072-
f: F,
1067+
/// Guard that writes a `usize` length back to a `LenUint` field on drop.
1068+
///
1069+
/// Used to keep the `ArrayVec` length consistent if a panic occurs during
1070+
/// element-by-element writing: on panic the vector is left with the temporally
1071+
/// correct initialized length.
1072+
struct WritebackGuard<'a> {
1073+
target: &'a mut LenUint,
1074+
len: usize,
10731075
}
10741076

1075-
impl<T, Data, F> Drop for ScopeExitGuard<T, Data, F>
1076-
where F: FnMut(&Data, &mut T)
1077-
{
1077+
impl Drop for WritebackGuard<'_> {
1078+
#[inline(always)]
10781079
fn drop(&mut self) {
1079-
(self.f)(&self.data, &mut self.value)
1080+
*self.target = self.len as LenUint;
10801081
}
10811082
}
10821083

@@ -1092,7 +1093,7 @@ impl<T, const CAP: usize> Extend<T> for ArrayVec<T, CAP> {
10921093
#[track_caller]
10931094
fn extend<I: IntoIterator<Item=T>>(&mut self, iter: I) {
10941095
unsafe {
1095-
self.extend_from_iter::<_, true>(iter)
1096+
self.extend_from_iter::<_, true>(iter.into_iter())
10961097
}
10971098
}
10981099
}
@@ -1104,50 +1105,37 @@ fn extend_panic() {
11041105
panic!("ArrayVec: capacity exceeded in extend/from_iter");
11051106
}
11061107

1108+
11071109
impl<T, const CAP: usize> ArrayVec<T, CAP> {
1108-
/// Extend the arrayvec from the iterable.
1110+
/// Extend the vector from the iterator.
1111+
///
1112+
/// ***Panics*** if extending the vector exceeds its capacity.
11091113
///
11101114
/// ## Safety
11111115
///
1112-
/// Unsafe because if CHECK is false, the length of the input is not checked.
1113-
/// The caller must ensure the length of the input fits in the capacity.
1116+
/// If CHECK is false, the iterator must yield at most `CAP - len()` elements.
11141117
#[track_caller]
1115-
pub(crate) unsafe fn extend_from_iter<I, const CHECK: bool>(&mut self, iterable: I)
1116-
where I: IntoIterator<Item = T>
1118+
unsafe fn extend_from_iter<I, const CHECK: bool>(&mut self, iter: I)
1119+
where I: Iterator<Item = T>
11171120
{
1118-
let take = self.capacity() - self.len();
11191121
let len = self.len();
1120-
let mut ptr = raw_ptr_add(self.as_mut_ptr(), len);
1121-
let end_ptr = raw_ptr_add(ptr, take);
1122-
// Keep the length in a separate variable, write it back on scope
1123-
// exit. To help the compiler with alias analysis and stuff.
1124-
// We update the length to handle panic in the iteration of the
1125-
// user's iterator, without dropping any elements on the floor.
1126-
let mut guard = ScopeExitGuard {
1127-
value: &mut self.len,
1128-
data: len,
1129-
f: move |&len, self_len| {
1130-
**self_len = len as LenUint;
1131-
}
1132-
};
1133-
let mut iter = iterable.into_iter();
1134-
loop {
1135-
if let Some(elt) = iter.next() {
1136-
if ptr == end_ptr && CHECK { extend_panic(); }
1137-
debug_assert_ne!(ptr, end_ptr);
1138-
if mem::size_of::<T>() != 0 {
1139-
ptr.write(elt);
1140-
} else {
1141-
// The ZST element has logically been moved into the vector.
1142-
// There is no memory to write, but dropping `elt` here would
1143-
// drop it once now and once again when the vector is dropped.
1144-
mem::forget(elt);
1145-
}
1146-
ptr = raw_ptr_add(ptr, 1);
1147-
guard.data += 1;
1148-
} else {
1149-
return; // success
1122+
let end = self.capacity();
1123+
let ptr = self.as_mut_ptr();
1124+
1125+
// WritebackGuard updates self.len on drop (both success and panic).
1126+
let mut guard = WritebackGuard { target: &mut self.len, len: len };
1127+
1128+
// Take elements from the input iterator and write them into
1129+
// the arrayvec, as long as there is capacity left.
1130+
let mut index = len;
1131+
for elt in iter {
1132+
if index == end && CHECK {
1133+
extend_panic();
11501134
}
1135+
debug_assert_ne!(index, end);
1136+
ptr.add(index).write(elt);
1137+
guard.len += 1;
1138+
index += 1;
11511139
}
11521140
}
11531141

@@ -1165,16 +1153,6 @@ impl<T, const CAP: usize> ArrayVec<T, CAP> {
11651153
}
11661154
}
11671155

1168-
/// Rawptr add but uses arithmetic distance for ZST
1169-
unsafe fn raw_ptr_add<T>(ptr: *mut T, offset: usize) -> *mut T {
1170-
if mem::size_of::<T>() == 0 {
1171-
// Special case for ZST
1172-
ptr.cast::<u8>().wrapping_add(offset).cast::<T>()
1173-
} else {
1174-
ptr.add(offset)
1175-
}
1176-
}
1177-
11781156
/// Create an `ArrayVec` from an iterator.
11791157
///
11801158
/// ***Panics*** if the number of elements in the iterator exceeds the arrayvec's capacity.

0 commit comments

Comments
 (0)