Skip to content

Commit 5eff4dc

Browse files
committed
Skip per-split zone-mask expansion when covering zones are uniform
`ZonedReader::pruning_evaluation` expanded the cached zone-level pruning mask into a row-aligned bit buffer for every split, then intersected it with the incoming mask. That cost a `Vec` of zone lengths (built eagerly, before the future was polled), a `BitBufferMut` of the full split length, a popcount over it, and a bitand, on every split of every scan. For most splits the zones covering that split are uniform: either none are pruned or all of them are. Both collapse to a constant stats mask, so counting the covered zone bits first - a handful of bit reads via `BitBuffer::count_range` - lets us skip the expansion entirely: - no covered zone pruned: the stats mask is all-true, so forward the incoming mask unchanged, - every covered zone pruned: return `Mask::new_false` directly, - otherwise: fall through to the existing expansion. The zone-length computation now lives on the non-uniform path, so uniform splits allocate nothing. Results and masks are unchanged. This is a simplification, not a measured speedup. The full `bench-sql` suite (TPC-H SF=1 and SF=10, TPC-DS, ClickBench, ClickBench Sorted, FineWeb, PolarSignals) returned "No clear signal" on every benchmark, scattered between -2.8% and +3.3% around zero. TPC-H SF=10 is the most trustworthy of those - its Parquet control spans only 0.96-1.03 - and it came back at -0.3%. Whatever is saved per split is too small a share of end-to-end query time to measure; the argument for the change is that the common path no longer allocates. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent c1ae775 commit 5eff4dc

1 file changed

Lines changed: 85 additions & 35 deletions

File tree

vortex-layout/src/layouts/zoned/reader.rs

Lines changed: 85 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,15 @@ use std::ops::Range;
66
use std::sync::Arc;
77

88
use futures::future::BoxFuture;
9-
use itertools::Itertools;
109
use tracing::trace;
1110
use vortex_array::ArrayRef;
1211
use vortex_array::MaskFuture;
1312
use vortex_array::dtype::DType;
1413
use vortex_array::dtype::FieldMask;
1514
use vortex_array::expr::BoundExpression;
1615
use vortex_buffer::BitBufferMut;
17-
use vortex_error::VortexError;
1816
use vortex_error::VortexResult;
17+
use vortex_mask::AllOr;
1918
use vortex_mask::Mask;
2019
use vortex_session::VortexSession;
2120

@@ -97,13 +96,13 @@ impl ZonedReader {
9796
let zone_end = row_range.end.div_ceil(zone_len_u64);
9897
zone_start..zone_end
9998
}
99+
}
100100

101-
/// Get the row index for the first row in a zone with the given `zone_index`.
102-
pub(crate) fn first_row_offset(&self, zone_idx: u64) -> u64 {
103-
zone_idx
104-
.saturating_mul(self.zone_len as u64)
105-
.min(self.row_count)
106-
}
101+
/// Get the row index for the first row in a zone with the given `zone_idx`.
102+
///
103+
/// Free function so that it can be used from a `'static` future without capturing the reader.
104+
fn first_row_offset(zone_idx: u64, zone_len: u64, row_count: u64) -> u64 {
105+
zone_idx.saturating_mul(zone_len).min(row_count)
107106
}
108107

