perf(scan): memoize FlatReader decoded array (filter+projection decode once)#66
Merged
Conversation
…e once) A column referenced by BOTH the filter (WHERE) and the projection (SELECT) — the common filtered-scan-returning-the-filter-column case — went through array_future() twice, decoding the chunk twice. Memoize the shared decode in a OnceLock<SharedArrayFuture> (mirroring DictReader::values_array) so it decodes once. The segment is still re-requested on every call so the segment reader keeps prioritization visibility; only the decode is cached. Bounded by the reader's lifetime (readers are held via Weak in the opener), so it does not reintroduce the unbounded retention that prompted removing the prior cache.
There was a problem hiding this comment.
Pull request overview
This PR targets a hot-path performance issue in FlatReader where the same column can be decoded twice when it is referenced by both the filter (WHERE) and the projection (SELECT). It introduces memoization so the decoded array future can be shared across those call sites.
Changes:
- Add a
OnceLock<SharedArrayFuture>toFlatReaderto memoize the decoded array. - Update
array_future()to useOnceLock::get_or_init(...)so filter and projection can share the same decode.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
75
| // We create the segment_fut here to ensure we give the segment reader visibility into how | ||
| // to prioritize this segment, even if the `array` future has already been initialized (it | ||
| // is, on the second of a filter+projection pair over the same column). This is gross... see | ||
| // the function's TODO for a maybe better solution? | ||
| let segment_fut = self.segment_source.request(self.layout.segment_id()); |
Comment on lines
+46
to
+49
| /// so it happens once. The segment is still re-requested on every | ||
| /// `array_future()` call so the segment reader keeps prioritization | ||
| /// visibility (see there). Bounded by the reader's lifetime — readers are | ||
| /// held via `Weak` in the opener, mirroring `DictReader`'s `values_array`. |
…decode once) array_future_memoizes_the_decode: the decode cache (OnceLock) starts empty, is populated after the first array_future() (the WHERE-filter touch), and the second touch (the SELECT projection of the same column) reuses it — so a column feeding both filter and projection decodes once. Mirrors the existing flat-reader test harness (TestSegments + FlatLayoutStrategy).
phillipleblanc
approved these changes
Jun 13, 2026
sgrebnov
approved these changes
Jun 13, 2026
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.
Why
A column referenced by BOTH the WHERE filter and the SELECT projection — the common filtered-scan-returning-the-filter-column case — goes through
FlatReader::array_future()twice, decoding the chunk twice.What
Memoize the shared decode in a
OnceLock<SharedArrayFuture>(mirroringDictReader::values_array), so the column decodes once. The segment is still re-requested on every call so the segment reader keeps prioritization visibility — only the decode is cached. Bounded by the reader's lifetime (readers held viaWeakin the opener), so it does not reintroduce the unbounded retention that prompted removing the prior flat-reader cache.Companion to the intra-file split (#62) and the per-ExecutionCtx kernel cache already on spiceai-53.
cargo check/clippy -p vortex-layoutclean; flat-reader tests pass; e2e validation in spiceai's 3-node CH-benCH.