peterxcli commented on code in PR #5538:
URL: https://github.com/apache/datafusion-comet/pull/5538#discussion_r3999199901
##########
spark/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala:
##########
@@ -358,26 +400,36 @@ object Utils extends CometTypeShim with Logging {
* a single Arrow IPC stream.
*/
def coalesceBroadcastBatches(
- input: Iterator[ChunkedByteBuffer]): (Array[ChunkedByteBuffer], Long,
Long) = {
+ input: Iterator[ChunkedByteBuffer],
+ nativeIpc: Boolean = false): (Array[ChunkedByteBuffer], Long, Long) = {
Review Comment:
Alongside TPC-H, could the benchmark include many small build-side batches
and report producer serialization, driver coalescing, allocation/GC, and total
query time with direct read off/on?
Coalescing still decodes each input stream on the JVM and serializes the
combined batch again, so consumer-only timings would miss the allocation cost
raised in the LZ4 thread. These measurements would also help decide whether
coalescing needs a separate optimization.
##########
spark/src/main/scala/org/apache/spark/sql/comet/CometBroadcastExchangeExec.scala:
##########
@@ -85,7 +87,7 @@ case class CometBroadcastExchangeExec(
"number of coalesced rows for broadcast"))
override def doCanonicalize(): SparkPlan = {
- CometBroadcastExchangeExec(null, null, mode, child.canonicalized)
+ CometBroadcastExchangeExec(null, null, mode, child.canonicalized,
directRead)
Review Comment:
Keeping `directRead` in canonicalization seems necessary: `decodeBatches`
accepts both formats, but the native block reader requires codec-prefixed IPC.
In non-AQE execution, exchange reuse runs after columnar conversion, so the
consumer can already contain a `BroadcastScan` when its exchange is replaced.
Could we document this distinction and add a reuse regression covering a
native join and JVM DPP consumer? The DPP construction sites should also carry
the format choice explicitly so they match the join's exchange.
--
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]
