Skip to content

Commit 4c3f787

Browse files
authored
bool array slice to support device buffers (#8875)
## Rationale for this change bool slicing on device buffers currently panic, because we assert host buffers when using slice on BitBuffer ## What changes are included in this PR? extract the slicing bounds check logic from BitBuffer, use it for both the host and the device buffer backed bool arrays --------- Signed-off-by: Onur Satici <onur@spiraldb.com>
1 parent c42934b commit 4c3f787

5 files changed

Lines changed: 90 additions & 28 deletions

File tree

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,12 @@ mod tests {
367367
use std::iter::once;
368368
use std::iter::repeat_n;
369369

370+
use vortex_buffer::Alignment;
370371
use vortex_buffer::BitBuffer;
371372
use vortex_buffer::BitBufferMut;
373+
use vortex_buffer::ByteBuffer;
372374
use vortex_buffer::buffer;
375+
use vortex_error::VortexResult;
373376

374377
use crate::IntoArray;
375378
use crate::VortexSessionExecute;
@@ -378,6 +381,7 @@ mod tests {
378381
use crate::arrays::PrimitiveArray;
379382
use crate::arrays::bool::BoolArrayExt;
380383
use crate::assert_arrays_eq;
384+
use crate::buffer::BufferHandle;
381385
use crate::patches::Patches;
382386
use crate::validity::Validity;
383387

@@ -471,6 +475,25 @@ mod tests {
471475
assert_arrays_eq!(sliced, BoolArray::from_iter([true; 8]), &mut ctx);
472476
}
473477

478+
#[test]
479+
fn slice_aligned_host_handle_at_unaligned_byte() -> VortexResult<()> {
480+
let bits: ByteBuffer = buffer![0b1010_1100_u8, 0b0110_1001, 0];
481+
let bits = bits.aligned(Alignment::of::<u64>());
482+
let array =
483+
BoolArray::new_handle(BufferHandle::new_host(bits), 0, 16, Validity::NonNullable)
484+
.into_array();
485+
486+
let sliced = array.slice(9..15)?;
487+
488+
let mut ctx = array_session().create_execution_ctx();
489+
assert_arrays_eq!(
490+
sliced,
491+
BoolArray::from_iter([false, false, true, false, true, true]),
492+
&mut ctx
493+
);
494+
Ok(())
495+
}
496+
474497
#[test]
475498
fn patch_bools_owned() {
476499
let mut ctx = array_session().create_execution_ctx();

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,23 @@ use crate::IntoArray;
1010
use crate::array::ArrayView;
1111
use crate::arrays::Bool;
1212
use crate::arrays::BoolArray;
13-
use crate::arrays::bool::BoolArrayExt;
1413
use crate::arrays::slice::SliceReduce;
14+
use crate::buffer::BufferHandle;
1515

1616
impl SliceReduce for Bool {
1717
fn slice(array: ArrayView<'_, Bool>, range: Range<usize>) -> VortexResult<Option<ArrayRef>> {
18-
let bit_buffer = array.to_bit_buffer().slice(range.clone());
18+
let (byte_start, meta) = array.meta.slice(range.clone());
19+
let byte_end = byte_start + meta.byte_len();
20+
21+
let bits = if let Some(host) = array.bits.as_host_opt() {
22+
BufferHandle::new_host(host.slice_unaligned(byte_start..byte_end))
23+
} else {
24+
array.bits.slice(byte_start..byte_end)
25+
};
1926
let validity = array.validity()?.slice(range)?;
2027

21-
// Safety:
22-
// range is verified in the callers and is the same for both bits and validity.
23-
let array = unsafe { BoolArray::new_unchecked(bit_buffer, validity).into_array() };
28+
let array = BoolArray::try_new_from_handle(bits, meta.offset(), meta.len(), validity)?;
2429

25-
Ok(Some(array))
30+
Ok(Some(array.into_array()))
2631
}
2732
}

vortex-buffer/src/bit/buf.rs

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ use std::fmt::Result as FmtResult;
77
use std::ops::BitAnd;
88
use std::ops::BitOr;
99
use std::ops::BitXor;
10-
use std::ops::Bound;
1110
use std::ops::Not;
1211
use std::ops::RangeBounds;
1312

1413
use crate::Alignment;
14+
use crate::BitBufferMeta;
1515
use crate::BitBufferMut;
1616
use crate::Buffer;
1717
use crate::BufferMut;
@@ -346,25 +346,7 @@ impl BitBuffer {
346346
///
347347
/// Panics if the slice would extend beyond the end of the buffer.
348348
pub fn slice(&self, range: impl RangeBounds<usize>) -> Self {
349-
let start = match range.start_bound() {
350-
Bound::Included(&s) => s,
351-
Bound::Excluded(&s) => s + 1,
352-
Bound::Unbounded => 0,
353-
};
354-
let end = match range.end_bound() {
355-
Bound::Included(&e) => e + 1,
356-
Bound::Excluded(&e) => e,
357-
Bound::Unbounded => self.len,
358-
};
359-
360-
assert!(start <= end);
361-
assert!(start <= self.len);
362-
assert!(end <= self.len);
363-
let len = end - start;
364-
365-
let offset = self.offset + start;
366-
let byte_offset = offset / 8;
367-
let bit_offset = offset % 8;
349+
let (byte_offset, meta) = BitBufferMeta::new(self.offset, self.len).slice(range);
368350

369351
// Trim whole bytes off the front directly rather than going through `new_with_offset`,
370352
// which would slice (and re-clone) the clone we'd have to pass it.
@@ -376,8 +358,8 @@ impl BitBuffer {
376358

377359
Self {
378360
buffer,
379-
offset: bit_offset,
380-
len,
361+
offset: meta.offset(),
362+
len: meta.len(),
381363
}
382364
}
383365

vortex-buffer/src/bit/meta.rs

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

4+
use std::ops::Bound;
5+
use std::ops::RangeBounds;
6+
7+
use vortex_error::VortexExpect;
8+
49
/// In-memory metadata describing a packed bitset: a normalized bit `offset` (always `< 8`) and a
510
/// logical bit `len`.
611
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -34,6 +39,34 @@ impl BitBufferMeta {
3439
)
3540
}
3641

42+
/// Return the leading byte offset and normalized metadata for a logical slice.
43+
///
44+
/// # Panics
45+
///
46+
/// Panics if the range is out of bounds or its end precedes its start.
47+
pub fn slice(&self, range: impl RangeBounds<usize>) -> (usize, Self) {
48+
let start = match range.start_bound() {
49+
Bound::Included(&start) => start,
50+
Bound::Excluded(&start) => start
51+
.checked_add(1)
52+
.vortex_expect("excluded slice start must not overflow"),
53+
Bound::Unbounded => 0,
54+
};
55+
let end = match range.end_bound() {
56+
Bound::Included(&end) => end
57+
.checked_add(1)
58+
.vortex_expect("included slice end must not overflow"),
59+
Bound::Excluded(&end) => end,
60+
Bound::Unbounded => self.len,
61+
};
62+
63+
assert!(start <= end);
64+
assert!(start <= self.len);
65+
assert!(end <= self.len);
66+
67+
Self::from_raw_offset(self.offset + start, end - start)
68+
}
69+
3770
/// The sub-byte bit offset. Always `< 8`.
3871
#[inline(always)]
3972
pub fn offset(&self) -> usize {

vortex-cuda/src/stream.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,9 @@ fn register_stream_callback(stream: &CudaStream) -> VortexResult<kanal::AsyncRec
259259
mod tests {
260260
use std::mem::size_of;
261261

262+
use vortex::array::IntoArray;
263+
use vortex::array::arrays::BoolArray;
264+
use vortex::array::validity::Validity;
262265
use vortex::error::VortexResult;
263266

264267
use super::padded_device_allocation_len;
@@ -327,4 +330,20 @@ mod tests {
327330

328331
Ok(())
329332
}
333+
334+
#[crate::test]
335+
async fn test_slice_device_bool_preserves_device_buffer() -> VortexResult<()> {
336+
let ctx = CudaSession::create_execution_ctx(&crate::cuda_session())?;
337+
let bits = ctx
338+
.stream()
339+
.copy_to_device(vec![0b1010_1100_u8, 0b0110_1001, 0b1100_0011])?
340+
.await?;
341+
let array = BoolArray::new_handle(bits, 3, 18, Validity::NonNullable).into_array();
342+
343+
let sliced = array.slice(7..16)?;
344+
345+
assert_eq!(sliced.len(), 9);
346+
assert!(sliced.buffer_handles()[0].is_on_device());
347+
Ok(())
348+
}
330349
}

0 commit comments

Comments
 (0)