Skip to content

Commit 318c2cc

Browse files
committed
less
Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent ebdf373 commit 318c2cc

5 files changed

Lines changed: 20 additions & 93 deletions

File tree

‎vortex-array/benches/take_slices_to_buffer_matrix.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//! Microbenchmarks for primitive `take_slices_to_buffer` copy-loop variants.
55
//!
66
//! The matrix covers:
7-
//! - append via `BufferMut::copy_from_slice`, indexed cursor copy, and advancing pointer copy
7+
//! - append via `BufferMut::extend_from_slice`, indexed cursor copy, and advancing pointer copy
88
//! into spare output capacity
99
//! - ordinary checked slicing vs a preverification pass followed by unchecked slicing
1010
//! - fixed-width short slices at the run counts used by the FSL take benchmarks
@@ -157,7 +157,7 @@ fn take_extend_safe(
157157
) -> Buffer<u16> {
158158
let mut result = BufferMut::<u16>::with_capacity(output_len);
159159
for (&start, &length) in starts.iter().zip(lengths) {
160-
result.copy_from_slice(&values[start..start + length]);
160+
result.extend_from_slice(&values[start..start + length]);
161161
}
162162
result.freeze()
163163
}
@@ -223,7 +223,7 @@ fn take_preverify_extend_unchecked(
223223
for (&start, &length) in starts.iter().zip(lengths) {
224224
// SAFETY: `preverify` checked every source range.
225225
unsafe {
226-
result.copy_from_slice(values.get_unchecked(start..start + length));
226+
result.extend_from_slice(values.get_unchecked(start..start + length));
227227
}
228228
}
229229
result.freeze()

‎vortex-array/src/arrays/filter/execute/slice.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ pub(super) fn filter_slice_by_slices<T: Copy>(
121121
) -> Buffer<T> {
122122
let mut out = BufferMut::<T>::with_capacity(output_len);
123123
for (start, end) in slices {
124-
out.copy_from_slice(&slice[*start..*end]);
124+
out.extend_from_slice(&slice[*start..*end]);
125125
}
126126

127127
out.freeze()

‎vortex-array/src/arrays/varbin/compute/take.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ fn take<Index: IntegerPType, Offset: IntegerPType>(
389389
let stop = offsets[idx + 1]
390390
.to_usize()
391391
.vortex_expect("Failed to cast max offset to usize");
392-
new_data.copy_from_slice(&data[start..stop]);
392+
new_data.extend_from_slice(&data[start..stop]);
393393
}
394394

395395
let array_validity = Validity::from(dtype.nullability());
@@ -798,7 +798,7 @@ fn take_nullable<Index: IntegerPType, Offset: IntegerPType>(
798798
let stop = offsets[data_idx + 1]
799799
.to_usize()
800800
.vortex_expect("Failed to cast max offset to usize");
801-
new_data.copy_from_slice(&data[start..stop]);
801+
new_data.extend_from_slice(&data[start..stop]);
802802
}
803803

804804
let array_validity = Validity::from(validity_buffer.freeze());

‎vortex-array/src/builders/primitive.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ impl<T: NativePType> PrimitiveBuilder<T> {
160160
"Cannot append primitive array with different ptype"
161161
);
162162

163-
self.values.copy_from_slice(array.as_slice::<T>());
163+
self.values.extend_from_slice(array.as_slice::<T>());
164164
self.nulls.append_validity_mask(
165165
&array
166166
.as_ref()

‎vortex-buffer/src/buffer_mut.rs‎

Lines changed: 13 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ impl<T> BufferMut<T> {
401401
preferred_alignment,
402402
allocator,
403403
);
404-
buffer.copy_from_slice(other);
404+
buffer.extend_from_slice(other);
405405
debug_assert_eq!(buffer.alignment(), alignment);
406406
buffer
407407
}
@@ -681,48 +681,17 @@ impl<T> BufferMut<T> {
681681
self.length += n;
682682
}
683683

684-
/// Appends a slice by cloning its elements, growing the internal buffer as needed.
685-
///
686-
/// Prefer [`Self::copy_from_slice`] when `T` implements `Copy`.
687-
///
688-
/// # Panics
689-
///
690-
/// If cloning an element panics, the buffer's length and existing values are unchanged.
691-
///
692-
/// # Example:
693-
///
694-
/// ```
695-
/// # use vortex_buffer::BufferMut;
696-
///
697-
/// let mut builder = BufferMut::<u16>::with_capacity(10);
698-
/// builder.extend_from_slice(&[42, 44, 46]);
699-
///
700-
/// assert_eq!(builder.len(), 3);
701-
/// ```
702-
#[inline]
703-
pub fn extend_from_slice(&mut self, slice: &[T])
704-
where
705-
T: Clone,
706-
{
707-
self.reserve(slice.len());
708-
// SAFETY: reserve guarantees at least slice.len() spare slots.
709-
let dst = unsafe { self.spare_capacity_mut().get_unchecked_mut(..slice.len()) };
710-
dst.write_clone_of_slice(slice);
711-
self.length += slice.len();
712-
}
713-
714684
/// Appends a slice by copying its elements, growing the internal buffer as needed.
715685
///
716-
/// This does not call [`Clone::clone`]. For types that only implement `Clone`, use
717-
/// [`Self::extend_from_slice`].
686+
/// This does not call [`Clone::clone`].
718687
///
719688
/// # Example
720689
///
721690
/// ```
722691
/// use vortex_buffer::BufferMut;
723692
///
724693
/// let mut buffer = BufferMut::from_iter([1, 2]);
725-
/// buffer.copy_from_slice(&[3, 4]);
694+
/// buffer.extend_from_slice(&[3, 4]);
726695
/// assert_eq!(buffer.as_slice(), &[1, 2, 3, 4]);
727696
/// ```
728697
///
@@ -735,10 +704,10 @@ impl<T> BufferMut<T> {
735704
/// struct NotCopy(u32);
736705
///
737706
/// let mut buffer = BufferMut::with_capacity(1);
738-
/// buffer.copy_from_slice(&[NotCopy(1)]);
707+
/// buffer.extend_from_slice(&[NotCopy(1)]);
739708
/// ```
740709
#[inline]
741-
pub fn copy_from_slice(&mut self, slice: &[T])
710+
pub fn extend_from_slice(&mut self, slice: &[T])
742711
where
743712
T: Copy,
744713
{
@@ -804,7 +773,7 @@ impl<T> BufferMut<T> {
804773
let capacity = self.capacity();
805774
let allocator = self.allocation.allocator().clone();
806775
let mut aligned = Self::with_capacity_aligned_in(capacity, alignment, allocator);
807-
aligned.copy_from_slice(&self);
776+
aligned.extend_from_slice(&self);
808777
aligned.capacity = capacity;
809778
aligned
810779
}
@@ -840,7 +809,7 @@ impl<T> BufferMut<T> {
840809
}
841810
}
842811

843-
impl<T: Clone> Clone for BufferMut<T> {
812+
impl<T: Copy> Clone for BufferMut<T> {
844813
fn clone(&self) -> Self {
845814
let mut buffer = BufferMut::<T>::with_capacity_aligned_in(
846815
self.capacity(),
@@ -1059,82 +1028,40 @@ impl<T> FromIterator<T> for BufferMut<T> {
10591028
#[cfg(test)]
10601029
mod tests {
10611030
use std::cell::Cell;
1062-
use std::panic::AssertUnwindSafe;
1063-
use std::panic::catch_unwind;
10641031

10651032
use crate::Alignment;
10661033
use crate::BufferMut;
10671034
use crate::buffer_mut;
10681035

1069-
#[derive(Clone, Debug, PartialEq)]
1070-
struct CloneOnly(u32);
1071-
10721036
#[derive(Copy, Debug, PartialEq)]
10731037
struct CopyWithCloneCounter<'a> {
10741038
value: u32,
10751039
clones: &'a Cell<usize>,
1076-
panic_at: Option<usize>,
10771040
}
10781041

10791042
#[allow(clippy::non_canonical_clone_impl)]
10801043
impl Clone for CopyWithCloneCounter<'_> {
10811044
fn clone(&self) -> Self {
10821045
self.clones.set(self.clones.get() + 1);
1083-
assert_ne!(Some(self.clones.get()), self.panic_at, "clone panicked");
10841046
*self
10851047
}
10861048
}
10871049

10881050
#[test]
1089-
fn extend_from_slice_supports_clone_only() {
1090-
let mut buffer = BufferMut::empty();
1091-
buffer.push(CloneOnly(1));
1092-
buffer.extend_from_slice(&[CloneOnly(2), CloneOnly(3)]);
1093-
1094-
assert_eq!(
1095-
buffer.as_slice(),
1096-
&[CloneOnly(1), CloneOnly(2), CloneOnly(3)]
1097-
);
1098-
assert_eq!(buffer.clone().as_slice(), buffer.as_slice());
1099-
}
1100-
1101-
#[test]
1102-
fn copy_from_slice_skips_clone() {
1051+
fn extend_from_slice_skips_clone() {
11031052
let clones = Cell::new(0);
11041053
let source = [CopyWithCloneCounter {
11051054
value: 42,
11061055
clones: &clones,
1107-
panic_at: None,
11081056
}];
11091057
let mut buffer = BufferMut::empty();
11101058
buffer.extend_from_slice(&source);
1111-
assert_eq!(clones.get(), 1);
1112-
1113-
buffer.copy_from_slice(&source);
1114-
buffer.copy_from_slice(&[]);
1115-
assert_eq!(clones.get(), 1);
1059+
buffer.extend_from_slice(&source);
1060+
buffer.extend_from_slice(&[]);
1061+
assert_eq!(clones.get(), 0);
11161062
assert_eq!(buffer.as_slice(), &[source[0], source[0]]);
1117-
}
1118-
1119-
#[test]
1120-
fn extend_from_slice_preserves_buffer_on_clone_panic() {
1121-
let clones = Cell::new(0);
1122-
let source = [CopyWithCloneCounter {
1123-
value: 42,
1124-
clones: &clones,
1125-
panic_at: Some(2),
1126-
}; 2];
1127-
let mut buffer = BufferMut::empty();
1128-
buffer.push(source[0]);
1129-
1130-
let result = catch_unwind(AssertUnwindSafe(|| buffer.extend_from_slice(&source)));
1131-
assert!(result.is_err());
1132-
assert_eq!(clones.get(), 2);
1133-
assert_eq!(buffer.as_slice(), &source[..1]);
1134-
1135-
buffer.copy_from_slice(&source);
1136-
assert_eq!(clones.get(), 2);
1137-
assert_eq!(buffer.as_slice(), &[source[0]; 3]);
1063+
assert_eq!(buffer.clone().as_slice(), buffer.as_slice());
1064+
assert_eq!(clones.get(), 0);
11381065
}
11391066

11401067
#[test]

0 commit comments

Comments
 (0)