Skip to content

Use Exact Hash-Table Allocation Accounting in GroupValuesPrimitive #25734

Description

@kosiew

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

  • An empty primitive group-values instance reports the map allocation using
    the exact hash-table layout rather than an entry-size approximation.
  • After inserting enough distinct values to allocate the map, the map term
    equals HashTable::allocation_size() and includes control-byte overhead.
  • After EmitTo::First or EmitTo::All, any retained map capacity remains
    included until the map allocation is actually released.
  • Existing grouping and emit results remain unchanged.
  • The implementation does not double count the map allocation through any
    other field or accounting helper.

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions