kosiew opened a new pull request, #25188: URL: https://github.com/apache/datafusion/pull/25188
## Which issue does this PR close? * Part of #23393 ## Rationale for this change `GroupValues::size()` can underreport memory retained by group-value state. In particular, the owner descriptors are not consistently included, and the column-based implementation does not account for several retained allocations used for collision handling, emit scratch space, and vectorized operations. This can make aggregate memory accounting lower than the memory actually retained by `GroupValues`, including after reusable buffers have been cleared or groups have been emitted. This PR updates the accounting so each `GroupValues` implementation reports its owner descriptor and owned retained allocations exactly once, using allocation capacity where appropriate. ## What changes are included in this PR? This PR: * Includes the owner descriptor in `GroupValuesPrimitive`, `GroupValuesRows`, and `GroupValuesColumn` memory accounting. * Includes concrete boxed `GroupColumn` owner descriptors in their `size()` implementations. * Updates row-backed accounting to avoid double-counting inline `Rows` and `RowConverter` descriptors. * Accounts for the `GroupValuesColumn` allocation backing its group-column collection. * Accounts for retained group-index lists, including their nested allocations. * Accounts for the emit group-index scratch buffer. * Adds accounting for vectorized-operation buffers, including retained capacity after they are cleared. * Continues to account for reusable buffers by capacity rather than logical length. * Adjusts two aggregate spill-test memory limits to accommodate the corrected memory accounting. ## Are these changes tested? Yes. This PR adds focused regression tests covering: * Empty and grown `GroupValuesPrimitive` memory accounting. * Empty and grown `GroupValuesRows` memory accounting. * Retained row and hash buffers across emit and subsequent reuse. * Boxed primitive and row-backed `GroupColumn` owner descriptors. * Collision group-index lists, emit scratch storage, and vectorized-operation buffers in `GroupValuesColumn`. * Retained vectorized and emit scratch capacity after buffers are cleared. The tests compute expected sizes from the relevant owner descriptors and retained allocations rather than relying on global allocator measurements. ## Are there any user-facing changes? There are no API changes. Aggregate memory accounting is more complete, so workloads using these `GroupValues` implementations may report or reserve more memory than before. This can affect when memory limits cause aggregation to spill, reflecting memory that was already retained but previously omitted from `GroupValues::size()`. ## LLM-generated code disclosure This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
