adriangb commented on PR #24857: URL: https://github.com/apache/datafusion/pull/24857#issuecomment-5513979857
## Repeat runs: RSS question resolved, and two corrections Following up on the open item in the summary above. Six further runs of `clickbench_extended` at `DATAFUSION_RUNTIME_MEMORY_LIMIT: 4G`, three of them base-against-base so both sides are the identical commit, which gives a null distribution to measure everything else against. ### Peak RSS: no effect. The earlier increase was a single high draw. Peak RSS at the merge-base across 11 readings of identical code: 9.4, 9.5, 9.7, 9.9, 10.0, 10.1, 10.2, 10.3, 10.4, 10.6, 10.6 GiB. Mean 10.06 GiB, standard deviation 0.41 GiB (4.1%), and a max/min span of **12.8%**. Within-run RSS delta on identical code: 0.0%, -2.9%, -4.0%. On the three base-vs-branch repeats: **-2.9%, -2.1%, -6.7%**, against the +8.1% originally reported. Welch t about -0.87, p about 0.43. There is also a mild order effect: the base side runs first in each pod and the second side reads about 2.3% lower on average, so the original +8.1% was measured against that bias rather than with it. The allocator-churn hypothesis is not supported and no mitigation is warranted. ### Query 2 reproduces exactly. The single-run caveat is retired. Null spread on identical code, 6 readings: 98.1, 98.1, 98.2, 98.2, 98.2, 98.6 MiB, a mean delta of +0.10% with standard deviation 0.27. Pool peak reproduces to well under 1%. Across the three repeats the changed side read 11.7 MiB every time, giving **-88.12%, -88.09%, -88.12%**, standard deviation 0.02 percentage points. With the original run that is four for four at -88.1%. It is also mechanistically predicted rather than only measured. `Entry<()>` in the view map is a `u128` plus a `u64`, 32 bytes aligned, and `HashTable::with_capacity(512)` rounds to 1024 buckets, so each pre-allocated per-group set costs about 33.8 KiB. Q2 is four `COUNT(DISTINCT)` aggregates grouped by `BrowserCountry`, roughly 2,900 accumulators, predicting about 96 MiB against the 98.3 MiB observed. Query 1 also holds: 3.4 MiB base and 2.4 MiB changed in every run, with zero variance on either side. ### Correction 1: the Query 12 number was noise. Retracting it. The summary above cited Q12 at -21.2%. That does not survive. The null spread on Q12 is -7.1% to +18.2% (standard deviation 14.6) on identical code, and the repeats gave -9.1%, -9.1% and +7.7%. Q12 is `MAX(FIRST_VALUE(...) GROUP BY "OS")` with no `COUNT(DISTINCT)` anywhere in it, so there is no mechanism by which this change could touch it. It is roughly a 1.2 MiB quantity reported to 0.1 MiB granularity. Please disregard that row. ### Correction 2: Query 0's pool peak genuinely increases, by about 4.2% This is a real, opposite-signed cost that the first run did not surface, and it should be on the record. Base commit, 11 readings: 796.8 to 840.8 MiB, mean 821.9. Branch, 4 readings: 846.5, 850.5, 858.5, 871.0 MiB, mean 856.6. **All four branch readings exceed all eleven base readings** (exact rank p = 0.0007). Restricting to second-position readings only, to control for the order effect, all four branch values still exceed all three base values: +4.2%, p = 0.029, about +35 MiB. Q0 is `COUNT(DISTINCT)` over three high-cardinality strings with **no** `GROUP BY`, so a handful of maps that each grow to millions of entries. That is the opposite population from the one this change targets, and the result is the expected flip side of removing a warm-up: a map destined to get large now reaches its size through more doubling steps, and the pool records the transient where the old and new tables are both live. The overall picture is coherent. Ungrouped and low cardinality (Q1) improves because the pre-allocation dominated; grouped with many tiny sets (Q2) improves enormously for the same reason; ungrouped and high cardinality (Q0) costs a little, because the pre-allocation was never the dominant term there. ### A note on this harness that applies beyond this PR Pool peak is a trustworthy instrument here and peak RSS is not. Pool peak reproduced to under 1% on a 98 MiB query and exactly on 3.4 and 11.7 MiB ones. Peak RSS has a 4.1% standard deviation plus a 2.3% systematic order bias, so any claim resting on a single-run RSS figure needs at least three runs and a null. Pool figures around 1 MiB, reported to 0.1 MiB, need repeats too, which is what caught Q12. Result comments: base-vs-base [1](https://github.com/apache/datafusion/pull/24857#issuecomment-5513902626), [2](https://github.com/apache/datafusion/pull/24857#issuecomment-5513918481), [3](https://github.com/apache/datafusion/pull/24857#issuecomment-5513921246); base-vs-branch [1](https://github.com/apache/datafusion/pull/24857#issuecomment-5513878693), [2](https://github.com/apache/datafusion/pull/24857#issuecomment-5513885404), [3](https://github.com/apache/datafusion/pull/24857#issuecomment-5513887805). -- 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]