109108
impl LayoutReader for ZonedReader {
@@ -149,44 +148,64 @@ impl LayoutReader for ZonedReader {
149148
return Ok(data_eval);
150149
};
151150

152-
let row_count = row_range.end - row_range.start;
151+
let split_row_count = row_range.end - row_range.start;
152+
let row_start = row_range.start;
153153
let zone_range = self.zone_range(row_range);
154-
let zone_lengths: Vec<_> = zone_range
155-
.clone()
156-
.map(|zone_idx| {
157-
// Figure out the range in the mask that corresponds to the zone
158-
let start = usize::try_from(
159-
self.first_row_offset(zone_idx)
160-
.saturating_sub(row_range.start),
161-
)?;
162-
let end = usize::try_from(
163-
self.first_row_offset(zone_idx + 1)
164-
.saturating_sub(row_range.start)
165-
.min(row_count),
166-
)?;
167-
Ok::<_, VortexError>(end - start)
168-
})
169-
.try_collect()?;
154+
let zone_start = usize::try_from(zone_range.start)?;
155+
let zone_end = usize::try_from(zone_range.end)?;
156+
let covered_zones = zone_end - zone_start;
157+
let zone_len = self.zone_len as u64;
158+
let layout_row_count = self.row_count;
170159

171160
let name = Arc::clone(&self.name);
172161
let expr = expr.clone();
162+
let mask_len = mask.len();
173163

174-
Ok(MaskFuture::new(mask.len(), async move {
164+
Ok(MaskFuture::new(mask_len, async move {
175165
trace!("Invoking stats pruning evaluation {}: {}", name, expr);
176166

177167
let pruning_mask = pruning_mask_future.await?.mask()?;
178168

179-
let mut builder = BitBufferMut::with_capacity(mask.len());
180-
for (zone_idx, &zone_length) in zone_range.clone().zip_eq(&zone_lengths) {
181-
builder.append_n(!pruning_mask.value(usize::try_from(zone_idx)?), zone_length);
182-
}
169+
// Only the zones covering this row range matter. Counting their pruned bits is a
170+
// handful of bit reads, and the overwhelming majority of splits are uniform: either
171+
// no covered zone is pruned, or all of them are. Both collapse to a constant stats
172+
// mask, so we can skip expanding zones into a row-aligned buffer entirely.
173+
let pruned_zones = match pruning_mask.bit_buffer() {
174+
AllOr::All => covered_zones,
175+
AllOr::None => 0,
176+
AllOr::Some(buffer) => buffer.count_range(zone_start, zone_end),
177+
};
183178

184-
let stats_mask = Mask::from(builder.freeze());
185-
assert_eq!(stats_mask.len(), mask.len(), "Mask length mismatch");
186-
187-
// Intersect the masks.
188179
let mask_density = mask.density();
189-
let mut stats_mask = mask.bitand(&stats_mask);
180+
let mut stats_mask = if pruned_zones == 0 {
181+
// The stats mask would be all-true, so intersecting it is a no-op.
182+
mask
183+
} else if pruned_zones == covered_zones {
184+
// The stats mask would be all-false.
185+
Mask::new_false(mask_len)
186+
} else {
187+
let mut builder = BitBufferMut::with_capacity(mask_len);
188+
for zone_idx in zone_start..zone_end {
189+
// Figure out the range in the mask that corresponds to the zone
190+
let zone_idx_u64 = zone_idx as u64;
191+
let start = usize::try_from(
192+
first_row_offset(zone_idx_u64, zone_len, layout_row_count)
193+
.saturating_sub(row_start),
194+
)?;
195+
let end = usize::try_from(
196+
first_row_offset(zone_idx_u64 + 1, zone_len, layout_row_count)
197+
.saturating_sub(row_start)
198+
.min(split_row_count),
199+
)?;
200+
builder.append_n(!pruning_mask.value(zone_idx), end - start);
201+
}
202+
203+
let stats_mask = Mask::from(builder.freeze());
204+
assert_eq!(stats_mask.len(), mask_len, "Mask length mismatch");
205+
206+
// Intersect the masks.
207+
mask.bitand(&stats_mask)
208+
};
190209

191210
// Forward to data child for further pruning.
192211
if !stats_mask.all_false() {
@@ -231,6 +250,7 @@ impl LayoutReader for ZonedReader {
231250
#[cfg(test)]
232251
mod test {
233252
use std::num::NonZeroUsize;
253+
use std::ops::Range;
234254
use std::sync::Arc;
235255

236256
use rstest::fixture;
@@ -375,6 +395,36 @@ mod test {
375395
})
376396
}
377397

398+
/// The zoned reader takes a uniform fast path when every zone covering the requested row
399+
/// range agrees, so exercise all-pruned, all-kept and mixed ranges.
400+
#[rstest]
401+
#[case::only_pruned_zones(0..6, vec![false; 6])]
402+
#[case::only_kept_zones(6..9, vec![true; 3])]
403+
#[case::single_pruned_zone(3..6, vec![false; 3])]
404+
#[case::partial_zones_mixed(1..8, vec![false, false, false, false, false, true, true])]
405+
#[case::empty_range(4..4, vec![])]
406+
fn test_stats_pruning_mask_zone_ranges(
407+
#[from(stats_layout)] (segments, layout): (Arc<dyn SegmentSource>, LayoutRef),
408+
#[case] row_range: Range<u64>,
409+
#[case] expected: Vec<bool>,
410+
) -> VortexResult<()> {
411+
block_on(|handle| async {
412+
let session = session_with_handle(handle);
413+
let reader = layout.new_reader("".into(), segments, &session, &Default::default())?;
414+
415+
// Values are 1..=9 in zones of 3, so `> 7` prunes zones 0 and 1 and keeps zone 2.
416+
let expr = gt(root(), lit(7)).bind(reader.dtype())?;
417+
let len = usize::try_from(row_range.end - row_range.start)?;
418+
419+
let result = reader
420+
.pruning_evaluation(&row_range, &expr, Mask::new_true(len))?
421+
.await?;
422+
423+
assert_eq!(result, Mask::from_iter(expected));
424+
Ok(())
425+
})
426+
}
427+
378428
#[test]
379429
fn test_default_zoned_null_count_pruning_mask() {
380430
let ctx = ArrayContext::empty();

0 commit comments

Comments
 (0)