Repository navigation
Shrink residual Vec in split_vec_min_alloc to fix capacity-based memory accounting on long split_off chains #22548
Description
Activity
Another idea: add an Option<additional_capacity> to split_vec_min_alloc:
- when it's None, call shrink_to_fit
- when it's Some(extra), shrink to
length + extrainstead of shrink_to_fit
Of course we need to check each call site for subsequent push operations to make sure we're not causing unintended reallocations as a result of the shrink
- added a commit that references this issue
on Jun 5, 2026 A caller in
datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes.rspushes onto the emitted prefix immediately after the call (first_n_offsets.push(...)); shrinking the prefix forces a realloc on the next push.This should be fixed by using a dedicated helper in #23730.
a vector of one million elements is drained 1024 at a time.
This would result in
$O(N^2)$ memcopy due to taking the drain-and-shift branch in all iterations but the last, so I'd look into the real-world cases where this occurs to optimize them in some different way.Yeah, that's why I couldn't fix #22526 with the approach of emitting one batch at a time in hash aggregation instead of creating a huge initial RecordBatch that's being sliced in the ProducingOutput phase
- added a commit that references this issue
on Jul 22, 2026
Summary
Follow-up to #22416. In
split_vec_min_alloc, the residual vector keeps its original capacity across a longsplit_offchain. Under DataFusion's capacity-based memory accounting, that residual is charged for the full backing allocation until it drops, even after most of the data has been emitted.Background
split_vec_min_alloclives indatafusion_common::utilsand is used by themulti_group_bygroup-value builders (bytes.rs,primitive.rs) to carve fixed-size chunks off a growing vector.In the
split_offbranch, the emitted prefix keeps the original capacity, and the residual likewise keeps its original capacity (unchanged bysplit_off).Concrete pathological case (raised by ariel-miculas in #22416 (comment)): a vector of one million elements is drained 1024 at a time. Each emitted slice has capacity 1024 except the final one, which inherits capacity one million via
split_off+mem::replace. Symmetrically, on every iteration before the final one the residual still owns the full backing buffer, so accounting charges for it for the entire chain.Why this was not fixed in #22416
#22416 considered calling
shrink_to_fiton the emitted prefix in thesplit_offbranch and backed it out. A caller indatafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes.rspushes onto the emitted prefix immediately after the call (first_n_offsets.push(...)); shrinking the prefix forces a realloc on the next push. The testemitted_prefix_does_not_realloc_on_pushindatafusion/common/src/utils/mod.rspins that constraint.The push-after-emit constraint applies to the prefix, not to the residual. The residual is the side that lingers; the prefix is the side that gets pushed onto.
Proposed shapes
Two candidates, in order of decreasing utility-side change:
Have
split_vec_min_allocshrink the residual when its length is much smaller than its capacity (some ratio threshold). One realloc near the end of a long chain replaces carrying the full allocation indefinitely. Adds a heuristic to the shared utility.Push the shrink to the caller's finalize path. Callers driving long split chains (the
multi_group_bybuilders) callshrink_to_fit(orshrink_to) on the residual once they know no more pushes are coming. Keeps the utility allocation-policy-free; each caller picks based on its own access pattern.Whichever shape lands should come with a benchmark on the 1M to 1024 case plus a smaller fixture to keep CI cheap, so the residual shrink can be shown not to regress the common short-chain path.
Related