You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "comphead (via GitHub)" <gi...@apache.org> on 2026/07/08 20:24:07 UTC
[I] feat: analyze memory usage for `size` functions [datafusion]
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: github-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] feat: analyze memory usage for `size` functions [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #23393:
URL: https://github.com/apache/datafusion/issues/23393#issuecomment-4919890858
would be also useful to prevent regressions when the underlying object changed and `size` doesn't reflect new dependencies
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] feat: analyze memory usage for `size` functions [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #23393:
URL: https://github.com/apache/datafusion/issues/23393#issuecomment-4919885866
Adding a small mimalloc comparison framework and modify the `SlidingAccumulator` size to make sure numbers are approximately close to each other
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] feat: analyze memory usage for `size` functions [datafusion]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on issue #23393:
URL: https://github.com/apache/datafusion/issues/23393#issuecomment-4959456025
Fragmentation is inevitable. Allocations are pretty much always going to be page-sized so it's hard to take too much away from measuring small allocations against heap-reported memory since it gets amortized over time by a good allocator. Stuff like this:
> 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.
That's valuable to surface since we should at least be counting non-ephemeral data structures.
More broadly, I think the biggest challenge with accurate memory accounting is when batches share underlying buffers, particularly the first stage out of a parquet read. For example in this PR, really anywhere we make decisions about data being accumulated and possibly spilling, I had to introduce logic to dedupe underlying buffers https://github.com/apache/datafusion-comet/pull/4507. This commit was the subset https://github.com/apache/datafusion-comet/pull/4507/changes/f909136e04ac1d242bafaa064b4a0608c872f945 related to memory accounting.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] feat: analyze memory usage for `size` functions [datafusion]
Posted by "pmoust (via GitHub)" <gi...@apache.org>.
pmoust commented on issue #23393:
URL: https://github.com/apache/datafusion/issues/23393#issuecomment-6086599381
One more data point for this audit, from `ArrowBytesViewMap::size()` in `datafusion-physical-expr-common` 54.1.0 (`binary_view_map.rs`). We measured it with a heap profile during a string-keyed `GROUP BY` (ClickBench Q35, 20M rows).
- **Views:** `size()` (around lines 473-486) counts the views as `views.len() * 16` (around line 474). The vector grows by doubling (`views.push`, around line 451), so its capacity runs 1-2x its length. In our run the views held 89 MB, of which about 22 MB were uncounted on average.
- **Hash table:** it is counted through `map_size`. `HashTableAllocExt::insert_accounted` (`datafusion-common/src/utils/proxy.rs`, around lines 158-184) adds `capacity * size_of::<Entry>()` on each growth. hashbrown actually allocates `buckets * (size_of::<Entry>() + 1)`, with buckets at capacity × 8/7, so about 18% more before allocator size-class rounding. In our run the table held 242 MB, about 36 MB of it uncounted.
- **Timing:** the reservation is updated only after a batch has been interned (`row_hash.rs`, around lines 1067-1090). A table doubling is therefore allocated before it is charged, while the old table is still live during the rehash.
Counting `views.capacity()`, and sizing the table from `allocation_size()` rather than from growth bumps, would close most of this.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org