This is an automated email from the ASF dual-hosted git repository.

pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-grpc.git


The following commit(s) were added to refs/heads/main by this push:
     new a670bbce Avoid buffer copies with a non-synchronized 
ByteStringOutputStream (#907)
a670bbce is described below

commit a670bbcec5b3f00d6ec01be1d34edeae0bba8dbd
Author: PJ Fanning <[email protected]>
AuthorDate: Tue Oct 13 10:34:26 2026 +0100

    Avoid buffer copies with a non-synchronized ByteStringOutputStream (#907)
    
    * Avoid buffer copy on Gzip compress/uncompress.
    
    * Use the non-synchronized ByteStringOutputStream from Pekko HTTP.
    
    Copied from apache/pekko-http http-core ByteStringOutputStream, which hands 
the
    buffered data over as a ByteString without copying and avoids the 
synchronized
    writes of ByteArrayOutputStream.
    
    Co-Authored-By: Claude Opus 5.5 <[email protected]>
    
    * Avoid private[this], which Scala 3.9 warns about.
    
    Co-Authored-By: Claude Opus 5.5 <[email protected]>
    
    * Add a benchmark for gzip and InputStream buffering.
    
    Motivation:
    The buffers used by Gzip.compress, Gzip.uncompress and
    ByteStringUtils.fromInputStream had no benchmark, so changes to them
    could not be measured.
    
    Modification:
    Add GzipBenchmark, covering compress, uncompress and fromInputStream
    for 1 KiB, 64 KiB and 1 MiB payloads.
    
    Result:
    The ByteStringOutputStream change can be compared with main.
    
    Tests:
    - sbt benchmarks/Jmh/compile: passed
    - sbt "benchmarks/Jmh/run -prof gc org.apache.pekko.grpc.GzipBenchmark": 
run on main and this branch
    
    References:
    Refs #905
    
    * Avoid deprecated = _ initializers in GzipBenchmark.
    
    Scala 3.9 deprecates them, which fails the next build under -Werror.
    
    ---------
    
    Co-authored-by: Tim Whittington <[email protected]>
    Co-authored-by: Claude Opus 5.5 <[email protected]>
    Co-authored-by: PJ Fanning <[email protected]>
---
 .../org/apache/pekko/grpc/GzipBenchmark.scala      |  63 ++++++++++++
 .../grpc/internal/ByteStringOutputStream.scala     | 113 +++++++++++++++++++++
 .../pekko/grpc/internal/ByteStringUtils.scala      |  34 ++++---
 .../org/apache/pekko/grpc/internal/Gzip.scala      |  30 ++----
 .../grpc/internal/ByteStringOutputStreamSpec.scala | 104 +++++++++++++++++++
 5 files changed, 307 insertions(+), 37 deletions(-)

diff --git 
a/benchmarks/src/main/scala/org/apache/pekko/grpc/GzipBenchmark.scala 
b/benchmarks/src/main/scala/org/apache/pekko/grpc/GzipBenchmark.scala
new file mode 100644
index 00000000..07c380d6
--- /dev/null
+++ b/benchmarks/src/main/scala/org/apache/pekko/grpc/GzipBenchmark.scala
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.grpc
+
+import java.io.ByteArrayInputStream
+
+import scala.util.Random
+
+import org.apache.pekko
+import pekko.grpc.internal.{ ByteStringUtils, Gzip }
+import pekko.util.ByteString
+import org.openjdk.jmh.annotations._
+
+// Microbenchmarks for the buffers used by gzip compression and by reading a 
message from an InputStream.
+class GzipBenchmark extends CommonBenchmark {
+  @Param(Array("1024", "65536", "1048576"))
+  var size: Int = 0
+
+  private var uncompressed: ByteString = ByteString.empty
+  private var compressed: ByteString = ByteString.empty
+  private var uncompressedArray: Array[Byte] = Array.emptyByteArray
+
+  @Setup
+  def setup(): Unit = {
+    // Compressible text, closer to a typical message than random bytes
+    val random = new Random(42)
+    val words = Vector.fill(256)(random.alphanumeric.take(3 + 
random.nextInt(8)).mkString)
+    val builder = new StringBuilder(size + 16)
+    while (builder.length < size) 
builder.append(words(random.nextInt(words.size))).append(' ')
+    uncompressedArray = builder.toString.take(size).getBytes("UTF-8")
+    uncompressed = ByteString(uncompressedArray)
+    compressed = Gzip.compress(uncompressed)
+  }
+
+  @Benchmark
+  def compress(): ByteString =
+    Gzip.compress(uncompressed)
+
+  @Benchmark
+  def uncompress(): ByteString =
+    Gzip.uncompress(compressed, Int.MaxValue)
+
+  // A plain ByteArrayInputStream is not a KnownLength stream, so messages 
larger than the 32 KiB read buffer
+  // take the buffering path.
+  @Benchmark
+  def fromInputStream(): ByteString =
+    ByteStringUtils.fromInputStream(new 
ByteArrayInputStream(uncompressedArray))
+}
diff --git 
a/runtime/src/main/scala/org/apache/pekko/grpc/internal/ByteStringOutputStream.scala
 
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/ByteStringOutputStream.scala
new file mode 100644
index 00000000..4d64c777
--- /dev/null
+++ 
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/ByteStringOutputStream.scala
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.grpc.internal
+
+import java.io.OutputStream
+import java.util.Arrays
+
+import org.apache.pekko
+import pekko.annotation.InternalApi
+import pekko.util.ByteString
+
+/**
+ * INTERNAL API
+ *
+ * An [[java.io.OutputStream]] that buffers into a byte array like 
[[java.io.ByteArrayOutputStream]] but
+ * that hands the buffered data over as a [[pekko.util.ByteString]] without 
copying it, unlike
+ * `ByteArrayOutputStream.toByteArray` which always creates a copy, and whose 
writes are not
+ * `synchronized`: every user owns the stream it writes to, so the per-call 
monitor is pure overhead.
+ *
+ * After a hand-over the stream is empty again and can be reused for the next 
block of data.
+ *
+ * Not thread-safe.
+ *
+ * Copied from the `ByteStringOutputStream` in Apache Pekko HTTP.
+ */
+@InternalApi
+private[grpc] final class ByteStringOutputStream(initialCapacity: Int) extends 
OutputStream {
+  if (initialCapacity < 0) throw new IllegalArgumentException(s"Illegal 
initial capacity: $initialCapacity")
+
+  private var buf: Array[Byte] = Array.emptyByteArray
+  private var count: Int = 0
+  // the size of the last block handed over; the array for the next block is 
allocated to hold it
+  private var lastCount: Int = 0
+
+  /** The number of bytes written since the last hand-over. */
+  def size: Int = count
+
+  override def write(b: Int): Unit = {
+    ensureCapacity(1)
+    buf(count) = b.toByte
+    count += 1
+  }
+
+  override def write(bytes: Array[Byte], offset: Int, length: Int): Unit = {
+    ensureCapacity(length)
+    System.arraycopy(bytes, offset, buf, count, length)
+    count += length
+  }
+
+  /**
+   * Reserves `length` bytes at the end of the buffered data for the caller to 
fill in directly.
+   *
+   * @return the position in [[array]] at which the reserved bytes start
+   */
+  def reserve(length: Int): Int = {
+    ensureCapacity(length)
+    val position = count
+    count += length
+    position
+  }
+
+  /**
+   * The array backing the buffered data. Only valid until the next write, 
reserve or hand-over, any of
+   * which may replace it.
+   */
+  def array: Array[Byte] = buf
+
+  /**
+   * Hands the buffered data over as a `ByteString` and leaves the stream 
empty for the next block.
+   *
+   * When most of the buffer is used the `ByteString` wraps the array without 
copying and the stream
+   * starts a fresh array for the next block; when only a small part is used 
the data is copied so the
+   * `ByteString` does not retain a large, mostly unused array, and the stream 
keeps its array.
+   */
+  def takeByteString(): ByteString =
+    if (count < 1) ByteString.empty
+    else {
+      val result =
+        if (count > (buf.length >> 1)) {
+          val wrapped = ByteString.fromArrayUnsafe(buf, 0, count)
+          lastCount = count
+          buf = Array.emptyByteArray
+          wrapped
+        } else ByteString.fromArray(buf, 0, count)
+      count = 0
+      result
+    }
+
+  private def ensureCapacity(additional: Int): Unit = {
+    val required = count + additional
+    if (required > buf.length) {
+      val capacity =
+        if (buf.length == 0) math.max(math.max(initialCapacity, lastCount), 
required)
+        else math.max(buf.length << 1, required)
+      buf = Arrays.copyOf(buf, capacity)
+    }
+  }
+}
diff --git 
a/runtime/src/main/scala/org/apache/pekko/grpc/internal/ByteStringUtils.scala 
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/ByteStringUtils.scala
index f970c862..7ffcf2ec 100644
--- 
a/runtime/src/main/scala/org/apache/pekko/grpc/internal/ByteStringUtils.scala
+++ 
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/ByteStringUtils.scala
@@ -18,7 +18,7 @@ import pekko.annotation.InternalApi
 import pekko.util.ByteString
 import io.grpc.KnownLength
 
-import java.io.{ ByteArrayOutputStream, InputStream }
+import java.io.InputStream
 
 @InternalApi
 private[grpc] object ByteStringUtils {
@@ -34,26 +34,30 @@ private[grpc] object ByteStringUtils {
     val initialBytes = stream.read(buffer, 0, buffer.length)
     val nextByte = if (initialBytes < 0) -1 else stream.read() // Test for EOF
 
-    if (nextByte == -1) {
-      if (initialBytes < 1) ByteString.empty // EOF immediately
-      else if (initialBytes > (buffer.length >> 1))
-        // Most of the buffer is used — reuse it to avoid a copy
-        ByteString.fromArrayUnsafe(buffer, 0, initialBytes)
-      else
-        // Small read from a large buffer — copy to right-size so the rest can 
be GC'd
-        ByteString.fromArray(buffer, 0, initialBytes)
-    } else {
-      val baos = new ByteArrayOutputStream(buffer.length * 2) // To avoid 
immediate resize
-      baos.write(buffer, 0, initialBytes)
-      baos.write(nextByte)
+    if (nextByte == -1)
+      toByteStringUnsafe(buffer, initialBytes)
+    else {
+      val bsos = new ByteStringOutputStream(buffer.length * 2) // To avoid 
immediate resize
+      bsos.write(buffer, 0, initialBytes)
+      bsos.write(nextByte)
 
       var bytesRead = stream.read(buffer)
       while (bytesRead >= 0) {
-        baos.write(buffer, 0, bytesRead)
+        bsos.write(buffer, 0, bytesRead)
         bytesRead = stream.read(buffer)
       }
 
-      ByteString.fromArrayUnsafe(baos.toByteArray)
+      bsos.takeByteString()
     }
   }
+
+  def toByteStringUnsafe(buf: Array[Byte], count: Int): ByteString =
+    if (count < 1)
+      ByteString.empty
+    else if (count > (buf.length >> 1))
+      // Most of the buffer is used — reuse it to avoid a copy
+      ByteString.fromArrayUnsafe(buf, 0, count)
+    else
+      // Small read from a large buffer — copy to right-size so the rest can 
be GC'd
+      ByteString.fromArray(buf, 0, count)
 }
diff --git a/runtime/src/main/scala/org/apache/pekko/grpc/internal/Gzip.scala 
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/Gzip.scala
index 21d3687b..643b986b 100644
--- a/runtime/src/main/scala/org/apache/pekko/grpc/internal/Gzip.scala
+++ b/runtime/src/main/scala/org/apache/pekko/grpc/internal/Gzip.scala
@@ -13,7 +13,6 @@
 
 package org.apache.pekko.grpc.internal
 
-import java.io.ByteArrayOutputStream
 import java.util.zip.{ GZIPInputStream, GZIPOutputStream }
 
 import org.apache.pekko.util.ByteString
@@ -25,28 +24,15 @@ object Gzip extends Codec {
   override val name: String = "gzip"
 
   override def compress(uncompressed: ByteString): ByteString = {
-    val baos = new ByteArrayOutputStream(uncompressed.size)
-    val gzos = new GZIPOutputStream(baos)
+    val bsos = new ByteStringOutputStream(uncompressed.size)
+    val gzos = new GZIPOutputStream(bsos)
     try gzos.write(uncompressed.toArrayUnsafe())
     finally gzos.close()
-    ByteString.fromArrayUnsafe(baos.toByteArray)
+    bsos.takeByteString()
   }
 
   @deprecated("Use uncompress(bytes, maxDecompressedSize), which bounds the 
decompressed size", "2.0.0")
-  override def uncompress(compressed: ByteString): ByteString = {
-    val gzis = new GZIPInputStream(compressed.asInputStream)
-
-    val baos = new ByteArrayOutputStream(compressed.size)
-    val buffer = new Array[Byte](32 * 1024)
-    try {
-      var read = gzis.read(buffer)
-      while (read != -1) {
-        baos.write(buffer, 0, read)
-        read = gzis.read(buffer)
-      }
-    } finally gzis.close()
-    ByteString.fromArrayUnsafe(baos.toByteArray)
-  }
+  override def uncompress(compressed: ByteString): ByteString = 
uncompress(compressed, Int.MaxValue)
 
   /**
    * Decompress with a maximum decompressed size limit.
@@ -57,10 +43,10 @@ object Gzip extends Codec {
     val limit = maxDecompressedSize.toLong
     // clamp: maxDecompressedSize is validated to be positive by the settings 
classes, but this
     // method is also reachable with a hand-constructed limit, and a negative 
initial size would
-    // make ByteArrayOutputStream throw IllegalArgumentException rather than a 
gRPC status.
+    // make ByteStringOutputStream throw IllegalArgumentException rather than 
a gRPC status.
     val initialSize = Math.max(0L, Math.min(compressed.size.toLong, 
limit)).toInt
     val gzis = new GZIPInputStream(compressed.asInputStream)
-    val baos = new ByteArrayOutputStream(initialSize)
+    val bsos = new ByteStringOutputStream(initialSize)
     val buffer = new Array[Byte](32 * 1024)
     // Long, so that a limit close to Int.MaxValue cannot be passed by an 
overflowing counter
     var totalBytes = 0L
@@ -72,11 +58,11 @@ object Gzip extends Codec {
           throw new StatusException(
             Status.RESOURCE_EXHAUSTED.withDescription(
               s"Decompressed message size exceeds maximum allowed 
$maxDecompressedSize bytes"))
-        baos.write(buffer, 0, read)
+        bsos.write(buffer, 0, read)
         read = gzis.read(buffer)
       }
     } finally gzis.close()
-    ByteString.fromArrayUnsafe(baos.toByteArray)
+    bsos.takeByteString()
   }
 
   @deprecated(
diff --git 
a/runtime/src/test/scala/org/apache/pekko/grpc/internal/ByteStringOutputStreamSpec.scala
 
b/runtime/src/test/scala/org/apache/pekko/grpc/internal/ByteStringOutputStreamSpec.scala
new file mode 100644
index 00000000..6ce3e126
--- /dev/null
+++ 
b/runtime/src/test/scala/org/apache/pekko/grpc/internal/ByteStringOutputStreamSpec.scala
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.grpc.internal
+
+import org.apache.pekko.util.ByteString
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+class ByteStringOutputStreamSpec extends AnyWordSpec with Matchers {
+
+  "ByteStringOutputStream" must {
+
+    "return an empty ByteString when nothing was written" in {
+      new ByteStringOutputStream(16).takeByteString() should 
===(ByteString.empty)
+    }
+
+    "return the bytes written when the buffer is exactly filled" in {
+      val out = new ByteStringOutputStream(4)
+      out.write(Array[Byte](1, 2, 3, 4))
+      out.takeByteString() should ===(ByteString(1, 2, 3, 4))
+    }
+
+    "return the bytes written when the buffer was grown" in {
+      val out = new ByteStringOutputStream(2)
+      val data = Array.tabulate[Byte](1000)(i => i.toByte)
+      out.write(data)
+      out.takeByteString() should ===(ByteString(data))
+    }
+
+    "return the bytes written when only a small part of the buffer is used" in 
{
+      val out = new ByteStringOutputStream(1024)
+      out.write(Array[Byte](1, 2, 3))
+      out.write(4)
+      out.takeByteString() should ===(ByteString(1, 2, 3, 4))
+    }
+
+    "not retain the buffer when only a small part of it is used" in {
+      val out = new ByteStringOutputStream(1024)
+      out.write(Array[Byte](1, 2, 3))
+      val array = out.array
+      // the ByteString is a copy, so it is not affected by later writes to 
the stream, which keeps its array
+      val result = out.takeByteString()
+      out.write(Array[Byte](9, 9, 9))
+      result should ===(ByteString(1, 2, 3))
+      (out.array eq array) shouldBe true
+    }
+
+    "hand over a mostly used buffer without copying and start the next block 
on a fresh array" in {
+      val out = new ByteStringOutputStream(4)
+      out.write(Array[Byte](1, 2, 3))
+      val array = out.array
+      val first = out.takeByteString()
+      first should ===(ByteString(1, 2, 3))
+      out.size shouldBe 0
+
+      // writes to the next block must not show up in the ByteString that was 
handed out
+      out.write(Array[Byte](9, 9, 9, 9), 0, 4)
+      (out.array eq array) shouldBe false
+      first should ===(ByteString(1, 2, 3))
+      out.takeByteString() should ===(ByteString(9, 9, 9, 9))
+    }
+
+    "write single bytes and byte ranges" in {
+      val out = new ByteStringOutputStream(8)
+      out.write(1)
+      out.write(Array[Byte](0, 2, 3, 0), 1, 2)
+      out.write(Array[Byte](4, 5, 6, 7, 8))
+      out.takeByteString() should ===(ByteString(1, 2, 3, 4, 5, 6, 7, 8))
+    }
+
+    "reserve a range that the caller fills in through the backing array" in {
+      val out = new ByteStringOutputStream(2)
+      out.write(7)
+      val position = out.reserve(3)
+      position shouldBe 1
+      out.size shouldBe 4
+      val array = out.array
+      array(position) = 1
+      array(position + 1) = 2
+      array(position + 2) = 3
+      out.write(8)
+      out.takeByteString() should ===(ByteString(7, 1, 2, 3, 8))
+    }
+
+    "reject a negative initial capacity" in {
+      an[IllegalArgumentException] should be thrownBy new 
ByteStringOutputStream(-1)
+    }
+  }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to