Skip to content

Commit 438600d

Browse files
Read integer validity a word per chunk, and fill nulls for runs (#10342)
## Summary Stacked on #10341, which adds the benchmarks. Integer statistics with nulls sliced the validity `BitBuffer` for every chunk of 64 values to count its set bits, then tested each value's bit with a branch. With nulls spread out, almost no chunk is fully valid (0.9⁶⁴ ≈ 0.1% at 10% nulls), so nearly every chunk took that slow path. ## Changes In `vortex-compressor/src/stats/integer.rs`: - Read the validity one `u64` word per chunk instead of slicing a bit buffer. A word of zeros is skipped, and a full word takes the non-null loop as before. - Without distinct values, fill each null with the valid value before it. A filled null adds no run, so the chunk takes the same branch-free run count as a non-null chunk. - With distinct values, push only the set bits, since a filled null would count as an occurrence. - The trailing values are padded into a last chunk whose padding is null. This replaces the separate per-bit remainder loop. Min/max is unchanged: it is still computed by the `min_max` aggregate before this loop. `test_matches_naive_reference` now also covers statistics without distinct values and chunks that are entirely null. Filling the nulls with their own values instead makes 38 of its cases fail. --------- Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9324b04 commit 438600d

1 file changed

Lines changed: 59 additions & 72 deletions

File tree

‎vortex-compressor/src/stats/integer.rs‎

Lines changed: 59 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use vortex_array::expr::stats::Stat;
1515
use vortex_array::match_each_integer_ptype;
1616
use vortex_array::scalar::PValue;
1717
use vortex_array::scalar::Scalar;
18-
use vortex_buffer::BitBuffer;
1918
use vortex_error::VortexError;
2019
use vortex_error::VortexExpect;
2120
use vortex_error::VortexResult;
@@ -394,48 +393,42 @@ where
394393

395394
let sliced = buffer.slice(head_idx..array.len());
396395
let (chunks, remainder) = sliced.as_slice().as_chunks::<64>();
397-
match validity.bit_buffer() {
396+
let remainder_valid = match validity.bit_buffer() {
398397
AllOr::All => {
399398
for chunk in chunks {
400399
inner_loop_nonnull(chunk, count_distinct_values, &mut loop_state)
401400
}
402-
inner_loop_naive(
403-
remainder,
404-
count_distinct_values,
405-
&BitBuffer::new_set(remainder.len()),
406-
&mut loop_state,
407-
);
401+
(1u64 << remainder.len()) - 1
408402
}
409403
AllOr::None => unreachable!("All invalid arrays have been handled before"),
410404
AllOr::Some(v) => {
411405
let mask = v.slice(head_idx..array.len());
412-
let mut offset = 0;
413-
for chunk in chunks {
414-
let validity = mask.slice(offset..(offset + 64));
415-
offset += 64;
416-
417-
match validity.true_count() {
406+
let words = mask.chunks();
407+
// One validity word per chunk of 64 values.
408+
for (chunk, valid) in chunks.iter().zip(words.iter()) {
409+
match valid {
418410
// All nulls -> no stats to update.
419-
0 => continue,
411+
0 => {}
420412
// Inner loop for when validity check can be elided.
421-
64 => inner_loop_nonnull(chunk, count_distinct_values, &mut loop_state),
413+
u64::MAX => inner_loop_nonnull(chunk, count_distinct_values, &mut loop_state),
422414
// Inner loop for when we need to check validity.
423-
_ => inner_loop_nullable(
424-
chunk,
425-
count_distinct_values,
426-
&validity,
427-
&mut loop_state,
428-
),
415+
_ => inner_loop_nullable(chunk, count_distinct_values, valid, &mut loop_state),
429416
}
430417
}
431-
// Final iteration, run naive loop.
432-
inner_loop_naive(
433-
remainder,
434-
count_distinct_values,
435-
&mask.slice(offset..(offset + remainder.len())),
436-
&mut loop_state,
437-
);
418+
words.remainder_bits()
438419
}
420+
};
421+
422+
// Pad the trailing values into a last chunk, whose padding is null.
423+
if remainder_valid != 0 {
424+
let mut last = [loop_state.prev; 64];
425+
last[..remainder.len()].copy_from_slice(remainder);
426+
inner_loop_nullable(
427+
&last,
428+
count_distinct_values,
429+
remainder_valid,
430+
&mut loop_state,
431+
);
439432
}
440433

441434
if count_distinct_values {
@@ -653,57 +646,45 @@ fn inner_loop_nonnull_impl<T: IntegerPType, const COUNT_DISTINCT_VALUES: bool>(
653646
state.prev = values[63];
654647
}
655648

656-
/// Inner loop for nullable chunks of 64 values.
649+
/// Inner loop for chunks of 64 values with nulls, whose bits are unset in `valid`.
657650
#[allow(clippy::inline_always)]
658651
#[inline(always)]
659652
fn inner_loop_nullable<T: IntegerPType>(
660653
values: &[T; 64],
661654
count_distinct_values: bool,
662-
is_valid: &BitBuffer,
655+
valid: u64,
663656
state: &mut LoopState<T>,
664657
) where
665658
NativeValue<T>: Eq + Hash,
666659
{
667660
if count_distinct_values {
668-
inner_loop_masked::<T, true>(values, is_valid, state);
669-
} else {
670-
inner_loop_masked::<T, false>(values, is_valid, state);
671-
}
672-
}
673-
674-
/// Fallback inner loop for remainder values.
675-
#[allow(clippy::inline_always)]
676-
#[inline(always)]
677-
fn inner_loop_naive<T: IntegerPType>(
678-
values: &[T],
679-
count_distinct_values: bool,
680-
is_valid: &BitBuffer,
681-
state: &mut LoopState<T>,
682-
) where
683-
NativeValue<T>: Eq + Hash,
684-
{
685-
if count_distinct_values {
686-
inner_loop_masked::<T, true>(values, is_valid, state);
661+
// A filled null would count as an occurrence, so only the valid values are pushed.
662+
let mut valid = valid;
663+
while valid != 0 {
664+
state.push::<true>(values[valid.trailing_zeros() as usize]);
665+
valid &= valid - 1;
666+
}
687667
} else {
688-
inner_loop_masked::<T, false>(values, is_valid, state);
668+
// A null filled with the valid value before it adds no run, so the chunk takes the
669+
// branch-free loop of non-null chunks.
670+
inner_loop_nonnull_impl::<T, false>(&forward_fill(values, valid, state.prev), state);
689671
}
690672
}
691673

692-
/// Processes values with a validity mask, skipping nulls without breaking runs.
674+
/// Returns `values` with each null, whose bit is unset in `valid`, replaced by the closest valid
675+
/// value before it, or by `prev`, the last valid value before the chunk. The cost is one store per
676+
/// null.
693677
#[allow(clippy::inline_always)]
694678
#[inline(always)]
695-
fn inner_loop_masked<T: IntegerPType, const COUNT_DISTINCT_VALUES: bool>(
696-
values: &[T],
697-
is_valid: &BitBuffer,
698-
state: &mut LoopState<T>,
699-
) where
700-
NativeValue<T>: Eq + Hash,
701-
{
702-
for (idx, &value) in values.iter().enumerate() {
703-
if is_valid.value(idx) {
704-
state.push::<COUNT_DISTINCT_VALUES>(value);
705-
}
706-
}
679+
fn forward_fill<T: Copy>(values: &[T; 64], valid: u64, prev: T) -> [T; 64] {
680+
let mut filled = *values;
681+
let mut nulls = !valid;
682+
while nulls != 0 {
683+
let i = nulls.trailing_zeros() as usize;
684+
filled[i] = if i == 0 { prev } else { filled[i - 1] };
685+
nulls &= nulls - 1;
686+
}
687+
filled
707688
}
708689

709690
#[cfg(test)]
@@ -853,16 +834,20 @@ mod tests {
853834

854835
/// Checks the chunked loops against a naive reference. The run lengths cover aligned
855836
/// constant chunks (64), runs that cross chunk boundaries (3, 100), and chunks where every
856-
/// value changes (1).
837+
/// value changes (1). With nulls, `null_chunks` also nulls two whole chunks.
857838
#[rstest]
858839
fn test_matches_naive_reference(
859840
#[values(1, 63, 64, 65, 64 * 20 + 17)] len: u32,
860841
#[values(1, 3, 64, 100)] run_len: u32,
861842
#[values(None, Some(97), Some(3))] null_every: Option<u32>,
843+
#[values(false, true)] null_chunks: bool,
844+
#[values(false, true)] count_distinct_values: bool,
862845
) -> VortexResult<()> {
863846
let values: Vec<u32> = (0..len).map(|i| (i / run_len) % 16).collect();
864847
let valid: Vec<bool> = (0..len)
865-
.map(|i| null_every.is_none_or(|n| i % n != 0))
848+
.map(|i| {
849+
null_every.is_none_or(|n| i % n != 0 && !(null_chunks && (64..192).contains(&i)))
850+
})
866851
.collect();
867852
let (expected, runs) = naive_stats(&values, &valid);
868853

@@ -872,16 +857,18 @@ mod tests {
872857
};
873858
let array = PrimitiveArray::new(Buffer::from(values), validity);
874859
let mut ctx = array_session().create_execution_ctx();
875-
let stats = typed_int_stats::<u32>(&array, true, &mut ctx)?;
860+
let stats = typed_int_stats::<u32>(&array, count_distinct_values, &mut ctx)?;
876861

877862
let ErasedStats::U32(typed) = stats.erased() else {
878863
unreachable!()
879864
};
880-
let actual: HashMap<u32, u32> = typed
881-
.distinct()
882-
.map(|d| d.distinct_values().iter().map(|(k, &c)| (k.0, c)).collect())
883-
.unwrap_or_default();
884-
assert_eq!(actual, expected);
865+
if count_distinct_values {
866+
let actual: HashMap<u32, u32> = typed
867+
.distinct()
868+
.map(|d| d.distinct_values().iter().map(|(k, &c)| (k.0, c)).collect())
869+
.unwrap_or_default();
870+
assert_eq!(actual, expected);
871+
}
885872
let value_count: u32 = expected.values().sum();
886873
assert_eq!(stats.value_count, value_count);
887874
if value_count > 0 {

0 commit comments

Comments
 (0)