andygrove commented on PR #5450: URL: https://github.com/apache/datafusion-comet/pull/5450#issuecomment-5469861596
The measurement you posted on #5397 matches what I'd expect, and I think the reason is worth writing down so we don't circle back to this later. On a wide nested schema the hashing cost isn't in the mixing function. It's in `create_hashes_internal` walking every leaf cell — per-array-type dispatch, recursion into every struct and list child, null handling — and swapping murmur3 for XXH3 changes only the innermost and cheapest part of that. XXH3's advantage is on long byte strings, whereas `xxh3_64_oneshot` called per cell over 4- and 8-byte primitives lands in its short-input path, which carries more per-call setup than murmur3's single round. So on exactly the schema shape we're targeting I'd expect it to come out behind, which is what you saw. The other half is that placement is only part of the bill. Even with a free hash, routing rows individually leaves the flush on `interleave_record_batch`, a per-row gather that re-walks every column and nested child a second time. #5449 removes the hash and the gather in one move, which is why it shifts the number by 2x rather than a few percent. There's also a narrower reason this direction doesn't really have a home. Round robin doesn't need a content hash at all, which is the premise of #5449, and hash partitioning can't change its hash — a plan can have one side of a join shuffled natively by Comet and the other by Spark, and both have to agree on murmur3 with seed 42 or the join silently drops rows. So the two partitionings that could use a faster mixer are respectively one that shouldn't be hashing and one that can't change hashes. Minor, but if we ever do want XXH3 somewhere: `native/spark-expr/Cargo.toml:43` already pulls `twox-hash` 2.1.2, which ships `xxhash3_64`, so we wouldn't need a second xxhash crate for it. Shall we close this in favour of #5449? -- 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]
