viirya opened a new pull request, #5822: URL: https://github.com/apache/datafusion-comet/pull/5822
## Which issue does this PR close? Closes #5818. ## Rationale for this change The end-to-end shuffle results in #5788 include planning, scan, key construction, normalization, hashing, shuffle and sink consumption. They cannot establish which part accounts for the larger-map cost. The existing hash and map-sort Criterion benchmarks use different inputs, so their results cannot be directly decomposed either. ## What changes are included in this PR? - Add shared inputs matching #5788's SplitMix64/pmod c1 generator, decimal string keys, aligned integer values and `struct<m: map<string,int>, i: int>` field order. Use the first 8,192 row ids of that sequence, retaining singleton maps and adding variable 2–10 / 2–50 entries in forward and reversed order. - Extend `hash.rs` with hashing over pre-normalized maps and structs, normalization followed by hashing, and a hash-buffer seed-reset control. Extend `map_sort.rs` with matched normalization-only cases. Preserve the existing benchmarks. - Check cardinalities, complete value sets, key/value alignment, lexical ordering, normalized forward/reversed equality and identical final map/struct hashes outside timing. Also check struct hashes against hashing m then i. - Report row throughput and actual entry counts. Input generation and hash-buffer allocation are outside timing; seed-42 reset is timed. Normalization includes internal allocations and output destruction. Combined struct cases include rebuilding/dropping the struct wrapper, while hashing-only wrappers are prebuilt. Outputs are consumed with `black_box`. Normalization-only measures `spark_map_sort` on the map child once, since that operation is shared by the bare map and enclosing struct. Hashing-only forward/reversed inputs are equivalent after normalization: timing differences between those labels are sampling variation, not sensitivity to raw map order. No production hashing, sorting, admission rules or nested-hashing defaults change. ## How are these changes tested? - Built both Criterion executables in the release/bench profile; all **31 new smoke cases passed**. - Release unit tests: **9 map_sort tests and 11 Murmur3 tests passed**. - `cargo fmt --all -- --check` and `git diff --check` passed. - Ran all **31 new Criterion measurements serially**, with 100 samples, 3 s warmup and a 5 s measurement target per case. Criterion extends sampling for slower cases. No other benchmark or heavy build was started during measurement. - A full `make` attempt with the restored upstream lockfile failed at dependency resolution: this environment's registry proxy lacks locked `aws-smithy-runtime-api` 1.16.0. JVM compilation was not reached. The release checks and measurements below used the disclosed local lockfile resolution, not an exact upstream-lockfile build. ### Matched measurements Measured 2026-09-09 on Apple M4 Max, 16 CPUs, 64 GiB RAM, macOS 26.6.2, AC power. Rust 1.97.1, aarch64-apple-darwin, LLVM 22.1.6; Criterion 0.7.0; DataFusion / DataFusion Spark 55.0.0. Repo release settings: thin LTO, codegen-units=1, overflow-checks=false, debug=true, strip=debuginfo; no RUSTFLAGS or target-cpu override. Benchmark implementation in `d5545c1863a95ff245014743b7f992df4072a29b`, based on upstream `fd8e09e699e4e5e8a399616083d292dfb49cbb78`. Measurement preceded only a comment clarification, with no executable-code change. Standalone executables directly link the Rust kernels; no Spark runtime, Maven artifact or separately copied Comet native library is involved. Executable checksums, successful build logs, commands, dependency lockfiles and raw Criterion samples are retained in the local validation bundle. **Dependency qualification:** upstream dependencies could not be fetched through the proxy, and direct crates.io was unreachable. A fresh local `cargo update --offline` selected Arrow facade 59.2.0 with Arrow array/ord/select and other subcrates 59.3.0; Parquet and parquet-variant packages 59.2.0. Other local downgrades were async-compression 0.4.43, aws-smithy-runtime-api 1.15.0, aws-smithy-types 1.6.2, compression-codecs 0.4.38, compression-core 0.4.32, mio 1.2.2 and tinyvec 1.12.0. The original lockfile is restored and **no dependency workaround is committed**. These measurements are not mixed with the prior #5788 cohort. Below: **ms per 8,192-row batch**, point estimate and **95% Criterion confidence interval**, for `struct<map<string,int>,int>`. N measures its map child; C includes struct reconstruction. Criterion's slope estimate is used where available, otherwise its mean estimate. | Entries/row | Input order | Hash only (H) | Normalize only (N) | Normalize + hash (C) | |---|---|---:|---:|---:| | 1 | forward | 0.0358 [0.0358, 0.0359] | 0.9121 [0.9076, 0.9176] | 0.9363 [0.9324, 0.9406] | | 1 | reversed | 0.0359 [0.0359, 0.0360] | 0.9248 [0.9228, 0.9270] | 0.9394 [0.9355, 0.9437] | | 2–10 | forward | 0.3147 [0.3136, 0.3159] | 1.4342 [1.4305, 1.4386] | 1.8228 [1.8180, 1.8275] | | 2–10 | reversed | 0.3106 [0.3098, 0.3113] | 1.8528 [1.8436, 1.8667] | 2.2370 [2.2305, 2.2445] | | 2–50 | forward | 2.0013 [1.9950, 2.0082] | 2.8697 [2.8627, 2.8783] | 5.0505 [5.0330, 5.0699] | | 2–50 | reversed | 2.0259 [2.0149, 2.0386] | 3.5534 [3.5473, 3.5597] | 5.7294 [5.7154, 5.7446] | Normalization is the larger standalone component for these inputs. For 2–50 entries it is 1.43x hashing in forward order and 1.75x in reversed order, but hashing still accounts for 41% / 36% of standalone H+N. This supports profiling the complete `spark_map_sort` path first, including per-row sorting, index construction, `take` and allocation; it does not identify sorting comparisons alone as the bottleneck. The wide-map hash loop remains material. These native measurements do not explain the entire JVM/native end-to-end difference or establish a defect in either kernel. C and H+N are separately sampled and need not match exactly. The seed-reset control measured 0.269 µs per batch (95% CI 0.265–0.273 µs); buffer allocation is excluded and this reset cost is not subtracted from H or C. **Scope:** data is non-null with 8,192 / 48,996 / 213,919 total entries per batch. This first batch happens not to cross any decimal key-width boundaries, so its forward order is also lexical order; keys are still unpadded decimal strings generated as in #5788. It does not cover all 1,048,576 rows, null/empty maps or long strings. Runs were sequential on a desktop without CPU pinning; confidence intervals describe within-run sampling, not cross-run or thermal variation. <details> <summary>All cases, including bare maps and row/entry throughput</summary> | Case | µs / batch (95% CI) | M rows/s | M entries/s | |---|---:|---:|---:| | matched_maps/hash_buffer_seed_reset | 0.269 [0.265, 0.273] | — | — | | matched_maps/hash_only/map/10/forward | 303.519 [302.876, 304.197] | 26.990 | 161.426 | | matched_maps/hash_only/map/10/reversed | 302.539 [301.755, 303.416] | 27.078 | 161.950 | | matched_maps/hash_only/map/1/forward | 29.132 [29.089, 29.173] | 281.204 | 281.204 | | matched_maps/hash_only/map/1/reversed | 29.125 [29.064, 29.192] | 281.274 | 281.274 | | matched_maps/hash_only/map/50/forward | 1982.576 [1976.738, 1989.338] | 4.132 | 107.900 | | matched_maps/hash_only/map/50/reversed | 1983.352 [1979.674, 1987.201] | 4.130 | 107.857 | | matched_maps/hash_only/struct_map_int/10/forward | 314.747 [313.647, 315.903] | 26.027 | 155.668 | | matched_maps/hash_only/struct_map_int/10/reversed | 310.582 [309.849, 311.288] | 26.376 | 157.755 | | matched_maps/hash_only/struct_map_int/1/forward | 35.847 [35.791, 35.907] | 228.528 | 228.528 | | matched_maps/hash_only/struct_map_int/1/reversed | 35.931 [35.896, 35.963] | 227.993 | 227.993 | | matched_maps/hash_only/struct_map_int/50/forward | 2001.285 [1994.990, 2008.248] | 4.093 | 106.891 | | matched_maps/hash_only/struct_map_int/50/reversed | 2025.901 [2014.875, 2038.650] | 4.044 | 105.592 | | matched_maps/normalize_hash/map/10/forward | 1817.499 [1806.236, 1830.089] | 4.507 | 26.958 | | matched_maps/normalize_hash/map/10/reversed | 2220.010 [2213.478, 2227.320] | 3.690 | 22.070 | | matched_maps/normalize_hash/map/1/forward | 930.957 [927.037, 935.349] | 8.800 | 8.800 | | matched_maps/normalize_hash/map/1/reversed | 934.486 [930.197, 938.759] | 8.766 | 8.766 | | matched_maps/normalize_hash/map/50/forward | 5006.378 [4995.133, 5018.801] | 1.636 | 42.729 | | matched_maps/normalize_hash/map/50/reversed | 5686.146 [5676.449, 5696.164] | 1.441 | 37.621 | | matched_maps/normalize_hash/struct_map_int/10/forward | 1822.775 [1817.980, 1827.520] | 4.494 | 26.880 | | matched_maps/normalize_hash/struct_map_int/10/reversed | 2237.048 [2230.502, 2244.471] | 3.662 | 21.902 | | matched_maps/normalize_hash/struct_map_int/1/forward | 936.334 [932.390, 940.635] | 8.749 | 8.749 | | matched_maps/normalize_hash/struct_map_int/1/reversed | 939.446 [935.460, 943.661] | 8.720 | 8.720 | | matched_maps/normalize_hash/struct_map_int/50/forward | 5050.528 [5032.982, 5069.861] | 1.622 | 42.356 | | matched_maps/normalize_hash/struct_map_int/50/reversed | 5729.384 [5715.395, 5744.583] | 1.430 | 37.337 | | matched_maps/normalize_only/map/10/forward | 1434.230 [1430.492, 1438.630] | 5.712 | 34.162 | | matched_maps/normalize_only/map/10/reversed | 1852.846 [1843.559, 1866.704] | 4.421 | 26.444 | | matched_maps/normalize_only/map/1/forward | 912.106 [907.615, 917.588] | 8.981 | 8.981 | | matched_maps/normalize_only/map/1/reversed | 924.842 [922.824, 926.981] | 8.858 | 8.858 | | matched_maps/normalize_only/map/50/forward | 2869.673 [2862.711, 2878.269] | 2.855 | 74.545 | | matched_maps/normalize_only/map/50/reversed | 3553.405 [3547.347, 3559.740] | 2.305 | 60.201 | Throughput is in millions of rows or entries per second, derived from actual batch counts. The seed-reset control has no row/entry throughput. </details> Reproduction with available dependencies: ```sh cargo bench --manifest-path native/Cargo.toml -p datafusion-comet-spark-expr --bench hash --bench map_sort --no-run --locked cargo bench --manifest-path native/Cargo.toml -p datafusion-comet-spark-expr --bench hash -- matched_maps --sample-size 100 --warm-up-time 3 --measurement-time 5 --noplot cargo bench --manifest-path native/Cargo.toml -p datafusion-comet-spark-expr --bench map_sort -- matched_maps --sample-size 100 --warm-up-time 3 --measurement-time 5 --noplot ``` The recorded run executed the freshly built binaries directly with those Criterion options and a dedicated `CRITERION_HOME`; using the committed upstream lockfile on an unrestricted registry produces a different dependency cohort from the locally qualified numbers above. -- 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]
