karuppayya commented on code in PR #4010:
URL: https://github.com/apache/datafusion-comet/pull/4010#discussion_r3114230272
##########
spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala:
##########
@@ -97,6 +97,32 @@ case class CometExecRule(session: SparkSession) extends
Rule[SparkPlan] {
private lazy val showTransformations =
CometConf.COMET_EXPLAIN_TRANSFORMATIONS.get()
+ /**
+ * Revert any `CometShuffleExchangeExec` with `CometColumnarShuffle` that is
sandwiched between
+ * two non-Comet operators back to the original Spark `ShuffleExchangeExec`.
Columnar shuffle
+ * converts row-based input to Arrow batches for the shuffle read side; if
neither the parent
+ * nor the child is a Comet plan that can consume columnar output, that
conversion is pure
+ * overhead (row->arrow->shuffle->arrow->row vs. row->shuffle->row).
+ */
+ private def revertRedundantColumnarShuffle(plan: SparkPlan): SparkPlan = {
Review Comment:
Thanks for the opttimzation @andygrove
I guess this is the first time where `CometShuffleExchangeExec` is reverted
back to a plain `ShuffleExchangeExec`.
The two shuffle paths use different memory systems:
- Comet columnar shuffle uses Comet's own memory pool. (off-heap)
- Spark vanilla shuffle uses the JVM execution memory pool , with spills
managed by ExternalSorter.
Users who have tuned their clusters for Comet (smaller JVM heap) could see
unexpected spills after this chang, shifting shuffle memory pressure back to
theJVM.
Additionally, Comet's Arrow IPC columnar format typically compresses better
than Spark's row-based UnsafeRowSerializer path, so shuffle I/O mayalso
increase.
It would be good to document or log when a shuffle is reverted so users can
correlate any unexpected behavior with this optimization.
--
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]
