Skip to content

Commit ad83fcb

Browse files
committed
Cache immutable whole-subtree support checks for sparse V2 scans
Sparse identity scans must validate complete list element and dictionary value subtrees, even when the selected root rows touch few chunks. Repeating these checks on each scan made the 64 KiB list layout disproportionately expensive. Cache only whole-subtree answers within ScanFile preparation; selection-dependent answers remain uncached. Retain keyed layout Arcs to prevent address reuse, and bypass the cache when the root layout changes. The prior 32 MiB estimated list-work admission cap and independently scheduled projection pieces are unchanged. No segment data is retained by this metadata cache. Errors are not cached; unsupported answers are. On warm local disk, 100 selected rows, three seeds, two alternating rounds of 64 queries (discard eight), full Arrow decode/drop: - 64 KiB clustered cached, 30 cores: 0.2053 -> 0.1525 ms (-25.7%). - 64 KiB clustered cached, one core: 0.2716 -> 0.2220 ms (-18.3%). - 64 KiB uniform cached, 30 cores: 0.8233 -> 0.7558 ms (-8.2%). - No aggregate regression above 3% across 48 cases versus previous V2. - Twelve longer reruns used four rounds of 128 queries, discard sixteen; the largest confirmed slowdown was 2.6% (nested structs, one core). This does not resolve all V1 gaps: 13/48 initial cases remain above +5%, including reopened clustered lists and parallel uniform feature vectors. Reopening receives little benefit because the preparation cache is fresh. Experiments with smaller clustered split targets and a direct contiguous list-element source did not improve end-to-end timing and were discarded. Validation: 207 targeted scan::v2 unit tests; 60 real-file Arrow comparisons against V1 full-scan-plus-take across empty, singleton, clustered, sparse, and all-row selections at one and 30 cores; pinned rustfmt; diff --check. No workspace-wide checks were run. Local experiment reports and rejected patches are under the list-layout-experiment directory in the scan bench workspace; the CI branch runs the comparable matrix on a dedicated runner. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent 024005e commit ad83fcb

2 files changed

Lines changed: 113 additions & 11 deletions

File tree

‎vortex-layout/src/scan/v2/mod.rs‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ use futures::TryStreamExt;
3232
use futures::stream;
3333
use futures::stream::BoxStream;
3434
use once_cell::sync::OnceCell;
35+
use parking_lot::Mutex;
3536
use vortex_array::ArrayRef;
3637
use vortex_error::VortexResult;
3738
use vortex_error::vortex_err;
@@ -43,6 +44,7 @@ use vortex_scan::planning::selection::SelectedRows;
4344
use vortex_scan::planning::selection::SelectionPlanner;
4445
use vortex_scan::selection::Selection;
4546
use vortex_session::VortexSession;
47+
use vortex_utils::aliases::hash_map::HashMap;
4648
use vortex_utils::parallelism::get_available_parallelism;
4749

4850
use crate::LayoutRef;
@@ -103,11 +105,15 @@ struct FilePreparation {
103105
segment_bytes: u64,
104106
projection_supported: OnceCell<bool>,
105107
identity_supported: OnceCell<bool>,
108+
whole_identity_support: WholeSupport,
106109
root: OnceCell<PlanRef>,
107110
ranges: OnceCell<SegmentRanges>,
108111
values: OnceCell<ValueCache>,
109112
}
110113

