perf(scan): intra-file decode parallelism + per-execution kernel cache + FlatReader decode memo#60
Closed
lukekim wants to merge 3 commits into
Closed
perf(scan): intra-file decode parallelism + per-execution kernel cache + FlatReader decode memo#60lukekim wants to merge 3 commits into
lukekim wants to merge 3 commits into
Conversation
Change 1: resolve ArrayKernels once at ExecutionCtx construction (a cheap KernelSnapshot holding the two registry maps via ArcSwap::load_full) instead of cloning the session and probing the sharded DashMap on every array node in the execute_until / single-step paths. ArrayKernels is a global TypeId-keyed registry populated at session construction and never mutated mid-scan, so the cached value is invariant for one execution. Change 2: SplitBy::Layout now sub-divides any chunk-boundary span wider than IDEAL_SPLIT_SIZE into evenly sized row-range splits, so files with few large chunks decode across multiple cores. Subdivision only inserts points strictly between existing adjacent boundaries, so the half-open ranges consumers derive remain a contiguous, non-overlapping, exact partition of the same rows.
There was a problem hiding this comment.
Pull request overview
This PR targets scan/execution hot paths by increasing parallelism when splitting scans by layout chunk boundaries and by removing per-node session/kernel lookups inside the executor loop via a per-execution kernel snapshot.
Changes:
- Subdivide large chunk-boundary spans into multiple row-range splits to improve intra-file decode parallelism (
SplitBy::Layout). - Add
ArrayKernels::snapshot()and aKernelSnapshotview to avoid repeated session probes. - Cache the
KernelSnapshotinExecutionCtxand use it inexecute_parentresolution.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vortex-layout/src/scan/split_by.rs | Adds subdivide_large_spans and applies it to SplitBy::Layout to improve parallel decoding of large spans. |
| vortex-array/src/optimizer/kernels.rs | Introduces KernelSnapshot and ArrayKernels::snapshot() to cheaply capture registry state. |
| vortex-array/src/executor.rs | Caches the kernel snapshot in ExecutionCtx and uses it to avoid repeated session/kernel lookups in hot loops. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…subdivision - Correct ArrayKernels/ExecutionCtx docs flagged by review: the registry is session-scoped and mutable via public register_* methods; an ExecutionCtx sees a point-in-time snapshot taken at construction, and later registrations are picked up by the next context. - Narrow KernelSnapshot and ArrayKernels::snapshot() to pub(crate) and drop the unused reduce_parent half of the snapshot (the optimizer still probes the live registry). - Document the SplitBy::Layout sub-division behavior on the enum variant. - Use saturating_add in subdivide_large_spans against the u64::MAX edge and add a boundary regression test; add an end-to-end vortex-file test that a 250k-row single flat chunk scans correctly across sub-divided splits. Signed-off-by: Luke Kim <80174+lukekim@users.noreply.github.com>
This was referenced Jun 12, 2026
Author
|
Splitting this into two independent PRs: #61 (per-ExecutionCtx ArrayKernels snapshot) and #62 (sub-split large chunk spans, plus fixed-size RowCount and string-chunk e2e tests). Both incorporate the review feedback addressed here (point-in-time snapshot doc semantics, pub(crate) narrowing, saturating arithmetic, added tests). |
lukekim
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three scan-path performance changes (correctness-proven + unit-tested), off the a677d11 pin (an ancestor of spiceai-53), applying cleanly onto spiceai-53.
1. Intra-file decode parallelism (vortex-layout/src/scan/split_by.rs)
SplitBy::Layoutproduced one split per chunk → a well-compacted file decoded a full-column scan on a single core.subdivide_large_spansdivides any inter-boundary span over IDEAL_SPLIT_SIZE (100K rows) into evenly-spaced row-range sub-splits. Exact-coverage (only inserts interior points; half-open ranges stay contiguous, non-overlapping, every row once); property test + layout tests pass.2. Per-execution ArrayKernels cache (vortex-array/src/executor.rs, optimizer/kernels.rs)
The decode/compute loop did a session clone + sharded DashMap ArrayKernels probe per array node per iteration, for an execution-invariant value. Resolved once via ArrayKernels::snapshot() (ArcSwap::load_full) on ExecutionCtx; removes the per-node lock. Kernel-path tests pass.
3. FlatReader decode memoization (vortex-layout/src/layouts/flat/reader.rs)
A column referenced by BOTH the WHERE filter and the SELECT projection went through
array_future()twice → decoded the chunk twice. Memoize the shared decode in aOnceLock<SharedArrayFuture>(mirroringDictReader::values_array); the segment is still re-requested per call for prioritization, only the decode is cached. Bounded by reader lifetime (readers held via Weak in the opener), so it does not reintroduce the unbounded retention that prompted the prior cache removal. Flat-reader tests pass.cargo check/clippy -p vortex-array -p vortex-layout clean. Validated end-to-end in spiceai 3-node CH-benCH.