Skip to content

Commit 2699169

Browse files
mhk197Matt KatzMatt KatzMatthew Katz
authored
perf: remove implicit ListViewArray rebuild during take and filter operations (#8048)
## Summary Removes implicit rebuilds from `ListViewArray` `take` and `filter` compute kernels, adds density-introspection methods to support deciding when to rebuild explicitly at materialization boundaries, and defers rebuilding until array export to duckdb and arrow. **Motivation.** The previous code rebuilt the elements buffer eagerly on every `take` or `filter` whose row-fraction dropped below `REBUILD_DENSITY_THRESHOLD`. In an execution tree like `take → take → ...`, an eager mid-pipeline rebuild costs an allocation and a full copy of referenced ranges that the next operator may immediately sparsify away. The row-fraction heuristic was also inaccurate. It doesn't account for per-row size variance, unreferenced elements, and duplicate references. Instead, `ListViewArray::estimate_density` uses sum of `sizes` instead of row-fraction. This will overestimate density when there are overlapping references, but it is typically preferable to not compact. **Changes:** - **Drop implicit rebuild from `TakeReduce::take` and `TakeExecute::take`** - **Drop implicit rebuild from `filter_listview`** - **Add methods to calculate reference density**: `compute_referenced_elements_mask`, `compute_density`, and `estimate_density` - **Estimate density and conditionally rebuild on export boundaries for duckdb and arrow** ## API Changes Adds `ListViewArray::estimate_density`, `ListViewArray::compute_density`, and `ListViewArray::compute_referenced_elements_mask`. ## Testing - New `vortex-array/src/arrays/listview/tests/density.rs` --------- Signed-off-by: Matthew Katz <katz@spiraldb.com> Co-authored-by: Matt Katz <mattkatz@Matts-MacBook-Pro.local> Co-authored-by: Matt Katz <mattkatz@Matts-MBP.localdomain> Co-authored-by: Matthew Katz <katz@spiraldb.com>
1 parent 495f30e commit 2699169

13 files changed

Lines changed: 334 additions & 62 deletions

File tree

vortex-array/public-api.lock

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4460,8 +4460,14 @@ pub vortex_array::arrays::listview::ListViewDataParts::sizes: vortex_array::Arra
44604460

44614461
pub vortex_array::arrays::listview::ListViewDataParts::validity: vortex_array::validity::Validity
44624462

4463+
pub const vortex_array::arrays::listview::DEFAULT_REBUILD_DENSITY_THRESHOLD: f32
4464+
44634465
pub trait vortex_array::arrays::listview::ListViewArrayExt: vortex_array::TypedArrayRef<vortex_array::arrays::ListView>
44644466

4467+
pub fn vortex_array::arrays::listview::ListViewArrayExt::compute_density(&self, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<f32>
4468+
4469+
pub fn vortex_array::arrays::listview::ListViewArrayExt::compute_referenced_elements_mask(&self, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_mask::Mask>
4470+
44654471
pub fn vortex_array::arrays::listview::ListViewArrayExt::elements(&self) -> &vortex_array::ArrayRef
44664472

44674473
pub fn vortex_array::arrays::listview::ListViewArrayExt::list_elements_at(&self, usize) -> vortex_error::VortexResult<vortex_array::ArrayRef>
@@ -4478,10 +4484,16 @@ pub fn vortex_array::arrays::listview::ListViewArrayExt::size_at(&self, usize) -
44784484

44794485
pub fn vortex_array::arrays::listview::ListViewArrayExt::sizes(&self) -> &vortex_array::ArrayRef
44804486

4487+
pub fn vortex_array::arrays::listview::ListViewArrayExt::upper_bound_density(&self, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<f32>
4488+
44814489
pub fn vortex_array::arrays::listview::ListViewArrayExt::verify_is_zero_copy_to_list(&self) -> bool
44824490

44834491
impl<T: vortex_array::TypedArrayRef<vortex_array::arrays::ListView>> vortex_array::arrays::listview::ListViewArrayExt for T
44844492

4493+
pub fn T::compute_density(&self, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<f32>
4494+
4495+
pub fn T::compute_referenced_elements_mask(&self, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_mask::Mask>
4496+
44854497
pub fn T::elements(&self) -> &vortex_array::ArrayRef
44864498

44874499
pub fn T::list_elements_at(&self, usize) -> vortex_error::VortexResult<vortex_array::ArrayRef>
@@ -4498,6 +4510,8 @@ pub fn T::size_at(&self, usize) -> usize
44984510

44994511
pub fn T::sizes(&self) -> &vortex_array::ArrayRef
45004512

4513+
pub fn T::upper_bound_density(&self, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<f32>
4514+
45014515
pub fn T::verify_is_zero_copy_to_list(&self) -> bool
45024516

45034517
pub fn vortex_array::arrays::listview::list_from_list_view(vortex_array::arrays::ListViewArray) -> vortex_error::VortexResult<vortex_array::arrays::ListArray>

vortex-array/src/arrays/filter/execute/listview.rs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ use vortex_mask::MaskValues;
99

1010
use crate::arrays::ListViewArray;
1111
use crate::arrays::filter::execute::filter_validity;
12-
use crate::arrays::listview;
1312
use crate::arrays::listview::ListViewArrayExt;
14-
use crate::arrays::listview::ListViewRebuildMode;
1513

1614
/// [`ListViewArray`] filter implementation.
1715
///
@@ -55,18 +53,7 @@ pub fn filter_listview(array: &ListViewArray, selection_mask: &Arc<MaskValues>)
5553
// - Offsets and sizes are derived from existing valid child arrays.
5654
// - Offsets and sizes have the same length (both filtered by `selection_mask`).
5755
// - Validity matches the filtered array's nullability.
58-
let new_array = unsafe {
59-
ListViewArray::new_unchecked(elements.clone(), new_offsets, new_sizes, new_validity)
60-
};
61-
62-
let kept_row_fraction = selection_mask.true_count() as f32 / array.sizes().len() as f32;
63-
if kept_row_fraction < listview::compute::REBUILD_DENSITY_THRESHOLD {
64-
new_array
65-
.rebuild(ListViewRebuildMode::MakeZeroCopyToList)
66-
.vortex_expect("ListViewArray rebuild to zero-copy List should always succeed")
67-
} else {
68-
new_array
69-
}
56+
unsafe { ListViewArray::new_unchecked(elements.clone(), new_offsets, new_sizes, new_validity) }
7057
}
7158

7259
#[cfg(test)]

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

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@ use std::sync::Arc;
77

88
use num_traits::AsPrimitive;
99
use smallvec::smallvec;
10+
use vortex_buffer::BitBufferMut;
1011
use vortex_error::VortexExpect;
1112
use vortex_error::VortexResult;
1213
use vortex_error::vortex_bail;
1314
use vortex_error::vortex_ensure;
1415
use vortex_error::vortex_err;
16+
use vortex_mask::Mask;
1517

1618
use crate::ArrayRef;
1719
use crate::ArraySlots;
20+
use crate::ExecutionCtx;
1821
use crate::LEGACY_SESSION;
1922
#[expect(deprecated)]
2023
use crate::ToCanonical as _;
@@ -30,6 +33,7 @@ use crate::arrays::PrimitiveArray;
3033
use crate::arrays::bool;
3134
use crate::dtype::DType;
3235
use crate::dtype::IntegerPType;
36+
use crate::expr::stats::Stat;
3337
use crate::match_each_integer_ptype;
3438
use crate::validity::Validity;
3539

@@ -311,6 +315,31 @@ impl Default for ListViewData {
311315
}
312316
}
313317

318+
/// Walks parallel `(offset, size)` slices and sets each range `[offset, offset + size]` in `buf`.
319+
///
320+
/// **Preconditions**
321+
///
322+
/// `offsets` and `sizes` must be the same length (which is always the case in valid `ListViewArray`s).
323+
fn fill_referenced_mask<O: IntegerPType, S: IntegerPType>(
324+
buf: &mut BitBufferMut,
325+
offsets: &[O],
326+
sizes: &[S],
327+
) {
328+
let len = offsets.len();
329+
330+
assert_eq!(
331+
len,
332+
sizes.len(),
333+
"offsets and sizes must be the same length"
334+
);
335+
336+
for i in 0..len {
337+
let start: usize = offsets[i].as_();
338+
let size: usize = sizes[i].as_();
339+
buf.fill_range(start, start + size, true);
340+
}
341+
}
342+
314343
pub trait ListViewArrayExt: TypedArrayRef<ListView> {
315344
fn nullability(&self) -> crate::dtype::Nullability {
316345
match self.as_ref().dtype() {
@@ -396,6 +425,94 @@ pub trait ListViewArrayExt: TypedArrayRef<ListView> {
396425
let sizes_primitive = self.sizes().to_primitive();
397426
validate_zctl(self.elements(), offsets_primitive, sizes_primitive).is_ok()
398427
}
428+
429+
/// Returns a [`Mask`] of length `elements.len()` where each bit is set iff that
430+
/// position in `elements` is referenced by at least one view. Caller must ensure `elements`
431+
/// is non-empty.
432+
///
433+
/// Walks every `(offset, size)` pair, canonicalizes both `offsets` and `sizes`,
434+
/// and allocates a `BitBuffer` of length `elements.len()`, so it is extremely costly.
435+
///
436+
/// **Preconditions**
437+
///
438+
/// `self.elements()` must be non-empty.
439+
fn compute_referenced_elements_mask(&self, ctx: &mut ExecutionCtx) -> VortexResult<Mask> {
440+
assert!(!self.elements().is_empty());
441+
let len = self.elements().len();
442+
443+
let offsets_primitive = self.offsets().clone().execute::<PrimitiveArray>(ctx)?;
444+
let sizes_primitive = self.sizes().clone().execute::<PrimitiveArray>(ctx)?;
445+
446+
let mut buf = BitBufferMut::new_unset(len);
447+
448+
match_each_integer_ptype!(offsets_primitive.ptype(), |O| {
449+
match_each_integer_ptype!(sizes_primitive.ptype(), |S| {
450+
fill_referenced_mask::<O, S>(
451+
&mut buf,
452+
offsets_primitive.as_slice::<O>(),
453+
sizes_primitive.as_slice::<S>(),
454+
);
455+
})
456+
});
457+
458+
Ok(Mask::from_buffer(buf.freeze()))
459+
}
460+
461+
/// Exact fraction of `elements` referenced by some view, in `[0.0, 1.0]`. Extremely costly.
462+
///
463+
/// Returns `Ok(1.0)` when `elements` is empty instead of dividing by 0.
464+
fn compute_density(&self, ctx: &mut ExecutionCtx) -> VortexResult<f32> {
465+
if self.elements().is_empty() {
466+
return Ok(1.0);
467+
}
468+
469+
if self.sizes().is_empty() {
470+
return Ok(0.0);
471+
}
472+
473+
let density = match self.compute_referenced_elements_mask(ctx)? {
474+
Mask::AllTrue(_) => 1.0,
475+
Mask::AllFalse(_) => 0.0,
476+
Mask::Values(values) => values.true_count() as f32 / self.elements().len() as f32,
477+
};
478+
479+
Ok(density)
480+
}
481+
482+
/// Upper-bound estimate of [`compute_density`](Self::compute_density) via
483+
/// `sum(sizes) / elements.len()`, clamped to `[0.0, 1.0]`.
484+
///
485+
/// Exact for non-overlapping views, but overcounts when multiple views share the same elements.
486+
///
487+
/// Returns `Ok(1.0)` when `elements` is empty instead of dividing by 0.
488+
fn upper_bound_density(&self, ctx: &mut ExecutionCtx) -> VortexResult<f32> {
489+
let n_elts = self.elements().len();
490+
if n_elts == 0 {
491+
return Ok(1.0);
492+
}
493+
494+
let sizes = self.sizes();
495+
if sizes.is_empty() {
496+
return Ok(0.0);
497+
}
498+
499+
// compute_stat short-circuits on a cached exact Sum and otherwise computes
500+
let sizes_sum = sizes
501+
.statistics()
502+
.compute_stat(Stat::Sum, ctx)?
503+
.vortex_expect("sizes array has integer ptype elements")
504+
.as_primitive()
505+
.as_::<u64>()
506+
.vortex_expect("integer ptypes can be upcast to u64");
507+
508+
// if the same elements are referenced more than once the estimate may be
509+
// greater than 1.0, so clamp
510+
let estimate = (sizes_sum as f32 / n_elts as f32).min(1.0);
511+
512+
debug_assert!(estimate >= 0.0);
513+
514+
Ok(estimate)
515+
}
399516
}
400517
impl<T: TypedArrayRef<ListView>> ListViewArrayExt for T {}
401518

vortex-array/src/arrays/listview/compute/mod.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,3 @@ mod mask;
66
pub(crate) mod rules;
77
mod slice;
88
mod take;
9-
10-
/// The threshold below which we rebuild the elements of a listview.
11-
///
12-
/// We don't touch `elements` on the metadata-only path since reorganizing it can be expensive.
13-
/// However, we also don't want to drag around a large amount of garbage data when the selection
14-
/// is sparse. Below this fraction of list rows retained, the rebuild is worth it.
15-
/// Rebuilding is needed when exporting the ListView's elements.
16-
///
17-
// TODO(connor)[ListView]: Ideally, we would only rebuild after all `take`s and `filter`
18-
// compute functions have run, at the "top" of the operator tree. However, we cannot do this
19-
// right now, so we will just rebuild every time (similar to [`ListArray`]).
20-
pub(crate) const REBUILD_DENSITY_THRESHOLD: f32 = 0.1;

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

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
use num_traits::Zero;
55
use vortex_error::VortexResult;
66

7-
use super::REBUILD_DENSITY_THRESHOLD;
87
use crate::ArrayRef;
98
use crate::ExecutionCtx;
109
use crate::IntoArray;
@@ -14,7 +13,6 @@ use crate::arrays::ListViewArray;
1413
use crate::arrays::dict::TakeExecute;
1514
use crate::arrays::dict::TakeReduce;
1615
use crate::arrays::listview::ListViewArrayExt;
17-
use crate::arrays::listview::ListViewRebuildMode;
1816
use crate::builtins::ArrayBuiltins;
1917
use crate::dtype::Nullability;
2018
use crate::match_each_integer_ptype;
@@ -23,43 +21,18 @@ use crate::scalar::Scalar;
2321
/// Metadata-only take for [`ListViewArray`].
2422
impl TakeReduce for ListView {
2523
fn take(array: ArrayView<'_, ListView>, indices: &ArrayRef) -> VortexResult<Option<ArrayRef>> {
26-
// Approximate element density by the fraction of list rows retained. Assumes roughly
27-
// uniform list sizes; good enough to decide whether dragging along the full `elements`
28-
// buffer is worth avoiding a rebuild.
29-
let kept_row_fraction = indices.len() as f32 / array.sizes().len() as f32;
30-
if kept_row_fraction < REBUILD_DENSITY_THRESHOLD {
31-
return Ok(None);
32-
}
33-
3424
Ok(Some(apply_take(array, indices)?.into_array()))
3525
}
3626
}
3727

3828
/// Execution-path take for [`ListViewArray`].
39-
///
40-
/// This does the same metadata-only take as [`TakeReduce`], but also rebuilds the array if the
41-
/// resulting array will be less dense than `REBUILD_DENSITY_THRESHOLD`.
4229
impl TakeExecute for ListView {
4330
fn take(
4431
array: ArrayView<'_, ListView>,
4532
indices: &ArrayRef,
4633
_ctx: &mut ExecutionCtx,
4734
) -> VortexResult<Option<ArrayRef>> {
48-
let kept_row_fraction = indices.len() as f32 / array.sizes().len() as f32;
49-
let taken = apply_take(array, indices)?;
50-
51-
if kept_row_fraction < REBUILD_DENSITY_THRESHOLD {
52-
// TODO(connor)[ListView]: Ideally, we would only rebuild after all `take`s and `filter`
53-
// compute functions have run, at the "top" of the operator tree. However, we cannot do
54-
// this right now, so we will just rebuild every time (similar to `ListArray`).
55-
Ok(Some(
56-
taken
57-
.rebuild(ListViewRebuildMode::MakeZeroCopyToList)?
58-
.into_array(),
59-
))
60-
} else {
61-
Ok(Some(taken.into_array()))
62-
}
35+
Ok(Some(apply_take(array, indices)?.into_array()))
6336
}
6437
}
6538

vortex-array/src/arrays/listview/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub use conversion::list_view_from_list;
1818
pub use conversion::recursive_list_from_list_view;
1919

2020
mod rebuild;
21+
pub use rebuild::DEFAULT_REBUILD_DENSITY_THRESHOLD;
2122
pub use rebuild::ListViewRebuildMode;
2223

2324
#[cfg(test)]

vortex-array/src/arrays/listview/rebuild.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@ use crate::match_each_integer_ptype;
2525
use crate::scalar::Scalar;
2626
use crate::scalar_fn::fns::operators::Operator;
2727

28+
/// Density threshold to decide whether to rebuild a sparse `ListViewArray`.
29+
///
30+
/// A `ListViewArray` can accumulate unreferenced bytes in its `elements` buffer after
31+
/// metadata-only operations like `take` and `filter`. When density (referenced fraction of `elements`)
32+
/// falls below this threshold, the benefits of a rebuild may outweigh its cost.
33+
///
34+
/// This is a somewhat arbitrary rule-of-thumb and may be suboptimal depending on different use cases and
35+
/// list element dtypes.
36+
pub const DEFAULT_REBUILD_DENSITY_THRESHOLD: f32 = 0.1;
37+
2838
/// Modes for rebuilding a [`ListViewArray`].
2939
pub enum ListViewRebuildMode {
3040
/// Removes all unused data and flattens out all list data, such that the array is zero-copyable

vortex-array/src/arrays/listview/tests/common.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ pub fn create_basic_listview() -> ListViewArray {
2222
}
2323
}
2424

25+
/// Creates a sparse ListView with two overlap regions
26+
/// `[[0,1,2], [1,2], [18, 19], [19]]` over 20 elements.
27+
pub fn create_sparse_overlapping_listview() -> ListViewArray {
28+
let elements = buffer![0i32..20].into_array();
29+
let offsets = buffer![0u32, 1, 18, 19].into_array();
30+
let sizes = buffer![3u32, 2, 2, 1].into_array();
31+
ListViewArray::new(elements, offsets, sizes, Validity::NonNullable)
32+
}
33+
2534
/// Creates a nullable ListView: [[10,20], null, [50]]
2635
pub fn create_nullable_listview() -> ListViewArray {
2736
let elements = buffer![10i32, 20, 30, 40, 50].into_array();
@@ -45,6 +54,17 @@ pub fn create_empty_lists_listview() -> ListViewArray {
4554
}
4655
}
4756

57+
/// Creates a ListView with empty lists and elements: [[]]
58+
pub fn create_empty_elements_listview() -> ListViewArray {
59+
let elements = PrimitiveArray::from_iter::<[i32; 0]>([]).into_array();
60+
let offsets = buffer![0u32; 0].into_array();
61+
let sizes = buffer![0u32; 0].into_array();
62+
unsafe {
63+
ListViewArray::new_unchecked(elements, offsets, sizes, Validity::NonNullable)
64+
.with_zero_copy_to_list(true)
65+
}
66+
}
67+
4868
/// Creates a ListView with overlapping lists and out-of-order offsets
4969
/// Lists: [[5,6,7], [2,3], [8,9], [0,1], [1,2,3,4]]
5070
pub fn create_overlapping_listview() -> ListViewArray {

0 commit comments

Comments
 (0)