Skip to content

Commit 16ad60f

Browse files
compute: hoist the flat_map decode arena out of the per-row closure (#37113)
## What `ArrangementFlavor::flat_map` and `flat_map_ok` constructed a fresh `RowArena::new()` *inside* the per-row `logic` closure, so an arena was allocated and dropped on every row processed. This hoists the arena to the enclosing scope and `clear()`s it once per row instead, reusing its allocation spine across invocations. ## Why Addresses post-merge review feedback from @antiguru on #37110 (the `ExtendDatums` refactor): the arena is the caller-provided decode target a compressed row representation decodes into. With per-column arrangement codecs (follow-up #37111) each row actually populates the arena, so reallocating it per row would churn; clearing-and-reusing keeps the spine. Note that `RowArena::clear()` today drops the inner per-value `Vec<u8>` buffers (it clears the outer `Vec<Vec<u8>>`), so this reuses the outer spine but not the individual buffers — recycling those is a separate, larger `RowArena` change. ## Tests No behavior change; existing `compute` tests cover these paths. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ff4065d commit 16ad60f

1 file changed

Lines changed: 10 additions & 4 deletions

File tree

src/compute/src/render/context.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,14 +318,17 @@ impl<'scope, T: RenderTimestamp> ArrangementFlavor<'scope, T> {
318318
+ 'static,
319319
{
320320
let mut datums = DatumVec::new();
321+
// Reused across invocations: cleared per row rather than reallocated, so the arena's
322+
// allocation spine persists. Declared before the borrow so it outlives any datums that
323+
// decode into it.
324+
let mut temp_storage = RowArena::new();
321325
let logic = move |k: DatumSeq,
322326
v: DatumSeq,
323327
t,
324328
d,
325329
ok_session: &mut Session<T, DCB>,
326330
err_session: &mut Session<T, ECB<T>>| {
327-
// Declared before the borrow so it outlives any datums that decode into it.
328-
let temp_storage = RowArena::new();
331+
temp_storage.clear();
329332
let mut datums_borrow = datums.borrow();
330333
k.extend_datums(&temp_storage, &mut datums_borrow, Some(max_demand));
331334
let max_demand = max_demand.saturating_sub(datums_borrow.len());
@@ -379,9 +382,12 @@ impl<'scope, T: RenderTimestamp> ArrangementFlavor<'scope, T> {
379382
+ 'static,
380383
{
381384
let mut datums = DatumVec::new();
385+
// Reused across invocations: cleared per row rather than reallocated, so the arena's
386+
// allocation spine persists. Declared before the borrow so it outlives any datums that
387+
// decode into it.
388+
let mut temp_storage = RowArena::new();
382389
let logic = move |k: DatumSeq, v: DatumSeq, t, d, ok_session: &mut Session<T, DCB>| {
383-
// Declared before the borrow so it outlives any datums that decode into it.
384-
let temp_storage = RowArena::new();
390+
temp_storage.clear();
385391
let mut datums_borrow = datums.borrow();
386392
k.extend_datums(&temp_storage, &mut datums_borrow, Some(max_demand));
387393
let max_demand = max_demand.saturating_sub(datums_borrow.len());

0 commit comments

Comments
 (0)