andygrove opened a new issue, #6597: URL: https://github.com/apache/datafusion-comet/issues/6597
### What is the problem the feature request solves? `spark.comet.shuffle.jvm.preferDictionary.ratio` defaults to 10. Any value above 1.0 makes the JVM columnar shuffle's native converter build every top-level string and binary column with a `StringDictionaryBuilder` or `BinaryDictionaryBuilder`, which hashes each value. At the end of each batch, `builder_to_array` keeps the dictionary only if there are more than ten keys per distinct value. Otherwise it casts the dictionary back to a plain array, copying the bytes a second time. On a column of short, mostly distinct strings (8192 rows of about 11 bytes, every tenth row null), the conversion costs 20-23 ns per row with the default, against 12 with the ratio at 0. Building the dictionary accounts for about half of the difference and the cast back for the rest. With the ASCII fast path proposed in #6596, it would be 16.7 ns against 5.1. When a column does repeat, the dictionary may pay for itself in shuffle bytes, so the default can't simply be turned off without measuring both sides. The config's doc says it is "only used when `spark.comet.shuffle.mode` is `jvm`". `CometShuffleExternalSorter` and `CometDiskBlockWriter` read it whenever the JVM columnar shuffle runs. That includes the default `auto` mode, for every shuffle whose child is a Spark plan. ### Describe the potential solution This needs measurements of write time and shuffle size across cardinalities first. Options: - Stop trying the dictionary for a column once it has failed to pay off. `process_sorted_row_partition` runs once per partition per spill, and a partition often has fewer rows than a batch, so the decision would have to outlive one call or come from the JVM. - Turn the trial off by default, if dictionary-encoded blocks rarely end up smaller once LZ4 has compressed them. Either way, the config's doc should say that it applies to the JVM columnar shuffle in `auto` mode too. ### Additional context Found while comparing the native converter with `ArrowWriter` (#6596), with the same measurement setup. -- 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]
