compute: scope per-batch decode arenas to the activation#37134
Open
frankmcsherry wants to merge 1 commit into
Open
compute: scope per-batch decode arenas to the activation#37134frankmcsherry wants to merge 1 commit into
frankmcsherry wants to merge 1 commit into
Conversation
…ivation
The `ArrangementFlatMap`, `ArrangementFlatMapOk`, and `FormArrangementKey`
operators created their decode `RowArena` at operator-build time and captured
it, so it lived for the whole dataflow. With the region-retaining `RowArena`
(its `clear` keeps the largest region for reuse), a build-captured, per-row
cleared arena would pin its high-water region for the operator's lifetime — a
small but permanent per-operator tax.
Move the arena (and its `DatumVec`) into the per-activation closure instead, so
it is dropped at the end of each scheduling invocation while still being reused
across the records processed within that activation. For `FormArrangementKey`
this is a one-line move; for the flat_map operators the per-row decode (and the
arena it decodes into) moves from a build-time wrapper closure into
`flat_map_core_{fallible,ok}`'s activation closure, which now takes the
already-`max_demand` and the decoded-form `logic` directly. `do_work`'s logic
bound drops a vestigial `'static` so it can accept the activation-scoped
(borrowing) decode closure.
No behavior change; the decode and emitted results are identical.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
frankmcsherry
added a commit
that referenced
this pull request
Jun 18, 2026
## What `render_decode_chunk` (the oneshot/COPY decode operator) created one `RowArena` in the operator body and reused it across every chunk and every row **without ever clearing it**. The arena owns whatever is pushed into it for its lifetime, so the MFP-evaluation temporaries for the *entire source* accumulated until the COPY finished — an unbounded arena proportional to the data volume. This scopes the arena to each decoded chunk and clears it per row, so it holds at most one row's worth of temporaries and is released when the chunk is processed. ## Why Found while auditing `RowArena` lifetimes (companion to the arena work in #37114 / #37134). This was the only arena in the audit that was both operator-lived **and** never cleared — i.e. a genuine unbounded accumulation rather than a bounded high-water. It brings the operator in line with the "arena per batch, cleared per row" idiom the other operators use. ## Tests No behavior change (the MFP output is identical; only the arena's lifetime changes). Covered by the existing oneshot-source / COPY FROM tests. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What
Moves the
RowArena(and accompanyingDatumVec) used for per-row decode in theArrangementFlatMap,ArrangementFlatMapOk, andFormArrangementKeyoperators from operator-build scope into the per-activation closure, so it is dropped at the end of each scheduling invocation while still being reused across the records processed within that activation.Why
These three operators created their decode arena once at build time and captured it for the operator's lifetime, clearing it per record. That's the only place in the compute/storage layer (verified by an exhaustive audit of every
RowArena.clear()site) where an arena is both clear-reused and dataflow-lived.It matters because of the companion change that makes
RowArena::clear()retain its largest region for reuse (#37114): under that semantics, a build-captured + per-row-cleared arena pins its high-water region for the operator's whole life — a small (one record's worth) but permanent per-operator memory tax. Scoping the arena to the activation eliminates the tax (the region is freed when the activation returns) while keeping the within-activation reuse that retention buys.The discipline this establishes — fresh arena per scheduling invocation, cleared per record — is the one already used by
LinearJoinKeyPreparation; this brings the remaining clear-reuse arenas in line with it.How
FormArrangementKey: a one-line move ofRowArena::new()from the build closure into themove |_frontiers|closure.flat_map/flat_map_ok: the per-row decode wrapper (which owned the arena +DatumVec) is removed;flat_map_core_{fallible,ok}now takemax_demandand the decoded-formlogicdirectly and build the decode closure inside their per-activation closure.do_work'slogicbound drops a vestigial+ 'static(never required bywalk_cursor) so it can accept the activation-scoped, borrowing decode closure.No public API change; no behavior change — the decode and emitted results are identical. This is intended to land before #37114 so the region-retention change can never introduce a dataflow-lived arena tax.
Tests
Covered by existing compute unit tests and the SLT/testdrive suites that exercise indexed MFPs, table functions, and arrangement formation. No behavior change, so no new tests.