Nook001 commented on PR #26941: URL: https://github.com/apache/datafusion/pull/26941#issuecomment-6714240245
### Final validation results Before marking this PR as ready for review, I'd like to share our final validation results. The fix eliminates a **98.41% memory underestimation** in our representative case. Performance testing also shows approximately **30% faster single-partition SQL execution** across all three platforms, although some non-null and merge workloads still have regressions. I'd appreciate the maintainers' feedback on whether these performance trade-offs are acceptable, and whether any additional tests or profiling data would be helpful. ### 1. Memory accounting Representative case: 8,192 input rows, 128 distinct values. | Type | Actual retained heap | Original estimate | Fixed estimate | |---|---:|---:|---:| | Generic Float64 | 147,472 B | 2,344 B | 147,472 B | | Primitive Int64 | 147,472 B | 2,344 B | 147,472 B | The fixed estimates match measured retained heap allocations across all three platforms. Values exclude the accumulator's 64-byte inline storage. ### 2. Correctness - **13,347 Rust tests passed**, 0 failed, 8 ignored. - **545/545 SQLLogicTest files** completed. - Formatting and full workspace Clippy passed. - Cross-platform allocation measurements confirmed accurate accounting. ### 3. Cross-platform performance All three systems used Rust 1.99.1 and the same benchmark suite, comparing the final implementation against upstream baseline `49c67c296`. - **Rocky Linux:** AMD EPYC 9654P, 6 CPU equivalents. - **CachyOS:** AMD Ryzen 5 5600. - **Windows 11/MSVC:** Intel Core Ultra 7 258V. #### Overall results Percentage change in geometric-mean wall time, with equal weight per scenario. Negative values indicate faster execution. | Benchmark group | Rocky Linux | CachyOS | Windows | |---|---:|---:|---:| | Single-partition SQL (10) | **−30.57%** | **−30.76%** | **−30.38%** | | Merge SQL (8) | +2.20% | −7.44% | −7.51% | | Controls, execution (5) | −5.00% | −4.81% | −7.43% | | Controls, end-to-end (5) | −3.17% | −2.82% | −5.12% | Single-partition SQL improved by approximately **30% on all three platforms**. #### Representative workloads Selected individual SQL results from Linux: | Workload | Rocky Linux | CachyOS | |---|---:|---:| | NDV 128, random NULLs | −75.48% | −75.62% | | NDV 65536, random NULLs | −44.18% | −43.62% | | NDV 128, non-null | +7.65% | +9.73% | | NDV 65536, non-null | +7.71% | +6.00% | The nullable Int64 control workload also improved consistently: | Metric | Rocky Linux | CachyOS | Windows | |---|---:|---:|---:| | Nullable Int64 execution | −23.25% | −20.78% | −26.31% | #### Remaining regressions - **Non-null SQL:** approximately 6–10% slower in the representative Linux cases. - **Merge SQL:** 2.20% slower overall on Rocky, versus approximately 7.5% faster on CachyOS and Windows. - **Rocky P128 merge:** individual wall-time regressions of 13.6–30.3%, despite CPU time decreasing by 2.7–15.1% in the same cases. -- 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]
