viirya opened a new pull request, #5788: URL: https://github.com/apache/datafusion-comet/pull/5788
## Which issue does this PR close? Closes #5787. ## Rationale for this change `spark.comet.shuffle.native.partitioning.hash.nested.enabled` (#5567) lets a struct, array or map be a native shuffle hash partitioning key, and nothing measured those shapes. So there was no baseline for whether native hashing of a nested key beats letting Spark do the shuffle, and no way to notice a regression in that path. The existing cases do not cover it. `shuffleArrayBenchmark` and `shuffleStructBenchmark` repartition on a nested column but have no native-shuffle case, because the gate rejected nested keys when they were written. `shuffleDeeplyNestedBenchmark` calls `repartition(n)` with no key, which is round robin rather than hash partitioning, so it never reaches the hash kernel. This is separate from #2904, which was about columnar/JVM shuffle throughput on nested schemas with a keyless `repartition(n)`. ## What changes are included in this PR? Adds `shuffleNestedHashKeyBenchmark` with Spark, Comet (Spark Shuffle), Comet (JVM Shuffle) and Comet (Native Shuffle) cases. The native case enables the nested config, so it measures the native hashing path rather than a silent fallback. The shapes separate the two code paths in the native Murmur3 kernel: a list whose elements are primitives is vectorized, while a list whose elements are themselves nested falls through to `hash_list_array!`, which slices a one-element array and re-enters `create_murmur3_hashes` for every element. ## How are these changes tested? This is a benchmark, so the result is the measurement. Apple M4 Max, `local[5]`, 1M rows, best time, relative to Spark doing the whole shuffle: | key shape | partitions | Spark | Comet native | relative | |---|---|---|---|---| | `struct<int, string>` | 5 | 79ms | **41ms** | 1.9x | | `struct<int, string>` | 201 | 126ms | **89ms** | 1.4x | | `array<int>` | 5 | 76ms | **52ms** | 1.5x | | `array<int>` | 201 | 145ms | **101ms** | 1.4x | | `struct<array<int>, string>` | 5 | 100ms | **68ms** | 1.5x | | `array<struct<int, string>>` | 5 | 164ms | 561ms | **0.3x** | | `array<struct<int, string>>` | 201 | 250ms | 647ms | **0.4x** | | `struct<map<string, int>, int>` | 5 | 121ms | **97ms** | 1.2x | | `struct<map<string, int>, int>` | 201 | 160ms | 155ms | 1.0x | `struct<array<int>, string>` at 201 partitions is omitted because it is not in this run's recorded output; a later run put it at 1.4x, in line with its 5-partition result. The native case is not silently falling back to Spark's shuffle. On a rerun of the `array<struct<..>>` case at 5 partitions the three Comet cases separate clearly, at 761ms (Spark shuffle), 1059ms (JVM shuffle) and 2340ms (native shuffle), so the native path is distinct and is the slow one. Absolute times move with machine load between runs, but the ordering and the set of shapes that win or lose reproduce. So `array<struct<..>>` is the one shape that loses, which is what the config defaults to off for. A map nested in a struct is not affected, because the kernel specializes common scalar key/value pairs. ## Additional context Measuring this is what surfaced #5777, and the fix for it is #5778. This benchmark is deliberately kept independent of that change so the before/after comparison has a reference point that is not moving at the same time. -- 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]
