chore: Codegen fallback explain for plan#4891
Conversation
|
|
||
| ``` | ||
| CometNativeColumnarToRow | ||
| +- CometProject [COMET-INFO: hypot: routes through JVM codegen dispatcher, levenshtein: routes through JVM codegen dispatcher] |
There was a problem hiding this comment.
Is it possible to combine the list of expressions to avoid duplication e.g. hypot and Levenshtein route through the JVM codegen dispatcher ?
|
Why make this configurable? Don't we always want this? |
This new tagging seems extremely verbose and seems more like a debugging tool. I don't think end users need to know about internal implementation details as long as Comet is executing the expression on the Arrow-native path.
We should explain this in https://datafusion.apache.org/comet/user-guide/0.17/understanding-comet-plans.html |
|
I was thinking about another approach for this. I haven't explored how practical it is to implement though. The extended explain plan just shows operators, but what if we include expressions and use a symbol to annotate whether the expression is rust or jvm? For example, here |
The way to do it, I think, would be to have a new tag for the expression nodes and then the explain plan generator can format it however we want it to be displayed. |
Maybe lets have at least possibility to recognize fallback codegen using this tool and we can improve it in follow up PR?
Did it configurable so it won't bleed to golden files, WDYT would be better? |
Adds a new
spark.comet.explainCodegen.enabledconfig (defaultfalse) that annotates each Spark expression routed through the JVM codegen dispatcher with a[COMET-INFO: <expr>: routes through JVM codegen dispatcher]message on the surrounding Comet operator. Also fills in the previously silent fallback branches insideCometScalaUDF.emitJvmCodegenDispatchso everyNonereturn now tags the expression viawithFallbackReasonwith an<expr>: <reason>prefix.What changed
spark/src/main/scala/org/apache/comet/CometConf.scala— newCOMET_EXPLAIN_CODEGEN_ENABLEDconfig in theCATEGORY_EXEC_EXPLAINcategory, modeled afterCOMET_EXPLAIN_FALLBACK_ENABLED. Defaultfalse, so behavior is off unless opted in.spark/src/main/scala/org/apache/comet/serde/CometScalaUDF.scalareturn NoneinemitJvmCodegenDispatchnow callswithFallbackReason(expr, "<prettyName>: ...")first (payload/data-arg/return-type serialization failures).canHandlereject) get the same<prettyName>:prefix so multiple expressions in one projection each surface distinctly (Set[String]no longer collapsesidentical strings).
withInfo(expr, "<prettyName>: routes through JVM codegen dispatcher")when the new flag is on.withInfowrites onlyEXTENSION_INFO, so no planning rule flips and the operatorremains Comet-native.
CometExecRule.rollUpInfoMessagespropagates it toCometProjectExec/ etc. for verbose explain rendering.spark/src/test/scala/org/apache/comet/CometCodegenSuite.scala— new testexplainCodegen.enabled surfaces routed expressions in COMET-INFOrunsSELECT hypot(a,b), levenshtein(s1,s2), asserts theplan is Comet-native (
checkSparkAnswerAndOperator), and asserts both expressions surface in the[COMET-INFO: ...]block.docs/source/user-guide/latest/understanding-comet-plans.md— adds a row + section for the new config alongside the existingexplainFallback/explain.format/explain.nativeconfigs, with anexample showing the exact rendered output.
Example
Why
Previously, expressions running through the JVM codegen dispatcher were indistinguishable from natively-executed ones in explain output. Users investigating perf / native coverage had no way to tell which
sub-expressions were running Spark's own
doGenCodeinside a Comet kernel vs a true DataFusion implementation. And when the dispatcher declined (return-type unsupported, closure-serialize failed, etc.), theplan silently fell back to Spark with no annotation on the expression.