From 599ed2bceb2d02a0dc486decd173eb7e43ea5cec Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Tue, 2 Jun 2026 12:19:04 +0100 Subject: [PATCH 1/3] fix Signed-off-by: Joe Isaacs --- .../aggregate_fn/fns/all_non_distinct/bool.rs | 2 +- .../src/aggregate_fn/fns/is_constant/bool.rs | 2 +- .../src/aggregate_fn/fns/is_sorted/bool.rs | 6 +- .../src/aggregate_fn/fns/min_max/bool.rs | 2 +- vortex-array/src/aggregate_fn/fns/sum/bool.rs | 2 +- vortex-array/src/arrays/bool/array.rs | 34 +- vortex-array/src/arrays/bool/compute/take.rs | 12 +- vortex-array/src/arrays/bool/patch.rs | 2 +- vortex-array/src/arrays/bool/vtable/mod.rs | 13 +- .../src/arrays/bool/vtable/operations.rs | 2 +- .../src/arrays/dict/compute/fill_null.rs | 2 +- .../src/arrays/patched/compute/compare.rs | 5 +- .../src/arrays/primitive/array/mod.rs | 2 +- vortex-array/src/canonical.rs | 18 +- .../src/scalar_fn/fns/list_contains/mod.rs | 4 +- vortex-buffer/src/bit/buf_mut.rs | 2 +- vortex-buffer/src/bit/meta.rs | 60 ++ vortex-buffer/src/bit/mod.rs | 4 + vortex-buffer/src/bit/view.rs | 546 ++++++++++++++++++ vortex-cuda/src/arrow/canonical.rs | 6 +- vortex-cuda/src/canonical.rs | 10 +- vortex-duckdb/src/exporter/struct_.rs | 2 +- 22 files changed, 686 insertions(+), 52 deletions(-) create mode 100644 vortex-buffer/src/bit/meta.rs create mode 100644 vortex-buffer/src/bit/view.rs diff --git a/vortex-array/src/aggregate_fn/fns/all_non_distinct/bool.rs b/vortex-array/src/aggregate_fn/fns/all_non_distinct/bool.rs index c06d335e458..f201653b815 100644 --- a/vortex-array/src/aggregate_fn/fns/all_non_distinct/bool.rs +++ b/vortex-array/src/aggregate_fn/fns/all_non_distinct/bool.rs @@ -10,5 +10,5 @@ where L: BoolArrayExt, R: BoolArrayExt, { - Ok(lhs.to_bit_buffer() == rhs.to_bit_buffer()) + Ok(lhs.bit_buffer_view() == rhs.bit_buffer_view()) } diff --git a/vortex-array/src/aggregate_fn/fns/is_constant/bool.rs b/vortex-array/src/aggregate_fn/fns/is_constant/bool.rs index c63025ccd7c..967f19058b6 100644 --- a/vortex-array/src/aggregate_fn/fns/is_constant/bool.rs +++ b/vortex-array/src/aggregate_fn/fns/is_constant/bool.rs @@ -5,6 +5,6 @@ use crate::arrays::BoolArray; use crate::arrays::bool::BoolArrayExt; pub(super) fn check_bool_constant(array: &BoolArray) -> bool { - let true_count = array.to_bit_buffer().true_count(); + let true_count = array.bit_buffer_view().true_count(); true_count == array.len() || true_count == 0 } diff --git a/vortex-array/src/aggregate_fn/fns/is_sorted/bool.rs b/vortex-array/src/aggregate_fn/fns/is_sorted/bool.rs index afd18f15902..69c0aaf6692 100644 --- a/vortex-array/src/aggregate_fn/fns/is_sorted/bool.rs +++ b/vortex-array/src/aggregate_fn/fns/is_sorted/bool.rs @@ -21,7 +21,7 @@ pub(super) fn check_bool_sorted( { Mask::AllFalse(_) => Ok(!strict), Mask::AllTrue(_) => { - let values = array.to_bit_buffer(); + let values = array.bit_buffer_view(); Ok(if strict { values.iter().is_strict_sorted() } else { @@ -31,7 +31,7 @@ pub(super) fn check_bool_sorted( Mask::Values(mask_values) => { if strict { let validity_buffer = mask_values.bit_buffer(); - let values = array.to_bit_buffer(); + let values = array.bit_buffer_view(); Ok(validity_buffer .iter() .zip(values.iter()) @@ -39,7 +39,7 @@ pub(super) fn check_bool_sorted( .is_strict_sorted()) } else { let set_indices = mask_values.bit_buffer().set_indices(); - let values = array.to_bit_buffer(); + let values = array.bit_buffer_view(); let values_iter = set_indices.map(|idx| // Safety: // All idxs are in-bounds for the array. diff --git a/vortex-array/src/aggregate_fn/fns/min_max/bool.rs b/vortex-array/src/aggregate_fn/fns/min_max/bool.rs index 69471eec556..b3a92c49ce0 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/bool.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/bool.rs @@ -29,7 +29,7 @@ pub(super) fn accumulate_bool( .execute_mask(array.as_ref().len(), ctx)?; let (true_count, valid_count) = match mask.bit_buffer() { AllOr::None => return Ok(()), - AllOr::All => (array.to_bit_buffer().true_count(), array.as_ref().len()), + AllOr::All => (array.bit_buffer_view().true_count(), array.as_ref().len()), AllOr::Some(validity) => ( array.to_bit_buffer().bitand(validity).true_count(), validity.true_count(), diff --git a/vortex-array/src/aggregate_fn/fns/sum/bool.rs b/vortex-array/src/aggregate_fn/fns/sum/bool.rs index a32f3ba72f7..7728d3f64af 100644 --- a/vortex-array/src/aggregate_fn/fns/sum/bool.rs +++ b/vortex-array/src/aggregate_fn/fns/sum/bool.rs @@ -25,7 +25,7 @@ pub(super) fn accumulate_bool( let mask = b.as_ref().validity()?.execute_mask(b.as_ref().len(), ctx)?; let true_count = match mask.bit_buffer() { AllOr::None => return Ok(false), - AllOr::All => b.to_bit_buffer().true_count() as u64, + AllOr::All => b.bit_buffer_view().true_count() as u64, AllOr::Some(validity) => b.to_bit_buffer().bitand(validity).true_count() as u64, }; diff --git a/vortex-array/src/arrays/bool/array.rs b/vortex-array/src/arrays/bool/array.rs index 6585e705899..9049a8e5faf 100644 --- a/vortex-array/src/arrays/bool/array.rs +++ b/vortex-array/src/arrays/bool/array.rs @@ -7,7 +7,9 @@ use std::fmt::Formatter; use arrow_array::BooleanArray; use smallvec::smallvec; use vortex_buffer::BitBuffer; +use vortex_buffer::BitBufferMeta; use vortex_buffer::BitBufferMut; +use vortex_buffer::BitBufferView; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_ensure; @@ -69,19 +71,18 @@ pub(super) const SLOT_NAMES: [&str; NUM_SLOTS] = ["validity"]; #[derive(Clone, Debug)] pub struct BoolData { pub(super) bits: BufferHandle, - pub(super) offset: usize, + pub(super) meta: BitBufferMeta, } impl Display for BoolData { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "offset: {}", self.offset) + write!(f, "offset: {}", self.meta.offset()) } } pub struct BoolDataParts { pub bits: BufferHandle, - pub offset: usize, - pub len: usize, + pub meta: BitBufferMeta, } pub trait BoolArrayExt: TypedArrayRef { @@ -101,7 +102,12 @@ pub trait BoolArrayExt: TypedArrayRef { fn to_bit_buffer(&self) -> BitBuffer { let buffer = self.bits.as_host().clone(); - BitBuffer::new_with_offset(buffer, self.as_ref().len(), self.offset) + BitBuffer::new_with_offset(buffer, self.meta.len(), self.meta.offset()) + } + + /// Borrow the array's packed bits as a [`BitBufferView`] without cloning the backing buffer. + fn bit_buffer_view(&self) -> BitBufferView<'_> { + BitBufferView::from_meta(self.bits.as_host().as_slice(), self.meta) } fn maybe_execute_mask(&self, ctx: &mut ExecutionCtx) -> VortexResult> { @@ -141,8 +147,7 @@ impl BoolData { pub fn into_parts(self, len: usize) -> BoolDataParts { BoolDataParts { bits: self.bits, - offset: self.offset, - len, + meta: BitBufferMeta::new(self.meta.offset(), len), } } @@ -242,7 +247,7 @@ impl Array { let len = self.len(); let data = self.into_data(); let buffer = data.bits.unwrap_host(); - BitBuffer::new_with_offset(buffer, len, data.offset) + BitBuffer::new_with_offset(buffer, len, data.meta.offset()) } } @@ -252,11 +257,11 @@ impl BoolData { let bits = bits.shrink_offset(); Self::validate(&bits, &validity)?; - let (offset, _len, buffer) = bits.into_inner(); + let (offset, len, buffer) = bits.into_inner(); Ok(Self { bits: BufferHandle::new_host(buffer), - offset, + meta: BitBufferMeta::new(offset, len), }) } @@ -281,18 +286,21 @@ impl BoolData { bits.len() * 8, ); - Ok(Self { bits, offset }) + Ok(Self { + bits, + meta: BitBufferMeta::new(offset, len), + }) } pub(super) unsafe fn new_unchecked(bits: BitBuffer, validity: Validity) -> Self { if cfg!(debug_assertions) { Self::try_new(bits, validity).vortex_expect("Failed to create BoolData") } else { - let (offset, _len, buffer) = bits.into_inner(); + let (offset, len, buffer) = bits.into_inner(); Self { bits: BufferHandle::new_host(buffer), - offset, + meta: BitBufferMeta::new(offset, len), } } } diff --git a/vortex-array/src/arrays/bool/compute/take.rs b/vortex-array/src/arrays/bool/compute/take.rs index 1f679445254..d70579136a4 100644 --- a/vortex-array/src/arrays/bool/compute/take.rs +++ b/vortex-array/src/arrays/bool/compute/take.rs @@ -4,6 +4,7 @@ use itertools::Itertools as _; use num_traits::AsPrimitive; use vortex_buffer::BitBuffer; +use vortex_buffer::BitBufferView; use vortex_buffer::get_bit; use vortex_error::VortexResult; use vortex_mask::Mask; @@ -42,7 +43,10 @@ impl TakeExecute for Bool { }; let indices_nulls_zeroed = indices_nulls_zeroed.execute::(ctx)?; let buffer = match_each_integer_ptype!(indices_nulls_zeroed.ptype(), |I| { - take_valid_indices(&array.to_bit_buffer(), indices_nulls_zeroed.as_slice::()) + take_valid_indices( + array.bit_buffer_view(), + indices_nulls_zeroed.as_slice::(), + ) }); Ok(Some( @@ -51,7 +55,7 @@ impl TakeExecute for Bool { } } -fn take_valid_indices>(bools: &BitBuffer, indices: &[I]) -> BitBuffer { +fn take_valid_indices>(bools: BitBufferView<'_>, indices: &[I]) -> BitBuffer { // For boolean arrays that roughly fit into a single page (at least, on Linux), it's worth // the overhead to convert to a Vec. if bools.len() <= 4096 { @@ -68,9 +72,9 @@ fn take_byte_bool>(bools: Vec, indices: &[I]) -> Bit }) } -fn take_bool_impl>(bools: &BitBuffer, indices: &[I]) -> BitBuffer { +fn take_bool_impl>(bools: BitBufferView<'_>, indices: &[I]) -> BitBuffer { // We dereference to underlying buffer to avoid access cost on every index. - let buffer = bools.inner().as_ref(); + let buffer = bools.inner(); BitBuffer::collect_bool(indices.len(), |idx| { // SAFETY: we can take from the indices unchecked since collect_bool just iterates len. let idx = unsafe { indices.get_unchecked(idx).as_() }; diff --git a/vortex-array/src/arrays/bool/patch.rs b/vortex-array/src/arrays/bool/patch.rs index fef03bc30fb..5efdac30231 100644 --- a/vortex-array/src/arrays/bool/patch.rs +++ b/vortex-array/src/arrays/bool/patch.rs @@ -31,7 +31,7 @@ impl BoolArray { for (idx, value) in indices .as_slice::() .iter() - .zip_eq(values.to_bit_buffer().iter()) + .zip_eq(values.bit_buffer_view().iter()) { #[allow(clippy::cast_possible_truncation)] own_values.set_to(*idx as usize - offset, value); diff --git a/vortex-array/src/arrays/bool/vtable/mod.rs b/vortex-array/src/arrays/bool/vtable/mod.rs index 35261fa8d89..899d6d3ab9a 100644 --- a/vortex-array/src/arrays/bool/vtable/mod.rs +++ b/vortex-array/src/arrays/bool/vtable/mod.rs @@ -53,13 +53,13 @@ pub struct BoolMetadata { impl ArrayHash for BoolData { fn array_hash(&self, state: &mut H, precision: Precision) { self.bits.array_hash(state, precision); - self.offset.hash(state); + self.meta.offset().hash(state); } } impl ArrayEq for BoolData { fn array_eq(&self, other: &Self, precision: Precision) -> bool { - self.offset == other.offset && self.bits.array_eq(&other.bits, precision) + self.meta.offset() == other.meta.offset() && self.bits.array_eq(&other.bits, precision) } } @@ -96,10 +96,11 @@ impl VTable for Bool { array: ArrayView<'_, Self>, _session: &VortexSession, ) -> VortexResult>> { - assert!(array.offset < 8, "Offset must be <8, got {}", array.offset); + let offset = array.meta.offset(); + assert!(offset < 8, "Offset must be <8, got {offset}"); Ok(Some( BoolMetadata { - offset: u32::try_from(array.offset).vortex_expect("checked"), + offset: u32::try_from(offset).vortex_expect("checked"), } .encode_to_vec(), )) @@ -116,9 +117,9 @@ impl VTable for Bool { vortex_bail!("Expected bool dtype, got {dtype:?}"); }; vortex_ensure!( - data.bits.len() * 8 >= data.offset + len, + data.bits.len() * 8 >= data.meta.offset() + len, "BoolArray buffer with offset {} cannot back outer length {} (buffer bits = {})", - data.offset, + data.meta.offset(), len, data.bits.len() * 8 ); diff --git a/vortex-array/src/arrays/bool/vtable/operations.rs b/vortex-array/src/arrays/bool/vtable/operations.rs index 11147b810b2..b4ac4682d23 100644 --- a/vortex-array/src/arrays/bool/vtable/operations.rs +++ b/vortex-array/src/arrays/bool/vtable/operations.rs @@ -17,7 +17,7 @@ impl OperationsVTable for Bool { _ctx: &mut ExecutionCtx, ) -> VortexResult { Ok(Scalar::bool( - array.to_bit_buffer().value(index), + array.bit_buffer_view().value(index), array.dtype().nullability(), )) } diff --git a/vortex-array/src/arrays/dict/compute/fill_null.rs b/vortex-array/src/arrays/dict/compute/fill_null.rs index 189d8240050..8f146d728a3 100644 --- a/vortex-array/src/arrays/dict/compute/fill_null.rs +++ b/vortex-array/src/arrays/dict/compute/fill_null.rs @@ -41,7 +41,7 @@ impl FillNullKernel for Dict { // We found the fill value already in the values at this given index. let Some(existing_fill_value_index) = - found_fill_values.to_bit_buffer().set_indices().next() + found_fill_values.bit_buffer_view().set_indices().next() else { // No fill values found, so we must canonicalize and fill_null. return Ok(Some( diff --git a/vortex-array/src/arrays/patched/compute/compare.rs b/vortex-array/src/arrays/patched/compute/compare.rs index c7d879323cb..0f1e6add00f 100644 --- a/vortex-array/src/arrays/patched/compute/compare.rs +++ b/vortex-array/src/arrays/patched/compute/compare.rs @@ -55,9 +55,10 @@ impl CompareKernel for Patched { let validity = child_to_validity(result.slots()[0].as_ref(), result.dtype().nullability()); let len = result.len(); - let BoolDataParts { bits, offset, len } = result.into_data().into_parts(len); + let BoolDataParts { bits, meta } = result.into_data().into_parts(len); - let mut bits = BitBufferMut::from_buffer(bits.unwrap_host().into_mut(), offset, len); + let mut bits = + BitBufferMut::from_buffer(bits.unwrap_host().into_mut(), meta.offset(), meta.len()); let lane_offsets = lhs.lane_offsets().clone().execute::(ctx)?; let indices = lhs.patch_indices().clone().execute::(ctx)?; diff --git a/vortex-array/src/arrays/primitive/array/mod.rs b/vortex-array/src/arrays/primitive/array/mod.rs index 5b7a5d95c8c..9290f9efcc6 100644 --- a/vortex-array/src/arrays/primitive/array/mod.rs +++ b/vortex-array/src/arrays/primitive/array/mod.rs @@ -557,7 +557,7 @@ impl PrimitiveData { Validity::Array(is_valid) => { #[expect(deprecated)] let bool_array = is_valid.to_bool(); - let bool_buffer = bool_array.to_bit_buffer(); + let bool_buffer = bool_array.bit_buffer_view(); let mut bytes = ByteBufferMut::zeroed_aligned(n_rows * byte_width, alignment); for (i, valid_i) in bool_buffer.set_indices().enumerate() { bytes[valid_i * byte_width..(valid_i + 1) * byte_width] diff --git a/vortex-array/src/canonical.rs b/vortex-array/src/canonical.rs index b1773d453f2..6fbdc154a6c 100644 --- a/vortex-array/src/canonical.rs +++ b/vortex-array/src/canonical.rs @@ -563,9 +563,14 @@ impl Executable for CanonicalValidity { Canonical::Bool(b) => { let validity = child_to_validity(b.slots()[0].as_ref(), b.dtype().nullability()); let len = b.len(); - let BoolDataParts { bits, offset, len } = b.into_data().into_parts(len); + let BoolDataParts { bits, meta } = b.into_data().into_parts(len); Ok(CanonicalValidity(Canonical::Bool( - BoolArray::try_new_from_handle(bits, offset, len, validity.execute(ctx)?)?, + BoolArray::try_new_from_handle( + bits, + meta.offset(), + meta.len(), + validity.execute(ctx)?, + )?, ))) } Canonical::Primitive(p) => { @@ -713,9 +718,14 @@ impl Executable for RecursiveCanonical { Canonical::Bool(b) => { let validity = child_to_validity(b.slots()[0].as_ref(), b.dtype().nullability()); let len = b.len(); - let BoolDataParts { bits, offset, len } = b.into_data().into_parts(len); + let BoolDataParts { bits, meta } = b.into_data().into_parts(len); Ok(RecursiveCanonical(Canonical::Bool( - BoolArray::try_new_from_handle(bits, offset, len, validity.execute(ctx)?)?, + BoolArray::try_new_from_handle( + bits, + meta.offset(), + meta.len(), + validity.execute(ctx)?, + )?, ))) } Canonical::Primitive(p) => { diff --git a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs index e91936ca6dc..0791a78a922 100644 --- a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs +++ b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs @@ -351,7 +351,7 @@ where { let offsets_slice = offsets.as_slice::(); let sizes_slice = sizes.as_slice::(); - let bits = matches.to_bit_buffer(); + let bits = matches.bit_buffer_view(); (0..list_array_len) .map(|i| { @@ -360,7 +360,7 @@ where // BitIndexIterator yields indices of true bits only. If `.next()` returns // `Some(_)`, at least one element in this list's range matches. - let mut set_bits = BitIndexIterator::new(bits.inner().as_ref(), offset, size); + let mut set_bits = BitIndexIterator::new(bits.inner(), offset, size); set_bits.next().is_some() }) .collect::() diff --git a/vortex-buffer/src/bit/buf_mut.rs b/vortex-buffer/src/bit/buf_mut.rs index d1c96f069e8..d74832c83fa 100644 --- a/vortex-buffer/src/bit/buf_mut.rs +++ b/vortex-buffer/src/bit/buf_mut.rs @@ -16,7 +16,7 @@ use crate::buffer_mut; /// Sets all bits in the bit-range `[start_bit, end_bit)` of `slice` to `value`. #[inline(always)] -fn fill_bits(slice: &mut [u8], start_bit: usize, end_bit: usize, value: bool) { +pub(crate) fn fill_bits(slice: &mut [u8], start_bit: usize, end_bit: usize, value: bool) { if start_bit >= end_bit { return; } diff --git a/vortex-buffer/src/bit/meta.rs b/vortex-buffer/src/bit/meta.rs new file mode 100644 index 00000000000..866a308647d --- /dev/null +++ b/vortex-buffer/src/bit/meta.rs @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +/// In-memory metadata describing a packed bitset: a normalized bit `offset` (always `< 8`) and a +/// logical bit `len`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct BitBufferMeta { + offset: usize, + len: usize, +} + +impl BitBufferMeta { + /// Create metadata for a bitset starting at bit `offset` with `len` bits. + /// + /// Panics if `offset >= 8`. Use [`from_raw_offset`](Self::from_raw_offset) to normalize a + /// larger offset. + pub fn new(offset: usize, len: usize) -> Self { + assert!(offset < 8, "BitBufferMeta offset must be < 8, got {offset}"); + Self { offset, len } + } + + /// Normalize a raw bit `offset` into a whole-byte offset plus metadata whose `offset` is + /// `< 8`. + /// + /// Returns `(byte_offset, meta)` so the caller can slice its backing buffer by `byte_offset` + /// and store the remaining sub-byte offset in `meta`. + pub fn from_raw_offset(offset: usize, len: usize) -> (usize, Self) { + ( + offset / 8, + Self { + offset: offset % 8, + len, + }, + ) + } + + /// The sub-byte bit offset. Always `< 8`. + #[inline(always)] + pub fn offset(&self) -> usize { + self.offset + } + + /// The logical length of the bitset in bits. + #[inline(always)] + pub fn len(&self) -> usize { + self.len + } + + /// Returns `true` if the bitset is empty. + #[inline(always)] + pub fn is_empty(&self) -> bool { + self.len == 0 + } + + /// The number of backing bytes required to hold `offset + len` bits. + #[inline] + pub fn byte_len(&self) -> usize { + (self.offset + self.len).div_ceil(8) + } +} diff --git a/vortex-buffer/src/bit/mod.rs b/vortex-buffer/src/bit/mod.rs index 37930d788b7..6ca44521d5a 100644 --- a/vortex-buffer/src/bit/mod.rs +++ b/vortex-buffer/src/bit/mod.rs @@ -12,8 +12,10 @@ mod buf; mod buf_mut; mod count_ones; mod macros; +mod meta; mod ops; mod select; +mod view; pub use arrow_buffer::bit_chunk_iterator::BitChunkIterator; pub use arrow_buffer::bit_chunk_iterator::BitChunks; @@ -24,6 +26,8 @@ pub use arrow_buffer::bit_iterator::BitIterator; pub use arrow_buffer::bit_iterator::BitSliceIterator; pub use buf::*; pub use buf_mut::*; +pub use meta::*; +pub use view::*; /// Get the bit value at `index` out of `buf`. /// diff --git a/vortex-buffer/src/bit/view.rs b/vortex-buffer/src/bit/view.rs new file mode 100644 index 00000000000..722119ee4ab --- /dev/null +++ b/vortex-buffer/src/bit/view.rs @@ -0,0 +1,546 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +use std::ops::Bound; +use std::ops::RangeBounds; + +use crate::BitBuffer; +use crate::BitBufferMeta; +use crate::BitBufferMut; +use crate::ByteBuffer; +use crate::bit::BitChunks; +use crate::bit::BitIndexIterator; +use crate::bit::BitIterator; +use crate::bit::BitSliceIterator; +use crate::bit::UnalignedBitChunk; +use crate::bit::buf_mut::fill_bits; +use crate::bit::count_ones::count_ones; +use crate::bit::get_bit_unchecked; +use crate::bit::select::bit_select; +use crate::bit::set_bit_unchecked; +use crate::bit::unset_bit_unchecked; + +/// Resolve `start..end` bounds against a logical length, panicking on invalid ranges. +#[inline] +fn resolve_range(range: impl RangeBounds, len: usize) -> (usize, usize) { + let start = match range.start_bound() { + Bound::Included(&s) => s, + Bound::Excluded(&s) => s + 1, + Bound::Unbounded => 0, + }; + let end = match range.end_bound() { + Bound::Included(&e) => e + 1, + Bound::Excluded(&e) => e, + Bound::Unbounded => len, + }; + + assert!(start <= end); + assert!(start <= len); + assert!(end <= len); + (start, end) +} + +/// Normalize a byte slice and bit offset so that the returned offset is `< 8`. +#[inline] +fn normalize(buffer: &[u8], offset: usize) -> (&[u8], usize) { + let byte_offset = offset / 8; + (&buffer[byte_offset..], offset % 8) +} + +/// An immutable, borrowed view over a packed bitset. +/// +/// This is the borrowing analogue of [`BitBuffer`]: it stores a byte slice together with a bit +/// `offset` (always `< 8`) and a logical bit `len`, without owning or reference-counting the +/// backing allocation. Use it to read a bitset without cloning the underlying [`ByteBuffer`]. +#[derive(Debug, Clone, Copy)] +pub struct BitBufferView<'a> { + buffer: &'a [u8], + offset: usize, + len: usize, +} + +impl<'a> BitBufferView<'a> { + /// Create a new view over `buffer` with `len` bits, starting at bit zero. + /// + /// Panics if the buffer is not large enough to hold `len` bits. + pub fn new(buffer: &'a [u8], len: usize) -> Self { + Self::new_with_offset(buffer, len, 0) + } + + /// Create a new view over `buffer` with `len` bits, starting at the given bit `offset`. + /// + /// Panics if the buffer is not large enough to hold `len` bits after the offset. + pub fn new_with_offset(buffer: &'a [u8], len: usize, offset: usize) -> Self { + assert!( + len.saturating_add(offset) <= buffer.len().saturating_mul(8), + "provided slice (len={}) not large enough to back BitBufferView with offset {offset} len {len}", + buffer.len() + ); + + let (buffer, offset) = normalize(buffer, offset); + Self { + buffer, + offset, + len, + } + } + + /// Create a new view over `buffer` described by `meta`. + pub fn from_meta(buffer: &'a [u8], meta: BitBufferMeta) -> Self { + Self::new_with_offset(buffer, meta.len(), meta.offset()) + } + + /// Returns the [`BitBufferMeta`] (offset and length) describing this view. + pub fn meta(&self) -> BitBufferMeta { + BitBufferMeta::new(self.offset, self.len) + } + + /// Get the logical length of this view in bits. + #[inline] + pub fn len(&self) -> usize { + self.len + } + + /// Returns `true` if the view is empty. + #[inline] + pub fn is_empty(&self) -> bool { + self.len == 0 + } + + /// Offset of the start of the view in bits. Always `< 8`. + #[inline(always)] + pub fn offset(&self) -> usize { + self.offset + } + + /// Get a reference to the underlying byte slice. + #[inline(always)] + pub fn inner(&self) -> &'a [u8] { + self.buffer + } + + /// Retrieve the value at the given index. + /// + /// Panics if the index is out of bounds. + #[inline] + pub fn value(&self, index: usize) -> bool { + assert!(index < self.len); + // SAFETY: checked by assertion + unsafe { self.value_unchecked(index) } + } + + /// Retrieve the value at the given index without bounds checking. + /// + /// # Safety + /// + /// Caller must ensure that `index` is within the range of the view. + #[inline] + pub unsafe fn value_unchecked(&self, index: usize) -> bool { + unsafe { get_bit_unchecked(self.buffer.as_ptr(), index + self.offset) } + } + + /// Create a new view over the range `[start, end)` of this view. + /// + /// Panics if the slice would extend beyond the end of the view. + pub fn slice(&self, range: impl RangeBounds) -> BitBufferView<'a> { + let (start, end) = resolve_range(range, self.len); + BitBufferView::new_with_offset(self.buffer, end - start, self.offset + start) + } + + /// Access chunks of the buffer aligned to an 8 byte boundary as + /// `[prefix, , suffix]`. + pub fn unaligned_chunks(&self) -> UnalignedBitChunk<'a> { + UnalignedBitChunk::new(self.buffer, self.offset, self.len) + } + + /// Access chunks of the underlying buffer as 8 byte chunks with a final trailer. + pub fn chunks(&self) -> BitChunks<'a> { + BitChunks::new(self.buffer, self.offset, self.len) + } + + /// Get the number of set bits in the view. + pub fn true_count(&self) -> usize { + count_ones(self.buffer, self.offset, self.len) + } + + /// Get the number of unset bits in the view. + pub fn false_count(&self) -> usize { + self.len - self.true_count() + } + + /// Returns the position of the `nth` set bit (0-indexed), or `None` if out of range. + pub fn select(&self, nth: usize) -> Option { + bit_select(self.buffer, self.offset, self.len, nth) + } + + /// Iterator over bits in the view. + pub fn iter(&self) -> BitIterator<'a> { + BitIterator::new(self.buffer, self.offset, self.len) + } + + /// Iterator over set indices of the underlying buffer. + pub fn set_indices(&self) -> BitIndexIterator<'a> { + BitIndexIterator::new(self.buffer, self.offset, self.len) + } + + /// Iterator over set slices of the underlying buffer. + pub fn set_slices(&self) -> BitSliceIterator<'a> { + BitSliceIterator::new(self.buffer, self.offset, self.len) + } + + /// Copy this view into an owned [`BitBuffer`]. + pub fn to_bit_buffer(&self) -> BitBuffer { + let bytes = (self.offset + self.len).div_ceil(8); + BitBuffer::new_with_offset( + ByteBuffer::copy_from(&self.buffer[..bytes]), + self.len, + self.offset, + ) + } +} + +impl<'a> IntoIterator for BitBufferView<'a> { + type Item = bool; + type IntoIter = BitIterator<'a>; + + fn into_iter(self) -> Self::IntoIter { + self.iter() + } +} + +impl PartialEq for BitBufferView<'_> { + fn eq(&self, other: &Self) -> bool { + if self.len != other.len { + return false; + } + + self.chunks() + .iter_padded() + .zip(other.chunks().iter_padded()) + .all(|(a, b)| a == b) + } +} + +impl Eq for BitBufferView<'_> {} + +/// A mutable, borrowed view over a packed bitset. +/// +/// This is the borrowing analogue of [`BitBufferMut`]: it stores a mutable byte slice together +/// with a bit `offset` (always `< 8`) and a logical bit `len`. Unlike [`BitBufferMut`] it cannot +/// grow or reallocate, so it only supports in-place reads and writes (such as +/// [`set`](Self::set), [`unset`](Self::unset), and [`fill_range`](Self::fill_range)). +#[derive(Debug)] +pub struct BitBufferMutView<'a> { + buffer: &'a mut [u8], + offset: usize, + len: usize, +} + +impl<'a> BitBufferMutView<'a> { + /// Create a new mutable view over `buffer` with `len` bits, starting at bit zero. + /// + /// Panics if the buffer is not large enough to hold `len` bits. + pub fn new(buffer: &'a mut [u8], len: usize) -> Self { + Self::new_with_offset(buffer, len, 0) + } + + /// Create a new mutable view over `buffer` with `len` bits, starting at bit `offset`. + /// + /// Panics if the buffer is not large enough to hold `len` bits after the offset. + pub fn new_with_offset(buffer: &'a mut [u8], len: usize, offset: usize) -> Self { + assert!( + len.saturating_add(offset) <= buffer.len().saturating_mul(8), + "provided slice (len={}) not large enough to back BitBufferMutView with offset {offset} len {len}", + buffer.len() + ); + + let byte_offset = offset / 8; + let offset = offset % 8; + Self { + buffer: &mut buffer[byte_offset..], + offset, + len, + } + } + + /// Borrow this mutable view as an immutable [`BitBufferView`]. + #[inline] + pub fn as_view(&self) -> BitBufferView<'_> { + BitBufferView { + buffer: self.buffer, + offset: self.offset, + len: self.len, + } + } + + /// Get the logical length of this view in bits. + #[inline] + pub fn len(&self) -> usize { + self.len + } + + /// Returns `true` if the view is empty. + #[inline] + pub fn is_empty(&self) -> bool { + self.len == 0 + } + + /// Offset of the start of the view in bits. Always `< 8`. + #[inline(always)] + pub fn offset(&self) -> usize { + self.offset + } + + /// Get the underlying bytes as a slice. + #[inline] + pub fn as_slice(&self) -> &[u8] { + self.buffer + } + + /// Get the underlying bytes as a mutable slice. + #[inline] + pub fn as_mut_slice(&mut self) -> &mut [u8] { + self.buffer + } + + /// Retrieve the value at the given index. + /// + /// Panics if the index is out of bounds. + #[inline] + pub fn value(&self, index: usize) -> bool { + assert!(index < self.len); + // SAFETY: checked by assertion + unsafe { self.value_unchecked(index) } + } + + /// Retrieve the value at the given index without bounds checking. + /// + /// # Safety + /// + /// Caller must ensure that `index` is within the range of the view. + #[inline] + pub unsafe fn value_unchecked(&self, index: usize) -> bool { + unsafe { get_bit_unchecked(self.buffer.as_ptr(), index + self.offset) } + } + + /// Get the number of set bits in the view. + pub fn true_count(&self) -> usize { + self.as_view().true_count() + } + + /// Get the number of unset bits in the view. + pub fn false_count(&self) -> usize { + self.as_view().false_count() + } + + /// Iterator over bits in the view. + pub fn iter(&self) -> BitIterator<'_> { + self.as_view().iter() + } + + /// Set the bit at `index` to the given boolean value. + /// + /// Panics if `index` exceeds the view length. + pub fn set_to(&mut self, index: usize, value: bool) { + if value { + self.set(index); + } else { + self.unset(index); + } + } + + /// Set the bit at `index` to the given boolean value without bounds checking. + /// + /// # Safety + /// + /// Caller must ensure that `index` is within the range of the view. + pub unsafe fn set_to_unchecked(&mut self, index: usize, value: bool) { + if value { + // SAFETY: checked by caller + unsafe { self.set_unchecked(index) } + } else { + // SAFETY: checked by caller + unsafe { self.unset_unchecked(index) } + } + } + + /// Set the bit at `index` to `true`. + /// + /// Panics if `index` exceeds the view length. + pub fn set(&mut self, index: usize) { + assert!(index < self.len, "index {index} exceeds len {}", self.len); + // SAFETY: checked by assertion + unsafe { self.set_unchecked(index) }; + } + + /// Set the bit at `index` to `false`. + /// + /// Panics if `index` exceeds the view length. + pub fn unset(&mut self, index: usize) { + assert!(index < self.len, "index {index} exceeds len {}", self.len); + // SAFETY: checked by assertion + unsafe { self.unset_unchecked(index) }; + } + + /// Set the bit at `index` to `true` without bounds checking. + /// + /// # Safety + /// + /// Caller must ensure that `index` is within the range of the view. + #[inline] + pub unsafe fn set_unchecked(&mut self, index: usize) { + // SAFETY: checked by caller + unsafe { set_bit_unchecked(self.buffer.as_mut_ptr(), self.offset + index) } + } + + /// Set the bit at `index` to `false` without bounds checking. + /// + /// # Safety + /// + /// Caller must ensure that `index` is within the range of the view. + #[inline] + pub unsafe fn unset_unchecked(&mut self, index: usize) { + // SAFETY: checked by caller + unsafe { unset_bit_unchecked(self.buffer.as_mut_ptr(), self.offset + index) } + } + + /// Sets all bits in the range `[start, end)` to `value`. + /// + /// Panics if `end > self.len()` or `start > end`. + #[inline(always)] + pub fn fill_range(&mut self, start: usize, end: usize, value: bool) { + assert!(end <= self.len, "end {end} exceeds len {}", self.len); + assert!(start <= end, "start {start} exceeds end {end}"); + // SAFETY: assertions guarantee start <= end <= self.len. + unsafe { self.fill_range_unchecked(start, end, value) } + } + + /// Sets all bits in the range `[start, end)` to `value` without bounds checking. + /// + /// # Safety + /// + /// Caller must ensure that `start <= end <= self.len()`. + #[inline(always)] + pub unsafe fn fill_range_unchecked(&mut self, start: usize, end: usize, value: bool) { + fill_bits(self.buffer, self.offset + start, self.offset + end, value); + } + + /// Copy this view into an owned [`BitBuffer`]. + pub fn to_bit_buffer(&self) -> BitBuffer { + self.as_view().to_bit_buffer() + } +} + +impl BitBuffer { + /// Borrow this buffer as a [`BitBufferView`] without cloning the backing allocation. + #[inline] + pub fn as_view(&self) -> BitBufferView<'_> { + BitBufferView { + buffer: self.inner().as_slice(), + offset: self.offset(), + len: self.len(), + } + } +} + +impl BitBufferMut { + /// Borrow this buffer as an immutable [`BitBufferView`]. + #[inline] + pub fn as_view(&self) -> BitBufferView<'_> { + BitBufferView { + buffer: self.as_slice(), + offset: self.offset(), + len: self.len(), + } + } + + /// Borrow this buffer as a [`BitBufferMutView`]. + #[inline] + pub fn as_mut_view(&mut self) -> BitBufferMutView<'_> { + let offset = self.offset(); + let len = self.len(); + BitBufferMutView { + buffer: self.as_mut_slice(), + offset, + len, + } + } +} + +#[cfg(test)] +mod tests { + use crate::BitBuffer; + use crate::BitBufferMut; + use crate::bitbuffer; + + #[test] + fn view_reads_match_buffer() { + let buffer = bitbuffer![true, false, true, true, false, true, false, false]; + let view = buffer.as_view(); + + assert_eq!(view.len(), buffer.len()); + assert_eq!(view.true_count(), buffer.true_count()); + assert_eq!(view.false_count(), buffer.false_count()); + for i in 0..buffer.len() { + assert_eq!(view.value(i), buffer.value(i)); + } + assert_eq!( + view.iter().collect::>(), + buffer.iter().collect::>() + ); + } + + #[test] + fn view_slice_preserves_offset() { + let buffer = BitBuffer::new_set(20); + let sliced = buffer.slice(5..17); + let view = buffer.as_view().slice(5..17); + + assert_eq!(view.len(), sliced.len()); + assert_eq!(view.true_count(), sliced.true_count()); + assert_eq!(view.to_bit_buffer(), sliced); + } + + #[test] + fn view_offset_buffer() { + let buffer = BitBuffer::new_set(64).slice(3..40); + let view = buffer.as_view(); + assert_eq!(view.offset(), buffer.offset()); + assert_eq!(view.len(), buffer.len()); + assert_eq!(view.to_bit_buffer(), buffer); + } + + #[test] + fn mut_view_set_unset() { + let mut buffer = BitBufferMut::new_unset(16); + { + let mut view = buffer.as_mut_view(); + view.set(0); + view.set(15); + view.set_to(7, true); + view.fill_range(2, 5, true); + assert!(view.value(0)); + assert_eq!(view.true_count(), 6); + view.unset(0); + } + let frozen = buffer.freeze(); + assert!(!frozen.value(0)); + assert!(frozen.value(2)); + assert!(frozen.value(4)); + assert!(frozen.value(7)); + assert!(frozen.value(15)); + assert_eq!(frozen.true_count(), 5); + } + + #[test] + fn mut_view_with_offset() { + let mut buffer = BitBufferMut::from_buffer(crate::buffer_mut![0u8; 4], 3, 20); + { + let mut view = buffer.as_mut_view(); + assert_eq!(view.offset(), 3); + view.fill_range(0, 20, true); + } + let frozen = buffer.freeze(); + assert_eq!(frozen.true_count(), 20); + } +} diff --git a/vortex-cuda/src/arrow/canonical.rs b/vortex-cuda/src/arrow/canonical.rs index 9b5e5bb55ef..2aa89e8fb1c 100644 --- a/vortex-cuda/src/arrow/canonical.rs +++ b/vortex-cuda/src/arrow/canonical.rs @@ -137,14 +137,12 @@ fn export_canonical( Canonical::Bool(bool_array) => { let len = bool_array.len(); let validity = bool_array.validity()?; - let BoolDataParts { - bits, offset, len, .. - } = bool_array.into_data().into_parts(len); + let BoolDataParts { bits, meta } = bool_array.into_data().into_parts(len); check_validity_empty(&validity)?; let bits = ctx.ensure_on_device(bits).await?; - export_fixed_size(bits, len, offset, ctx) + export_fixed_size(bits, meta.len(), meta.offset(), ctx) } Canonical::VarBinView(varbinview) => { let len = varbinview.len(); diff --git a/vortex-cuda/src/canonical.rs b/vortex-cuda/src/canonical.rs index 591f1be404f..f38e1b8d554 100644 --- a/vortex-cuda/src/canonical.rs +++ b/vortex-cuda/src/canonical.rs @@ -75,11 +75,13 @@ impl CanonicalCudaExt for Canonical { // Also update other method to copy validity to host. let len = bool.len(); let validity = bool.validity()?; - let BoolDataParts { - bits, offset, len, .. - } = bool.into_data().into_parts(len); + let BoolDataParts { bits, meta } = bool.into_data().into_parts(len); - let bits = BitBuffer::new_with_offset(bits.try_into_host()?.await?, offset, len); + let bits = BitBuffer::new_with_offset( + bits.try_into_host()?.await?, + meta.offset(), + meta.len(), + ); Ok(Canonical::Bool(BoolArray::new(bits, validity))) } Canonical::Primitive(prim) => { diff --git a/vortex-duckdb/src/exporter/struct_.rs b/vortex-duckdb/src/exporter/struct_.rs index f18c5495754..76c07d672a3 100644 --- a/vortex-duckdb/src/exporter/struct_.rs +++ b/vortex-duckdb/src/exporter/struct_.rs @@ -43,7 +43,7 @@ pub(crate) fn new_exporter( let children = fields .iter() .map(|child| { - if validity.to_bit_buffer().true_count() != validity.len() { + if validity.bit_buffer_view().true_count() != validity.len() { // TODO(joe): use new mask. new_array_exporter( child.clone().mask(validity.clone().into_array())?, From b8e4310412c4a4da3add352acea5e5550186739b Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Tue, 2 Jun 2026 12:20:20 +0100 Subject: [PATCH 2/3] fix Signed-off-by: Joe Isaacs --- vortex-cuda/src/arrow/canonical.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/vortex-cuda/src/arrow/canonical.rs b/vortex-cuda/src/arrow/canonical.rs index 68d3b200e9f..9035d1b8875 100644 --- a/vortex-cuda/src/arrow/canonical.rs +++ b/vortex-cuda/src/arrow/canonical.rs @@ -145,7 +145,14 @@ fn export_canonical( export_arrow_validity_buffer(validity, len, offset, ctx).await?; let bits = ctx.ensure_on_device(bits).await?; - export_fixed_size(bits, meta.len(), meta.offset(), validity_buffer, null_count, ctx) + export_fixed_size( + bits, + meta.len(), + meta.offset(), + validity_buffer, + null_count, + ctx, + ) } Canonical::VarBinView(varbinview) => { let len = varbinview.len(); From 25d93a33c4ac7df036c9951c6c66621114a4009b Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Tue, 2 Jun 2026 11:27:50 +0000 Subject: [PATCH 3/3] Fix vortex-cuda after BoolDataParts rename - arrow/canonical.rs:145 still referenced a bare `offset` local that no longer exists after the destructure was updated to `{ bits, meta }`. Use `meta.offset()` so the validity buffer export matches the bit offset that's passed to `export_fixed_size` on the next line. - canonical.rs:80 passes the `meta` fields to `BitBuffer::new_with_offset(buffer, len, offset)` in the wrong order (offset was being passed as len, and vice versa). Swap them. Signed-off-by: Joe Isaacs --- vortex-cuda/src/arrow/canonical.rs | 2 +- vortex-cuda/src/canonical.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vortex-cuda/src/arrow/canonical.rs b/vortex-cuda/src/arrow/canonical.rs index 9035d1b8875..b8ad7f40ffa 100644 --- a/vortex-cuda/src/arrow/canonical.rs +++ b/vortex-cuda/src/arrow/canonical.rs @@ -142,7 +142,7 @@ fn export_canonical( let BoolDataParts { bits, meta } = bool_array.into_data().into_parts(len); let (validity_buffer, null_count) = - export_arrow_validity_buffer(validity, len, offset, ctx).await?; + export_arrow_validity_buffer(validity, len, meta.offset(), ctx).await?; let bits = ctx.ensure_on_device(bits).await?; export_fixed_size( diff --git a/vortex-cuda/src/canonical.rs b/vortex-cuda/src/canonical.rs index f38e1b8d554..4f3d6fd37e3 100644 --- a/vortex-cuda/src/canonical.rs +++ b/vortex-cuda/src/canonical.rs @@ -79,8 +79,8 @@ impl CanonicalCudaExt for Canonical { let bits = BitBuffer::new_with_offset( bits.try_into_host()?.await?, - meta.offset(), meta.len(), + meta.offset(), ); Ok(Canonical::Bool(BoolArray::new(bits, validity))) }