andygrove commented on code in PR #6985:
URL: https://github.com/apache/datafusion-comet/pull/6985#discussion_r4506885050
##########
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:
Added the probe to `CometInMemoryCacheBenchmark` as a write-only case
("in-memory cache write", `--serialize-only` runs just it), and merged main to
clear the conflict with #7225: the sized stream now sits inside
`Utils.withRebasedOffsets`. The description has the numbers from the case
rather than from the old probe.
Allocation is where it moves: about 1 MiB per batch falls to 15 KiB at 100
rows and 79 KiB at 8192. Write time per batch is 13 to 18% lower at 100 to 2000
rows and inside the noise at 8192. I have not run `runBuildBenchmark` on both
sides, so I can't say yet what a whole cache build gains. Would you like that
run before this merges?
--
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]
