adriangb commented on PR #23565: URL: https://github.com/apache/datafusion/pull/23565#issuecomment-5785097696
## Benchmark bot results (head `cbdecca5ea`, merge-base `5b5bed9`) A/B = main vs this PR. A/A = main vs main (same commit both sides), the noise control. All numbers are the bot's mean times; ratios are branch / base, so >1.00 is slower with this PR. ### 1. Default settings (no memory limit, nothing spills) | Benchmark | A/B run 1 | A/B run 2 | A/A run 1 | A/A run 2 | | --- | --- | --- | --- | --- | | tpch_sf1 total | 0.99x | 0.99x | 0.97x | 1.00x | | tpcds_sf1 total | 0.99x | 0.99x | 0.99x | 0.99x | | clickbench_partitioned total | 1.02x | 1.01x | 1.00x | 1.02x | No effect, as expected: without a memory limit no operator spills, so the changed code never runs. Every A/B value is inside the A/A range. ### 2. Memory pressure: sort_tpch SF1, 512M limit, 4 partitions Mean ms, base → branch. Q1–Q7 and Q10 show no change in all four runs; Q3 fails on both sides. | Query | A/B run 1 | A/B run 2 | A/A run 1 | A/A run 2 | | --- | --- | --- | --- | --- | | Q8 (`l_comment`) | 501 → 522 (1.04x) | 486 → 513 (1.06x) | 487 → 492 | 493 → 495 | | Q9 (`l_comment`) | 556 → 573 (1.03x) | 536 → 565 (1.05x) | 530 → 535 | 535 → 548 | | Q11 (`l_comment`) | 385 → 427 (1.11x) | 378 → 410 (1.08x) | 376 → 377 | 396 → 379 | | Total | 4409 → 4477 (1.02x) | 4294 → 4398 (1.02x) | 1.00x | 1.00x | ### 3. spill_views (each query sets its own limit) | Query | A/B | A/B run 2 | A/A | | --- | --- | --- | --- | | q01 sort, 1 distinct value | 106.8 → 90.8 ms (**0.85x**) | | 1.00x, 1.02x, 1.00x | | q02 sort, 1000 distinct values | 102.4 → 100.3 ms (0.98x) | | 1.00x | | q03 sort, BinaryView, 1000 distinct | 102.0 → 100.5 ms (0.99x) | | 1.00x | | q04 sort, all distinct | 137.7 → 172.5 ms (**1.25x**) | 123.5 → 178.6 ms (**1.45x**) | 0.94x | | q05 GROUP BY, all distinct | OOM on the branch side | OOM on the branch side | passes, 1.00x (3 runs) | ### 4. Spill size and memory (spill_views) | Metric | A/B base | A/B branch | A/A base | A/A branch | | --- | --- | --- | --- | --- | | Peak spill, q01–q03 | 84.2 MiB | **30.6 MiB** | 84.2 MiB | 84.2 MiB | | Peak spill, q04 | 84.2 MiB | 74.0 MiB | 52.4 MiB | 84.2 MiB | | Peak RSS, q01–q03 | 412.6 MiB | 493.9 MiB | 432.4 MiB | 445.5 MiB | Peak spill for q01–q03 is the point of the PR: 2.8x less written. The q04 and sort_tpch spill figures are sampled once per second and move by that much between identical runs (see the A/A columns), so only the q01–q03 figures are solid. ### 5. q05 out of memory `spill_views` q05 (GROUP BY on an all-distinct string key, 96M limit, greedy pool) fails on the branch side with `ResourcesExhausted` in `FinalHashAggregateStream`. The whole suite run aborts when it happens. | | Runs | Result | | --- | --- | --- | | A/B (main vs PR) | 4 | 4 failed, always on the branch side | | A/A (main vs main) | 3 | 0 failed | Locally on macOS the same query at 96M fails about 1 in 12 runs on **both** builds, and at 160M it fails on both (10/12 main, 12/12 PR), so the limit interacts with spilling in a way that predates this PR. But on the bot the split is clean, and peak RSS for q01–q03 is about 80 MiB higher on the branch side, which suggests the PR does need more headroom. -- 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]
