Related PR
#25188
Problem
GroupValuesPrimitive::size() estimates its hash-table allocation as
map.capacity() * size_of::<(usize, u64)>(). This counts only the entry array
and omits hashbrown's control bytes and trailing group allocation. The reported
size is therefore smaller than the retained allocation, especially for small
tables where layout overhead is a larger fraction of the total.
The implementation is in
datafusion/physical-plan/src/aggregates/group_values/single_group_by/primitive.rs.
The same file already reports the owner descriptor and the value vector, so the
remaining issue is the map term itself.
Why it matters
GroupValues::size() drives memory-pool reservations and spill decisions. An
under-reported map can allow a query to retain more memory than the pool
believes it owns, and makes primitive single-column grouping inconsistent with
ArrowBytesMap::size(), which uses HashTable::allocation_size().
This is a retained-capacity accounting correction, not a change to hashing,
group identity, insertion behavior, or map reuse.
Invariant / desired behavior
GroupValuesPrimitive::size() reports the complete retained allocation of its
HashTable<(usize, u64)> exactly once:
- the map term uses
HashTable::allocation_size(), including control bytes and
trailing layout;
- the value vector and
size_of::<Self>() remain charged once;
- clearing or emitting values does not stop retained map capacity from being
charged until the allocation is released;
- the result does not depend on logical map length when capacity is retained.
Proposed direction
Replace the capacity() * size_of::<(usize, u64)>() term in
GroupValuesPrimitive::size() with self.map.allocation_size(), following the
existing ArrowBytesMap::size() implementation in
datafusion/physical-expr-common/src/binary_map.rs.
Use the existing hashbrown allocation API directly. Do not change the
insert_accounted path or introduce allocator-live-byte measurements; this
issue is about making the public memory estimate reflect the allocation layout.
Scope
In
datafusion/physical-plan/src/aggregates/group_values/single_group_by/primitive.rs.
- The primitive map contribution to
GroupValuesPrimitive::size().
- Focused tests for empty, grown, and retained-after-emit map capacity.
Out
- Hashing, collision handling, group-id assignment, and emit semantics.
GroupValuesRows or GroupValuesColumn changes; they have separate issue
boundaries so each implementation can be reviewed independently.
- Changes to
HashTableAllocExt, hashbrown, or the memory-pool contract.
- Global allocator-live-byte assertions.
Acceptance criteria
Tests / verification
- Add or extend focused
primitive.rs tests that compare the reported size
delta against the independently observed map.allocation_size() delta;
avoid asserting a platform-specific absolute byte count.
- Exercise a post-emit/reuse state so a lower logical length cannot hide
retained capacity.
- Run
cargo test -p datafusion-physical-plan.
- Before merge, run
cargo fmt --all,
cargo clippy --all-targets --all-features -- -D warnings, and
./dev/rust_lint.sh.
Related PR
#25188
Problem
GroupValuesPrimitive::size()estimates its hash-table allocation asmap.capacity() * size_of::<(usize, u64)>(). This counts only the entry arrayand omits hashbrown's control bytes and trailing group allocation. The reported
size is therefore smaller than the retained allocation, especially for small
tables where layout overhead is a larger fraction of the total.
The implementation is in
datafusion/physical-plan/src/aggregates/group_values/single_group_by/primitive.rs.The same file already reports the owner descriptor and the value vector, so the
remaining issue is the map term itself.
Why it matters
GroupValues::size()drives memory-pool reservations and spill decisions. Anunder-reported map can allow a query to retain more memory than the pool
believes it owns, and makes primitive single-column grouping inconsistent with
ArrowBytesMap::size(), which usesHashTable::allocation_size().This is a retained-capacity accounting correction, not a change to hashing,
group identity, insertion behavior, or map reuse.
Invariant / desired behavior
GroupValuesPrimitive::size()reports the complete retained allocation of itsHashTable<(usize, u64)>exactly once:HashTable::allocation_size(), including control bytes andtrailing layout;
size_of::<Self>()remain charged once;charged until the allocation is released;
Proposed direction
Replace the
capacity() * size_of::<(usize, u64)>()term inGroupValuesPrimitive::size()withself.map.allocation_size(), following theexisting
ArrowBytesMap::size()implementation indatafusion/physical-expr-common/src/binary_map.rs.Use the existing hashbrown allocation API directly. Do not change the
insert_accountedpath or introduce allocator-live-byte measurements; thisissue is about making the public memory estimate reflect the allocation layout.
Scope
In
datafusion/physical-plan/src/aggregates/group_values/single_group_by/primitive.rs.GroupValuesPrimitive::size().Out
GroupValuesRowsorGroupValuesColumnchanges; they have separate issueboundaries so each implementation can be reviewed independently.
HashTableAllocExt, hashbrown, or the memory-pool contract.Acceptance criteria
the exact hash-table layout rather than an entry-size approximation.
equals
HashTable::allocation_size()and includes control-byte overhead.EmitTo::FirstorEmitTo::All, any retained map capacity remainsincluded until the map allocation is actually released.
other field or accounting helper.
Tests / verification
primitive.rstests that compare the reported sizedelta against the independently observed
map.allocation_size()delta;avoid asserting a platform-specific absolute byte count.
retained capacity.
cargo test -p datafusion-physical-plan.cargo fmt --all,cargo clippy --all-targets --all-features -- -D warnings, and./dev/rust_lint.sh.