viirya opened a new pull request, #5765: URL: https://github.com/apache/datafusion-comet/pull/5765
## Which issue does this PR close? None; raised in review of #5754. ## Rationale for this change There is no hash benchmark among the benches in `native/spark-expr`, although these kernels back the `hash` and `xxhash64` expressions and, since #5567, native shuffle hash partitioning. @andygrove noted the gap while reviewing #5754. ## What changes are included in this PR? `native/spark-expr/benches/hash.rs`, covering `int32` and `utf8` as reference points and then the shapes that are now admissible as partitioning keys: `struct`, `array<int32>`, `array<struct<..>>` and `map<utf8, int32>`. The two list shapes are deliberately adjacent. A list whose elements are primitives is hashed by a vectorized path, while a list whose elements are nested falls through to a per-element path, and the benchmark makes that split visible: | case | time | |---|---| | `int32` | 7.26 µs | | `utf8` | 18.2 µs | | `struct` | 25.7 µs | | `array<int32>` x10 | 141 µs | | `map` x10 | 590 µs | | `array<struct<..>>` x10 | **9660 µs** | 8192 rows, Apple M4 Max. The last two list rows hold the same number of elements, so the 68x is the per-element path rather than the volume of data. That is the shape behind `spark.comet.shuffle.native.partitioning.hash.nested.enabled` defaulting to off in #5567. Only murmur3 is covered: `create_xxhash64_hashes` is `pub(crate)`, so a benchmark cannot reach it, and widening visibility purely for a benchmark seemed the wrong trade. The two share `create_hashes_internal!`, so a change to that macro still shows up here. ## How are these changes tested? It is a benchmark, so there is nothing to assert. `cargo bench -p datafusion-comet-spark-expr --bench hash` produces the table above; `cargo fmt --check`, `clippy -D warnings` and the existing 710 + 5 tests are clean. ## Additional context I have separate work in progress on the `array<struct<..>>` path, and landing the benchmark on its own means those numbers can be reproduced against `main` rather than against a benchmark that arrives with the change it is measuring. -- 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]
