Skip to content

Commit 1113b83

Browse files
dankingclauderobert3005
authored
Use cursor copies for PiecewiseSequence slices (#8832)
## Summary - add a shared checked cursor-copy helper for PiecewiseSequence slice gathering - replace `extend_from_slice` in the primitive, decimal, VarBinView, and VarBin PiecewiseSequence consumers - keep the safety checks explicit: each copy verifies the destination cursor range against the declared output length and capacity, and callers verify the final cursor before `set_len` ## Validation - `cargo +nightly fmt --all` - `cargo clippy -p vortex-array --all-targets --all-features -- -D warnings` - `cargo bench -p vortex-array --bench listview_rebuild` - `cargo bench -p vortex-array --bench cast_decimal` - `cargo test -p vortex-array piecewise_sequence` --------- Signed-off-by: Daniel King <dan@spiraldb.com> Signed-off-by: "Robert" <robert@spiraldb.com> Signed-off-by: Robert Kruszewski <github@robertk.io> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Robert Kruszewski <github@robertk.io>
1 parent 4c3f787 commit 1113b83

6 files changed

Lines changed: 154 additions & 21 deletions

File tree

vortex-array/src/arrays/bool/compute/take.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ where
159159
);
160160

161161
let mut values = BitBufferMut::with_capacity(output_len);
162-
for &start in starts {
162+
for start in starts {
163163
let start = start.as_();
164164
values.append_buffer(&source.slice(start..).slice(..length));
165165
}

vortex-array/src/arrays/decimal/compute/take.rs

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4+
use std::ptr;
5+
46
use itertools::Itertools as _;
57
use vortex_buffer::Buffer;
68
use vortex_buffer::BufferMut;
@@ -199,11 +201,28 @@ where
199201
);
200202

201203
let mut result = BufferMut::<T>::with_capacity(output_len);
204+
let spare = &mut result.spare_capacity_mut()[..output_len];
205+
let mut cursor = 0usize;
202206
for &start in starts {
203207
let start = start.as_();
204-
result.extend_from_slice(&values[start..][..length]);
208+
let src = &values[start..][..length];
209+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
210+
unsafe {
211+
ptr::copy_nonoverlapping(
212+
src.as_ptr(),
213+
spare[cursor..][..src.len()].as_mut_ptr().cast::<T>(),
214+
src.len(),
215+
);
216+
}
217+
cursor += src.len();
205218
}
206-
219+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
220+
unsafe { result.set_len(cursor) };
221+
vortex_ensure!(
222+
result.len() == output_len,
223+
"PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",
224+
result.len()
225+
);
207226
Ok(result.freeze())
208227
}
209228

