repr: back RowArena with a doubling region allocator#37114
Merged
frankmcsherry merged 2 commits intoJun 18, 2026
Conversation
frankmcsherry
commented
Jun 17, 2026
frankmcsherry
added a commit
to frankmcsherry/materialize
that referenced
this pull request
Jun 17, 2026
Addresses review feedback on MaterializeInc#37114: when reserve() must stage a fresh region (the active region holds live data), size it like push_bytes does — at least double the current region — so a run of small reserve() calls yields at most log-many regions instead of many small ones. Notes in clear() that region capacities are therefore monotonic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 18, 2026
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>
`RowArena` previously stored each `push_bytes` argument as its own `Vec<u8>` allocation and freed them all on `clear`. Rework it as a bump allocator over a stack of byte regions: `push_bytes` copies into the active region, allocating a new (doubled) region only when the active one lacks spare capacity, and never resizing a region that already holds data (so returned references stay valid). `clear` now retains the single largest region, emptied, so an arena reused across clears — e.g. decoding rows one at a time — becomes allocation-free in steady state. `push_bytes` now accepts any `Deref<Target = [u8]>` (e.g. `Vec<u8>`, `&[u8]`) since it copies rather than taking ownership; existing `Vec<u8>` callers are unaffected, and borrowed sources no longer need a throwaway allocation. `reserve` reserves bytes in the active region rather than slots in the outer vector, and `with_capacity` sizes the initial region. Adds tests covering reference validity across region growth, reading a unary row from a non-zero region offset, and reuse across `clear`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review feedback on MaterializeInc#37114: when reserve() must stage a fresh region (the active region holds live data), size it like push_bytes does — at least double the current region — so a run of small reserve() calls yields at most log-many regions instead of many small ones. Notes in clear() that region capacities are therefore monotonic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
19d4375 to
83f087a
Compare
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
Reworks
RowArenafrom "oneVec<u8>allocation perpush_bytes, all freed onclear" into a bump allocator over a stack of byte regions:push_bytescopies bytes into the active (last) region. When that region lacks spare capacity it allocates a new, larger region (doubling) rather than growing the current one — so a region that already holds data is never reallocated and references returned earlier stay valid. The outerVecmay reallocate as regions are added, but that only moves theVec<u8>headers, not the heap buffers they own.push_bytesnow accepts anyB: Deref<Target = [u8]>(e.g.Vec<u8>,&[u8]). ExistingVec<u8>callers are unchanged; borrowed sources no longer need a throwaway allocation just to hand bytes over.clearretains only the single largest region (emptied) and drops the rest, right-sizing the arena. An arena reused acrossclearcycles — e.g. decoding arrangement rows one at a time — becomes allocation-free in steady state.reservenow reserves bytes in the active region (was: slots in the outer vector);with_capacitysizes the initial region. Neither has load-bearing callers (thereserveusers inexprpass size hints).Why
Follows up the
ExtendDatumsarena work (#37110) and the per-row arena hoist (#37113). Once the arena is the decode target for compressed arrangement rows (per-column codecs, #37111), a fresh allocation per value per row is the dominant cost; a reused doubling region removes it. The pattern mirrors columnation's region allocator.The trade-off is the standard bump-allocator one: a single very large row leaves the arena holding a large region until the next
clear. For the per-row-cleared decode/eval paths the high-water mark is just the largest single row, so it stays bounded.Tests
Adds
mz_ore::tests inrepr::rowcovering:push_bytesremaining valid after later pushes force new regions,push_unary_rowreading a row back from a non-zero region offset (confirms decoding is position/alignment independent),clear.No behavior change for callers; no release note.