peter-toth commented on code in PR #59998:
URL: https://github.com/apache/spark/pull/59998#discussion_r4436648005


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/arrow/ArrowCompressionUtils.scala:
##########
@@ -105,6 +113,28 @@ private[sql] class 
BoundedZstdCompressionCodec(compressionLevel: Int)
     compressedBuffer.writerIndex(CompressionUtil.SIZE_OF_UNCOMPRESSED_LENGTH + 
bytesWritten)
     compressedBuffer
   }
+
+  override def compress(allocator: BufferAllocator, uncompressedBuffer: 
ArrowBuf): ArrowBuf = {
+    val compressed = super.compress(allocator, uncompressedBuffer)
+    val length = compressed.writerIndex()
+    // Requiring at least half of the buffer back also limits the extra memory 
that the copy needs
+    // while both buffers are alive. Empty and raw outputs are already in a 
buffer of their size.
+    if (allocator.getRoundingPolicy.getRoundedSize(length) > 
compressed.capacity() / 2) {

Review Comment:
   **Finding 1.** The threshold is only relative, so small buffers are copied 
too. A 100-row `BIGINT` column, for example, gets a 1 KiB `compressBound` 
buffer. Copying its output gives back less than 1 KiB, but it still costs an 
allocation, a copy and a release. On such buffers the head does more work than 
base for no real memory gain.
   
   The CPU table in the description shows this. 100-row batches cost up to 8.6% 
more in `getRecordBatch` and `close`. 1,000-row batches cost up to 3.5% more. 
Small batches are common with grouped and cogrouped pandas UDFs, because each 
group is written as its own batch.
   
   A floor on the absolute saving would keep these buffers on the base path, 
for example:
   
   ```scala
       val roundedLength = allocator.getRoundingPolicy.getRoundedSize(length)
       if (roundedLength > compressed.capacity() / 2 ||
           compressed.capacity() - roundedLength < MIN_SHRINK_SAVING_BYTES) {
         compressed
   ```
   
   The value is your call. A higher floor gives up a little memory on small 
batches to get their base CPU cost back.
   



-- 
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]

Reply via email to