sunchao commented on code in PR #5875:
URL: https://github.com/apache/datafusion-comet/pull/5875#discussion_r3997504094
##########
spark/src/main/scala/org/apache/comet/serde/maps.scala:
##########
@@ -114,7 +117,9 @@ object CometMapValues extends
CometExpressionSerde[MapValues] {
}
}
-object CometMapExtract extends CometExpressionSerde[GetMapValue] {
+object CometMapExtract extends CometExpressionSerde[GetMapValue] with
CodegenDispatchFallback {
Review Comment:
### Performance
[P2] Could you add and run map-lookup cases in
`CometCodegenDispatchBenchmark` before enabling these routes by default? This
mixin makes previously falling-back projections pay the dispatcher's per-task
kernel setup, per-batch Arrow output allocation and JVM map traversal. Whether
retaining Comet outweighs those costs depends on the workload, particularly for
small maps and lookup-only projections. The PR explicitly reports no
benchmarks, and the existing driver has no map-lookup case. Please compare
dispatcher on, dispatcher off and pure Spark using column inputs, short and
larger maps, scalar and complex keys, and both lookup-only and mixed
projections. Include first-use and warmed results with answer and route checks
so the new default has measured support. This is a request for missing
evidence, not a measured slowdown.
--
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]
