comphead opened a new issue, #23393: URL: https://github.com/apache/datafusion/issues/23393
### Is your feature request related to a problem or challenge? A follow up on https://github.com/apache/datafusion/pull/23357 ### Summary Several `Accumulator::size()`, `GroupsAccumulator::size()`, and related implementations report far less memory than they actually hold, which lets the `MemoryPool` limits be silently bypassed. An audit of ~157 `fn size()` sites plus a measurement test against a tracking allocator surface both worst-case defects and a systemic pattern. ### Evidence Measured with a tracking global allocator around each populated struct, comparing `struct.size()` to real heap-live bytes: | struct | rows / inputs | `size()` reports | actual heap | undercount | |---|---|---:|---:|---:| | `SlidingDistinctCountAccumulator<Int64>` | 1 024 distinct | **64 B** | 165 960 B | **−100.0%** | | `SlidingDistinctCountAccumulator<Int64>` | 16 384 distinct | **64 B** | 2 654 280 B | **−100.0%** | | `SlidingDistinctCountAccumulator<Utf8>` | 8 192 distinct | **64 B** | 1 425 480 B | **−100.0%** | | `ArrayAggAccumulator<Utf8>` | 8 192 unique strings | 131 208 B | 164 288 B | −20.1% | | `ArrowBytesSet<i32>` | 32 768 unique utf8 | 2 419 608 B | 2 687 136 B | −10.0% | | `ArrowBytesViewSet` | 32 768 unique utf8-view | 2 592 992 B | 2 949 352 B | −12.1% | `ScalarValue::size()` and `TrivialFirst/LastValueAccumulator::size()` were byte-exact — the pattern is fixable, not intrinsic. End-to-end, on `SELECT k, count(v), sum(v), avg(v) FROM t GROUP BY k` over 200 k rows the aggregate operators reserve **55 MB** against a real live peak of **~7 MB** (720% ratio) — accounted allocations *also* don't get released as batches emit downstream. ### Root causes found in the audit (25 verified findings, high-severity subset) - `SlidingDistinctCountAccumulator` / `SlidingDistinctSumAccumulator` — `size()` returns only `size_of_val(self)`, ignoring the `HashMap` bucket capacity and ScalarValue key heap. `functions-aggregate/src/count.rs:557`, `sum.rs:619`. - `CountGroupsAccumulator` — uses `size_of::<usize>()` for a `Vec<i64>`. Wrong element type. `count.rs:771`. - `SymmetricHashJoinStream` — omits `batch_transformer`, which can hold multi-MB `RecordBatch`. `symmetric_hash_join.rs:1650`. - `OneSideHashJoiner` — double-counts fields already in `size_of_val(self)`; `HashSet<usize>` counted at 8 B/slot instead of 16 B. `symmetric_hash_join.rs:1170`. - `ArrowBytesViewMap::size()` — sums `buffer.len()` instead of `buffer.capacity()`; misses the `completed: Vec<Buffer>` container. `binary_view_map.rs:476`. - `GroupValuesPrimitive::size` / `GroupValuesRows::size` — missing `size_of::<Self>()`. - `GroupValuesColumn::size` — ignores `group_index_lists`, `emit_group_index_list_buffer`, and the 5 Vecs in `vectorized_operation_buffers` (can be 100s of KB). - `GroupOrdering::size` — **over**counts by adding both `size_of::<Self>()` and the inner variant's `size()` (which already includes it). `aggregates/order/mod.rs:137`. - `FirstLastGroupsAccumulator`, `ArrayAggGroupsAccumulator`, `VarianceGroupsAccumulator`, `AvgGroupsAccumulator`, `HllGroupsAccumulator`, `PercentileContGroupsAccumulator`, `MinMaxBytesState`, `ArrayMap`, `TopK` (missing `common_sort_prefix_converter`), `TrivialNthValueAccumulator`, `StringAggAccumulator`, and `ExprIntervalGraph` — all confirmed under/over-count issues. Full list with concrete failure scenarios and recommended fixes available on request. ### Describe the solution you'd like _No response_ ### Describe alternatives you've considered _No response_ ### Additional context _No response_ -- 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]