114+
// Retain each keyed layout so its address cannot be reused for a different support answer.
115+
type WholeSupport = Mutex<HashMap<usize, (LayoutRef, bool)>>;
116+
111117
impl ScanFile {
112118
/// Creates a file description without retaining preparation between scans.
113119
pub fn new(
@@ -123,9 +129,9 @@ impl ScanFile {
123129
}
124130
}
125131

126-
/// Shares immutable layout plans, segment locations, and whole dictionary values between
127-
/// this file's scans. Only identity projections retain dictionary values; selections and
128-
/// expression results remain owned by each scan.
132+
/// Shares immutable layout support checks and plans, segment locations, and whole dictionary
133+
/// values between this file's scans. Only identity projections retain dictionary values;
134+
/// selections and expression results remain owned by each scan.
129135
pub fn with_caching(mut self) -> Self {
130136
self.preparation = Some(Arc::new(FilePreparation {
131137
layout: Arc::clone(&self.layout),
@@ -138,6 +144,7 @@ impl ScanFile {
138144
.sum(),
139145
projection_supported: OnceCell::new(),
140146
identity_supported: OnceCell::new(),
147+
whole_identity_support: Mutex::default(),
141148
root: OnceCell::new(),
142149
ranges: OnceCell::new(),
143150
values: OnceCell::new(),
@@ -182,7 +189,13 @@ impl ScanFile {
182189
let rows = rows.cloned().unwrap_or(0..self.layout.row_count());
183190
if let Some(indices) = sparse_indices(selection, &rows) {
184191
// A selection-specific answer cannot be cached as support for the whole file.
185-
return supports_selected_layout(&self.layout, &indices, 0);
192+
return supports_selected_layout(
193+
&self.layout,
194+
&indices,
195+
0,
196+
self.layout_preparation()
197+
.map(|cache| &cache.whole_identity_support),
198+
);
186199
}
187200
}
188201
self.supports(identity)
@@ -327,6 +340,7 @@ fn supports_selected_layout(
327340
layout: &LayoutRef,
328341
indices: &[u64],
329342
offset: u64,
343+
whole: Option<&WholeSupport>,
330344
) -> VortexResult<bool> {
331345
if layout.is::<Flat>() {
332346
return Ok(true);
@@ -346,7 +360,12 @@ fn supports_selected_layout(
346360
let child = layout
347361
.slot(chunk)?
348362
.ok_or_else(|| vortex_err!("Missing selected chunk"))?;
349-
if !supports_selected_layout(&child, &indices[first..last], offset + offsets[chunk])? {
363+
if !supports_selected_layout(
364+
&child,
365+
&indices[first..last],
366+
offset + offsets[chunk],
367+
whole,
368+
)? {
350369
return Ok(false);
351370
}
352371
first = last;
@@ -355,7 +374,7 @@ fn supports_selected_layout(
355374
}
356375
if layout.is::<Zoned>() || layout.is::<LegacyStats>() {
357376
return match layout.slot(0)? {
358-
Some(data) => supports_selected_layout(&data, indices, offset),
377+
Some(data) => supports_selected_layout(&data, indices, offset, whole),
359378
None => Ok(false),
360379
};
361380
}
@@ -366,16 +385,15 @@ fn supports_selected_layout(
366385
let codes = layout
367386
.slot(1)?
368387
.ok_or_else(|| vortex_err!("Missing dictionary codes"))?;
369-
return Ok(
370-
supports_layout(&values, true)? && supports_selected_layout(&codes, indices, offset)?
371-
);
388+
return Ok(supports_whole_identity(&values, whole)?
389+
&& supports_selected_layout(&codes, indices, offset, whole)?);
372390
}
373391
if layout.is::<List>() {
374-
return supports_layout(layout, true);
392+
return supports_whole_identity(layout, whole);
375393
}
376394
if layout.is::<Struct>() {
377395
for child in layout.children()? {
378-
if !supports_selected_layout(&child, indices, offset)? {
396+
if !supports_selected_layout(&child, indices, offset, whole)? {
379397
return Ok(false);
380398
}
381399
}
@@ -384,6 +402,20 @@ fn supports_selected_layout(
384402
Ok(false)
385403
}
386404

405+
/// Unlike a selected-chunk check, this answer is independent of the selected rows.
406+
fn supports_whole_identity(layout: &LayoutRef, cache: Option<&WholeSupport>) -> VortexResult<bool> {
407+
let Some(cache) = cache else {
408+
return supports_layout(layout, true);
409+
};
410+
let key = Arc::as_ptr(layout).cast::<()>() as usize;
411+
if let Some((_, supported)) = cache.lock().get(&key) {
412+
return Ok(*supported);
413+
}
414+
let supported = supports_layout(layout, true)?;
415+
cache.lock().insert(key, (Arc::clone(layout), supported));
416+
Ok(supported)
417+
}
418+
387419
/// A scan planned over its file, ready to run its splits.
388420
pub(crate) struct PreparedScan<A: 'static + Send> {
389421
session: VortexSession,

‎vortex-layout/src/scan/v2/tests.rs‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ use crate::layouts::chunked::writer::ChunkedLayoutStrategy;
5959
use crate::layouts::dict::DictLayout;
6060
use crate::layouts::flat::writer::FlatLayoutStrategy;
6161
use crate::layouts::foreign::new_foreign_layout;
62+
use crate::layouts::list::ListLayout;
6263
use crate::layouts::list::writer::ListLayoutStrategy;
6364
use crate::layouts::row_idx::row_idx;
6465
use crate::layouts::struct_::StructLayout;
@@ -488,6 +489,75 @@ impl LayoutChildren for CountedLayoutChildren {
488489
}
489490
}
490491

492+
#[rstest]
493+
#[tokio::test]
494+
async fn caches_whole_subtree_support_without_reusing_replaced_layouts(
495+
#[values(false, true)] caching: bool,
496+
#[values(false, true)] unsupported: bool,
497+
#[values(false, true)] list: bool,
498+
) -> VortexResult<()> {
499+
let session = new_session().with_tokio();
500+
let (segments, layout) = write(&session, table().as_ref()).await?;
501+
let ints = layout.slot(1)?.ok_or_else(|| vortex_err!("missing ints"))?;
502+
let mut children = ints.children()?;
503+
if unsupported {
504+
children[1] = new_foreign_layout(
505+
LayoutId::new("test.unsupported"),
506+
ints.dtype().clone(),
507+
CHUNK_ROWS as u64,
508+
Vec::new(),
509+
Vec::new(),
510+
Vec::new(),
511+
);
512+
}
513+
let values = ints.as_::<Chunked>().with_children(children);
514+
let visited = Arc::new(AtomicUsize::new(0));
515+
let values = ChunkedLayout::new(
516+
values.row_count(),
517+
values.dtype().clone(),
518+
Arc::new(CountedLayoutChildren {
519+
inner: Arc::clone(values.children()),
520+
materialized: Arc::clone(&visited),
521+
encoding_hints: false,
522+
}),
523+
)
524+
.into_layout();
525+
let make_layout = |values: LayoutRef, ints: LayoutRef| {
526+
if list {
527+
ListLayout::new(
528+
DType::List(Arc::new(values.dtype().clone()), false.into()),
529+
values,
530+
ints,
531+
None,
532+
)
533+
.into_layout()
534+
} else {
535+
DictLayout::new(values, ints).into_layout()
536+
}
537+
};
538+
let layout = make_layout(Arc::clone(&values), Arc::clone(&ints));
539+
let mut file = scan_file(&segments, &layout)?;
540+
if caching {
541+
file = file.with_caching();
542+
}
543+
visited.store(0, Ordering::Relaxed);
544+
let selection = Selection::IncludeRoaring([0, 3000].into_iter().collect());
545+
assert_eq!(file.supports_scan(true, &selection, None)?, !unsupported);
546+
let first = visited.load(Ordering::Relaxed);
547+
assert!(first > 0);
548+
assert_eq!(file.supports_scan(true, &selection, None)?, !unsupported);
549+
assert_eq!(
550+
visited.load(Ordering::Relaxed),
551+
first * if caching { 1 } else { 2 }
552+
);
553+
554+
let before = visited.load(Ordering::Relaxed);
555+
file.layout = make_layout(values, ints);
556+
assert_eq!(file.supports_scan(true, &selection, None)?, !unsupported);
557+
assert_eq!(visited.load(Ordering::Relaxed), before + first);
558+
Ok(())
559+
}
560+
491561
#[rstest]
492562
#[tokio::test]
493563
async fn support_checks_flat_tags_without_materializing_chunks(

0 commit comments

Comments
 (0)