andygrove commented on PR #6180: URL: https://github.com/apache/datafusion-comet/pull/6180#issuecomment-6130147769
I ran this PR against main (fallback), the codegen dispatcher and Spark on a release build (Spark 4.1, 64M values per dataset, interleaved runs, wall and process CPU time). The native kernel holds up well. For the default `na.drop()` it used 0.7-1.0x Spark's CPU on numeric columns and 0.5-0.75x on strings, and it keeps a following group-by native. Dispatch was the slowest option everywhere, roughly 2-16x the native predicate cost and up to 64% more CPU end to end, which matches what #6093 reported. So I think native is the right direction. My main concern is the `smallBatchThreshold` config. The general-n counting step cost 18-65 ms per 64M values in my runs, against roughly 400 ms for the scan, and batches are normally 8192 rows, so I don't see anything for a user to tune there. Could we drop the config, its proto field and the tuning docs, and keep a single counting strategy? If the simpler row-counter path is close at 8192-row batches, I'd lean towards keeping only that one. One thing to be aware of, though I don't think it should block this PR. On a narrow, tall table where every column is consumed (4 double columns, 16M rows, `na.drop()` followed by a sum of each column), the native plan was about 1.7x slower in wall time than the fallback on main. That cost is DataFusion's FilterExec copying the surviving rows, which Spark's fused codegen avoids, so it applies to any native filter rather than to this kernel. I've opened #6908 to track it. -- 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]
