andygrove commented on code in PR #6985:
URL: https://github.com/apache/datafusion-comet/pull/6985#discussion_r4541444798
##########
spark/src/main/scala/org/apache/spark/sql/comet/execution/arrow/CachedBatchIpc.scala:
##########
@@ -234,7 +238,9 @@ private[comet] object CachedBatchIpc {
// own buffers by now, so this does not touch it.
root.clear()
- val out = new ChunkedByteBufferOutputStream(chunkSize,
ByteBuffer.allocate)
+ val out = new ChunkedByteBufferOutputStream(
+ math.min(chunkSize.toLong, messageSize(recordBatch)).toInt,
+ ByteBuffer.allocate)
Review Comment:
I ran `runBuildBenchmark` on both sides: the same tree and release libcomet,
with the only difference being whether the stream is sized to the message or
fixed at the 1 MiB chunk size. Three interleaved runs each, 5M rows.
Whole-build time does not move beyond the noise. Over Comet operators the
zstd build is 1081 ms against 1129 ms and the uncompressed build is 647 ms
against 640 ms. Over Spark operators the best times lean toward this change
(zstd 2201 ms against 2361 ms, uncompressed 1666 ms against 1893 ms), but the
same arm varied by up to 15% between runs on a busy machine, so I would not
call that a gain.
So the effect is the allocation: about 1 MiB per batch falls to 15 to 79
KiB, and the write is 13 to 18% faster per batch at 100 to 2000 rows. A whole
build is dominated by other costs, and I have not seen it regress. I have left
the description as is, since it already says allocation is where this moves.
--
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]
