Skip to content

Commit 1269e82

Browse files
perf: exit compute_referenced_values_mask early once every value is referenced (#10423)
`DictArray::compute_referenced_values_mask` always visited every code, even after every dictionary value had been seen and the result could no longer change. It runs from the `min_max` and `is_constant` dict kernels whenever `all_values_referenced` is unset, which is the case after any slice, filter or take of a dict array. For a typical low-cardinality dictionary every value is usually seen within the first few hundred codes, so most of that work was wasted.
1 parent 345317b commit 1269e82

1 file changed

Lines changed: 144 additions & 23 deletions

File tree

‎vortex-array/src/arrays/dict/array.rs‎

Lines changed: 144 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -154,36 +154,111 @@ pub trait DictArrayExt: TypedArrayRef<Dict> + DictArraySlotsExt {
154154
let codes_primitive = codes.clone().execute::<PrimitiveArray>(ctx)?;
155155
let values_len = self.values().len();
156156

157-
let init_value = !referenced;
158-
let referenced_value = referenced;
159-
160-
let mut values_vec = vec![init_value; values_len];
161-
match codes_validity.bit_buffer() {
162-
AllOr::All => {
163-
match_each_integer_ptype!(codes_primitive.ptype(), |P| {
164-
for idx in codes_primitive.as_slice::<P>() {
165-
let idxu: usize = idx.as_();
166-
values_vec[idxu] = referenced_value;
167-
}
168-
});
169-
}
170-
AllOr::None => {}
171-
AllOr::Some(mask) => {
172-
match_each_integer_ptype!(codes_primitive.ptype(), |P| {
173-
let codes = codes_primitive.as_slice::<P>();
174-
mask.set_indices().for_each(|idx| {
175-
let idxu: usize = codes[idx].as_();
176-
values_vec[idxu] = referenced_value;
177-
});
178-
});
179-
}
157+
let mut values_vec = vec![!referenced; values_len];
158+
let saturated = match codes_validity.bit_buffer() {
159+
AllOr::All => match_each_integer_ptype!(codes_primitive.ptype(), |P| {
160+
mark_codes(&mut values_vec, referenced, codes_primitive.as_slice::<P>())
161+
}),
162+
AllOr::None => false,
163+
AllOr::Some(mask) => match_each_integer_ptype!(codes_primitive.ptype(), |P| {
164+
mark_masked_codes(
165+
&mut values_vec,
166+
referenced,
167+
codes_primitive.as_slice::<P>(),
168+
mask,
169+
codes_validity.true_count(),
170+
)
171+
}),
172+
};
173+
if saturated {
174+
return Ok(BitBuffer::full(referenced, values_len));
180175
}
181176

182177
Ok(BitBuffer::from(values_vec))
183178
}
184179
}
185180
impl<T: TypedArrayRef<Dict>> DictArrayExt for T {}
186181

182+
/// Number of codes visited between checks for whether every dictionary value is referenced.
183+
const EARLY_EXIT_CHUNK: usize = 2048;
184+
185+
/// Sets `values[code]` to `referenced` for every code, returning `true` early once every entry
186+
/// of `values` is `referenced`, since further codes cannot change the result.
187+
///
188+
/// With fewer codes than values not every value can be referenced, so the saturation checks are
189+
/// skipped entirely.
190+
fn mark_codes<P: AsPrimitive<usize>>(values: &mut [bool], referenced: bool, codes: &[P]) -> bool {
191+
if codes.len() < values.len() {
192+
for code in codes {
193+
values[code.as_()] = referenced;
194+
}
195+
return false;
196+
}
197+
198+
let mut cursor = 0;
199+
for chunk in codes.chunks(EARLY_EXIT_CHUNK) {
200+
for code in chunk {
201+
values[code.as_()] = referenced;
202+
}
203+
if is_saturated(values, &mut cursor, referenced) {
204+
return true;
205+
}
206+
}
207+
false
208+
}
209+
210+
/// Like [`mark_codes`], but only for the codes at the `valid_count` set positions of `mask`.
211+
fn mark_masked_codes<P: AsPrimitive<usize>>(
212+
values: &mut [bool],
213+
referenced: bool,
214+
codes: &[P],
215+
mask: &BitBuffer,
216+
valid_count: usize,
217+
) -> bool {
218+
if valid_count < values.len() {
219+
mask.set_indices().for_each(|idx| {
220+
values[codes[idx].as_()] = referenced;
221+
});
222+
return false;
223+
}
224+
225+
let mut cursor = 0;
226+
for start in (0..codes.len()).step_by(EARLY_EXIT_CHUNK) {
227+
let end = (start + EARLY_EXIT_CHUNK).min(codes.len());
228+
let codes = &codes[start..end];
229+
mask.slice(start..end).set_indices().for_each(|idx| {
230+
values[codes[idx].as_()] = referenced;
231+
});
232+
if is_saturated(values, &mut cursor, referenced) {
233+
return true;
234+
}
235+
}
236+
false
237+
}
238+
239+
/// Advances `cursor` past the entries of `values` equal to `referenced` and returns whether that
240+
/// is all of them. Callers share one cursor that only moves forward, so repeated checks cost
241+
/// `O(values.len())` in total.
242+
#[inline]
243+
fn is_saturated(values: &[bool], cursor: &mut usize, referenced: bool) -> bool {
244+
// Skip whole blocks with a branch-free comparison the compiler can vectorize, then find the
245+
// exact stopping point within the first block that is not fully marked.
246+
const BLOCK: usize = 32;
247+
let rest = &values[*cursor..];
248+
let full_blocks = rest
249+
.as_chunks::<BLOCK>()
250+
.0
251+
.iter()
252+
.take_while(|block| block.iter().fold(true, |acc, &v| acc & (v == referenced)))
253+
.count();
254+
*cursor += full_blocks * BLOCK;
255+
*cursor += values[*cursor..]
256+
.iter()
257+
.take_while(|&&v| v == referenced)
258+
.count();
259+
*cursor == values.len()
260+
}
261+
187262
/// Concrete parts of a [`DictArray`](super::DictArray) after iterative execution.
188263
pub struct DictParts {
189264
pub dtype: DType,
@@ -301,6 +376,7 @@ mod test {
301376
use crate::arrays::DictArray;
302377
use crate::arrays::PrimitiveArray;
303378
use crate::arrays::VarBinViewArray;
379+
use crate::arrays::dict::DictArrayExt;
304380
use crate::assert_arrays_eq;
305381
use crate::builders::VarBinBuilder;
306382
use crate::builders::builder_with_capacity_in;
@@ -510,4 +586,49 @@ mod test {
510586
.encode_to_vec(),
511587
);
512588
}
589+
590+
/// Covers the early exit taken once every value is referenced, including when that happens
591+
/// after the first chunk, alongside cases that never saturate.
592+
#[rstest::rstest]
593+
#[case::saturates_in_first_chunk(16, 5000, 16, None)]
594+
#[case::saturates_in_later_chunk(4096, 60_000, 4096, None)]
595+
#[case::saturates_unaligned_to_scan_block(4001, 60_000, 4001, None)]
596+
#[case::never_saturates(1024, 10_000, 1000, None)]
597+
#[case::fewer_codes_than_values(1024, 1000, 1024, None)]
598+
#[case::fewer_valid_codes_than_values(1024, 1500, 1024, Some(0.5))]
599+
#[case::saturates_with_nulls(64, 10_000, 64, Some(0.5))]
600+
#[case::never_saturates_with_nulls(64, 10_000, 63, Some(0.5))]
601+
fn referenced_values_mask_matches_reference(
602+
#[case] num_values: u32,
603+
#[case] num_codes: usize,
604+
#[case] referenced_range: u32,
605+
#[case] valid_fraction: Option<f64>,
606+
#[values(true, false)] referenced: bool,
607+
) -> VortexResult<()> {
608+
let mut rng = StdRng::seed_from_u64(0);
609+
let codes: Vec<Option<u32>> = (0..num_codes)
610+
.map(|_| {
611+
valid_fraction
612+
.is_none_or(|f| rng.random_bool(f))
613+
.then(|| rng.random_range(0..referenced_range))
614+
})
615+
.collect();
616+
617+
let mut expected = vec![!referenced; num_values as usize];
618+
for code in codes.iter().flatten() {
619+
expected[*code as usize] = referenced;
620+
}
621+
622+
let codes = match valid_fraction {
623+
Some(_) => PrimitiveArray::from_option_iter(codes).into_array(),
624+
None => PrimitiveArray::from_iter(codes.into_iter().flatten()).into_array(),
625+
};
626+
let dict =
627+
DictArray::try_new(codes, PrimitiveArray::from_iter(0..num_values).into_array())?;
628+
let mut ctx = array_session().create_execution_ctx();
629+
let mask = dict.compute_referenced_values_mask(referenced, &mut ctx)?;
630+
631+
assert_eq!(mask, BitBuffer::from(expected));
632+
Ok(())
633+
}
513634
}

0 commit comments

Comments
 (0)