comphead opened a new pull request, #23399: URL: https://github.com/apache/datafusion/pull/23399
## Rationale for this change Related to https://github.com/apache/datafusion/issues/23393 `Accumulator::size()` (and its siblings on `GroupsAccumulator`, `GroupValues`, join hash maps) is the single input to `MemoryPool` reservations — memory limits, spill decisions, and metrics all trust it. But these bodies are hand-written arithmetic over the struct's fields and silently rot when the backing types change: a `Vec` becomes a `HashMap`, a scratch buffer is added, a `ScalarValue` variant grows, `hashbrown`'s bucket layout shifts. Nothing catches it today except reviewers eyeballing the formula. The consequence is bugs like the one this PR fixes: `SlidingDistinctCountAccumulator::size()` returned a flat 64 B regardless of contents (measured **64 B reported vs. 2.65 MB actual** at 16 k distinct values — 100 % undercount), silently defeating memory limits for `COUNT(DISTINCT …)` window aggregates. To pin these formulas down we need an **independent ground truth**. A mimalloc-wrapping `GlobalAlloc` fits: it reports the exact sum of `layout.size()` bytes handed out, is unaffected by size-class rounding or thread-cache lag (unlike jemalloc's `stats::allocated`), and is deterministic across CI runs. With it in `test-utils`, "`size()` stays within X %" becomes a testable contract for any accumulator whose memory shape may change in the future. ## What changes are included in this PR? - **Fix `SlidingDistinctCountAccumulator::size()`** — mirrors `DistinctCountAccumulator::full_size`: self + HashMap bucket capacity + per-key inner heap + `DataType` inner heap. - **New `test-utils::mimalloc_tracking::TrackingAlloc`** — reusable `GlobalAlloc` wrapping `mimalloc::MiMalloc` with an atomic live-bytes counter. - **`mimalloc` feature** on `test-utils`, `datafusion-functions-aggregate`, and `datafusion` (off by default; top-level forwards through). - **CI** — `mimalloc` added to `--features` in `.github/workflows/extended.yml` and mirrored in `AGENTS.md`. - **Cleanup** — removed orphaned `datafusion/core/tests/size_vs_jemalloc.rs`. ## Are these changes tested? Yes. `sliding_distinct_count_size_matches_allocator` populates the accumulator at 5 sizes (small → xxlarge, 64 → 1 048 576 distinct values) and asserts `|reported − actual| / actual ≤ 30 %`. Measured deviation after the fix is steady at **16.7 – 18.2 %**. Existing sliding-distinct unit tests continue to pass. ## Are there any user-facing changes? No API changes. `SlidingDistinctCountAccumulator` reservations are now accurate, so queries that previously slipped past `MemoryPool` limits may now spill or fail earlier — intended. -- 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]
