andygrove opened a new pull request, #5115: URL: https://github.com/apache/datafusion-comet/pull/5115
## Which issue does this PR close? Part of #5112. ## Rationale for this change #5112 shows that native columnar-to-row conversion carries roughly 10us of fixed JNI and FFI cost per batch, which makes it up to 15.7x slower than the JVM implementation for small batches (the output of selective filters, small shuffle partitions, and broadcast dimension tables). This PR addresses the dominant part of that regression by converting small batches with the JVM implementation, and shaves part of the fixed per-batch cost by caching JNI handles. Isolated conversion benchmark from #5112 (4 columns: long, int, double, string; Apple M3 Ultra; ns/row): | batch size | JVM path | native before | native after (this PR) | |---|---|---|---| | 8192 | 7.9 | 32.6 | 32.4 (native path, unchanged) | | 512 | 8.2 | 50.3 | 49.5 (native path, JNI caching) | | 32 | 16.2 | 288.8 | 28.6 (JVM fallback, 10x faster) | ## What changes are included in this PR? - New config `spark.comet.exec.columnarToRow.native.minBatchSize` (default 512): when native columnar-to-row conversion is enabled, batches with fewer rows than this are converted with the JVM `UnsafeProjection` path inside `NativeColumnarToRowConverter`. The fallback copies each row, preserving the converter's contract that returned rows are independent copies (the same contract the native path has honored since #3367). Setting the config to 0 always converts natively. - `CometNativeColumnarToRowExec` reads the threshold on the driver and passes it to the converter in both the `doExecute` and broadcast paths. - The JNI class and constructor lookups for `NativeColumnarToRowInfo` are now resolved once per process (cached in a `OnceLock`) instead of `FindClass` plus signature lookup on every per-batch `columnarToRowConvert` call. The lookup is warmed in `columnarToRowInit` so convert calls never pay for it. The remaining fixed cost is dominated by the per-batch Arrow FFI export and import, which needs a protocol change (for example exporting the schema once per converter) and is left for follow-up work under #5112. ## How are these changes tested? - New test in `CometNativeColumnarToRowSuite` verifying the JVM fallback path returns the same rows as the native path and that both return independent row copies. - The suite's shared config now pins `minBatchSize=0` so its small test batches continue to exercise the native conversion path; the #3308 regression test constructs the converter directly and is unaffected (the constructor default is 0). - Ran `CometNativeColumnarToRowSuite` locally against the release native build. -- 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]