@@ -219,12 +238,24 @@ where
219238
T: NativeDecimalType,
220239
{
221240
let mut result = BufferMut::<T>::with_capacity(output_len);
241+
let spare = &mut result.spare_capacity_mut()[..output_len];
242+
let mut cursor = 0usize;
222243
for (&start, &length) in starts.iter().zip_eq(lengths) {
223244
let start = start.as_();
224245
let length = length.as_();
225-
result.extend_from_slice(&values[start..][..length]);
246+
let src = &values[start..][..length];
247+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
248+
unsafe {
249+
ptr::copy_nonoverlapping(
250+
src.as_ptr(),
251+
spare[cursor..][..src.len()].as_mut_ptr().cast::<T>(),
252+
src.len(),
253+
);
254+
}
255+
cursor += src.len();
226256
}
227-
257+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
258+
unsafe { result.set_len(cursor) };
228259
vortex_ensure!(
229260
result.len() == output_len,
230261
"PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",

vortex-array/src/arrays/list/compute/take.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ where
463463
}
464464

465465
let mut total = 0usize;
466-
for &start in starts {
466+
for start in starts {
467467
let start: usize = start.as_();
468468
let offset_range = &offsets[start..][..=length];
469469
let element_start: usize = offset_range[0].as_();
@@ -490,7 +490,7 @@ where
490490
}
491491

492492
let mut total = 0usize;
493-
for &start in starts {
493+
for start in starts {
494494
let start: usize = start.as_();
495495
let additional = valid_piece_elements_len(offsets, data_validity, start, length)?;
496496
total = total
@@ -593,7 +593,7 @@ where
593593
let mut output_elements = 0usize;
594594

595595
new_offsets.push(OutputOffset::zero());
596-
for &start in starts {
596+
for start in starts {
597597
let start: usize = start.as_();
598598
if length == 0 {
599599
continue;
@@ -658,7 +658,7 @@ where
658658
};
659659

660660
gather.new_offsets.push(OutputOffset::zero());
661-
for &start in starts {
661+
for start in starts {
662662
let start: usize = start.as_();
663663
if length == 0 {
664664
continue;

vortex-array/src/arrays/primitive/compute/take/mod.rs

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#[cfg(any(target_arch = "x86_64", target_arch = "x86"))]
55
mod avx2;
66

7+
use std::ptr;
78
use std::sync::LazyLock;
89

910
use itertools::Itertools as _;
@@ -245,12 +246,24 @@ where
245246
L: UnsignedPType,
246247
{
247248
let mut values = BufferMut::<T>::with_capacity(output_len);
249+
let spare = &mut values.spare_capacity_mut()[..output_len];
250+
let mut cursor = 0usize;
248251
for (&start, &length) in starts.iter().zip_eq(lengths) {
249252
let start = start.as_();
250253
let length = length.as_();
251-
values.extend_from_slice(&source[start..][..length]);
254+
let src = &source[start..][..length];
255+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
256+
unsafe {
257+
ptr::copy_nonoverlapping(
258+
src.as_ptr(),
259+
spare[cursor..][..src.len()].as_mut_ptr().cast::<T>(),
260+
src.len(),
261+
);
262+
}
263+
cursor += src.len();
252264
}
253-
265+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
266+
unsafe { values.set_len(cursor) };
254267
vortex_ensure!(
255268
values.len() == output_len,
256269
"PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",
@@ -312,11 +325,28 @@ where
312325
);
313326

314327
let mut values = BufferMut::<T>::with_capacity(output_len);
328+
let spare = &mut values.spare_capacity_mut()[..output_len];
329+
let mut cursor = 0usize;
315330
for &start in starts {
316331
let start = start.as_();
317-
values.extend_from_slice(&source[start..][..length]);
332+
let src = &source[start..][..length];
333+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
334+
unsafe {
335+
ptr::copy_nonoverlapping(
336+
src.as_ptr(),
337+
spare[cursor..][..src.len()].as_mut_ptr().cast::<T>(),
338+
src.len(),
339+
);
340+
}
341+
cursor += src.len();
318342
}
319-
343+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
344+
unsafe { values.set_len(cursor) };
345+
vortex_ensure!(
346+
values.len() == output_len,
347+
"PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",
348+
values.len()
349+
);
320350
Ok(values.freeze())
321351
}
322352

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

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4+
use std::ptr;
5+
46
use itertools::Itertools as _;
57
use vortex_buffer::BitBufferMut;
68
use vortex_buffer::BufferMut;
@@ -442,7 +444,7 @@ where
442444
new_offsets.push(NewOffset::zero());
443445
let mut output_bytes = 0usize;
444446

445-
for &start in starts {
447+
for start in starts {
446448
let start = start.as_();
447449
if length == 0 {
448450
continue;
@@ -474,7 +476,9 @@ where
474476
}
475477

476478
let mut new_data = ByteBufferMut::with_capacity(output_bytes);
477-
for &start in starts {
479+
let spare = &mut new_data.spare_capacity_mut()[..output_bytes];
480+
let mut cursor = 0usize;
481+
for start in starts {
478482
let start = start.as_();
479483
if length == 0 {
480484
continue;
@@ -483,8 +487,24 @@ where
483487
let offset_range = &offsets[start..][..=length];
484488
let byte_start = offset_range[0].as_();
485489
let byte_end = offset_range[length].as_();
486-
new_data.extend_from_slice(&data[byte_start..][..byte_end - byte_start]);
490+
let src = &data[byte_start..byte_end];
491+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
492+
unsafe {
493+
ptr::copy_nonoverlapping(
494+
src.as_ptr(),
495+
spare[cursor..][..src.len()].as_mut_ptr().cast::<u8>(),
496+
src.len(),
497+
);
498+
}
499+
cursor += src.len();
487500
}
501+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
502+
unsafe { new_data.set_len(cursor) };
503+
vortex_ensure!(
504+
new_data.len() == output_bytes,
505+
"PiecewiseSequenceArray gathered byte length {} does not match declared byte length {output_bytes}",
506+
new_data.len()
507+
);
488508

489509
let offsets = PrimitiveArray::new(new_offsets.freeze(), Validity::NonNullable)
490510
.reinterpret_cast(out_offset_ptype)
@@ -551,6 +571,8 @@ where
551571
);
552572

553573
let mut new_data = ByteBufferMut::with_capacity(output_bytes);
574+
let spare = &mut new_data.spare_capacity_mut()[..output_bytes];
575+
let mut cursor = 0usize;
554576
for (&start, &length) in starts.iter().zip_eq(lengths) {
555577
let start = start.as_();
556578
let length = length.as_();
@@ -561,8 +583,24 @@ where
561583
let offset_range = &offsets[start..][..=length];
562584
let byte_start = offset_range[0].as_();
563585
let byte_end = offset_range[length].as_();
564-
new_data.extend_from_slice(&data[byte_start..][..byte_end - byte_start]);
586+
let src = &data[byte_start..byte_end];
587+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
588+
unsafe {
589+
ptr::copy_nonoverlapping(
590+
src.as_ptr(),
591+
spare[cursor..][..src.len()].as_mut_ptr().cast::<u8>(),
592+
src.len(),
593+
);
594+
}
595+
cursor += src.len();
565596
}
597+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
598+
unsafe { new_data.set_len(cursor) };
599+
vortex_ensure!(
600+
new_data.len() == output_bytes,
601+
"PiecewiseSequenceArray gathered byte length {} does not match declared byte length {output_bytes}",
602+
new_data.len()
603+
);
566604

567605
let offsets = PrimitiveArray::new(new_offsets.freeze(), Validity::NonNullable)
568606
.reinterpret_cast(out_offset_ptype)

vortex-array/src/arrays/varbinview/compute/take.rs

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
use std::iter;
5+
use std::ptr;
56
use std::sync::Arc;
67

78
use itertools::Itertools as _;
@@ -174,11 +175,30 @@ where
174175
);
175176

176177
let mut views = BufferMut::<BinaryView>::with_capacity(output_len);
178+
let spare = &mut views.spare_capacity_mut()[..output_len];
179+
let mut cursor = 0usize;
177180
for &start in starts {
178181
let start = start.as_();
179-
views.extend_from_slice(&source[start..][..length]);
182+
let src = &source[start..][..length];
183+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
184+
unsafe {
185+
ptr::copy_nonoverlapping(
186+
src.as_ptr(),
187+
spare[cursor..][..src.len()]
188+
.as_mut_ptr()
189+
.cast::<BinaryView>(),
190+
src.len(),
191+
);
192+
}
193+
cursor += src.len();
180194
}
181-
195+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
196+
unsafe { views.set_len(cursor) };
197+
vortex_ensure!(
198+
views.len() == output_len,
199+
"PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",
200+
views.len()
201+
);
182202
Ok(views.freeze())
183203
}
184204

@@ -193,12 +213,26 @@ where
193213
L: UnsignedPType,
194214
{
195215
let mut views = BufferMut::<BinaryView>::with_capacity(output_len);
216+
let spare = &mut views.spare_capacity_mut()[..output_len];
217+
let mut cursor = 0usize;
196218
for (&start, &length) in starts.iter().zip_eq(lengths) {
197219
let start = start.as_();
198220
let length = length.as_();
199-
views.extend_from_slice(&source[start..][..length]);
221+
let src = &source[start..][..length];
222+
// SAFETY: `src` and the checked `spare` range have equal lengths and cannot overlap.
223+
unsafe {
224+
ptr::copy_nonoverlapping(
225+
src.as_ptr(),
226+
spare[cursor..][..src.len()]
227+
.as_mut_ptr()
228+
.cast::<BinaryView>(),
229+
src.len(),
230+
);
231+
}
232+
cursor += src.len();
200233
}
201-
234+
// SAFETY: the loop initialized the prefix `0..cursor` of the spare capacity.
235+
unsafe { views.set_len(cursor) };
202236
vortex_ensure!(
203237
views.len() == output_len,
204238
"PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",

0 commit comments

Comments
 (0)