pjfanning opened a new pull request, #1228: URL: https://github.com/apache/pekko-http/pull/1228
### Motivation Three places in the gzip/deflate stages copy a `ByteString` into a `byte[]` purely to hand it to `java.util.zip`: - `DeflateDecompressorBase.Inflate.parse` calls `inflater.setInput(reader.remainingData.toArray)` on **every** parse round with the whole remaining buffer, so every round copies it. - `GzipCompressor.updateCrc` and `GzipDecompressor.crc16` use `toArrayUnsafe()`. `toArrayUnsafe()` only avoids the copy for a *compact* `ByteString`; for a slice or a multi-fragment one it falls back to `toArray`. On pekko-actor 2.0.0-M4, `compact.toArrayUnsafe() eq compact.toArrayUnsafe()` is `true`, but `false` for `compact.drop(1)` and for a rope. And `ByteReader.remainingData` is `input.drop(off)`, so it is never compact once any byte has been consumed — which is always the case by the time `Inflate` runs, since the wrap-probe / gzip-header steps have already consumed bytes. `Inflater`/`Deflater` have accepted `ByteBuffer` input since Java 11 and `CRC32` since Java 8; the build targets JDK 17 (`javacTarget` in `project/Common.scala`). ### Modification - `inflater.setInput(reader.remainingData.asByteBuffer)`. `getRemaining` reports the buffer's remaining bytes when input was supplied as a `ByteBuffer`, so the following `reader.skip(reader.remainingSize - inflater.getRemaining)` needs no change. For a multi-fragment input, `asByteBuffer` compacts internally — one copy, i.e. no worse than today. - Feed both checksums one buffer per fragment via `asByteBuffers`. Plural, not singular: `CRC32` is incremental, so per-fragment updates stay copy-free even for a rope, where `asByteBuffer` would compact it first. `ByteString.asByteBuffer` returns a read-only heap buffer (`ByteBuffer.wrap(...).asReadOnlyBuffer()`, so neither `hasArray` nor `isDirect`). I verified on JDK 21 that `Inflater.setInput` accepts it — a 1 MB round trip inflated correctly with `getRemaining == 0` — and that `CRC32.update` agrees with the array path. All three classes are `@InternalApi` / `private[coding]`. ### Result No array copy per inflate round, and no full-size copy per gzipped entity. CRC32 over a 1 MB input, 200 updates: | input | via `toArrayUnsafe()` | via `ByteBuffer` | |---|---|---| | sliced | 221 ms | 154 ms | | compact | 102 ms | 98 ms | Never slower, ~30% faster when the input is not compact, and it stops allocating a full-size copy of everything being gzipped. Note the read-only heap buffer sends `CRC32.update` down the JDK's internal 4 KB chunked-copy path rather than a straight array pass, which is why the win is ~30% and not total. `crc16` only ever sees the gzip header (tens of bytes); that one is for consistency, not speed. ### Tests - `sbt "http-tests / Test / testOnly org.apache.pekko.http.scaladsl.coding.*"` - 78 passed. `CoderSpec` already covers the changed paths thoroughly: *"works for any split in prefix + suffix"* decodes at every possible split of the compressed stream, so `remainingData` is a slice at every offset; plus *"decompress in very small chunks"*, *"be able to decode chunk-by-chunk"*, corrupt-input handling and the round-trips. - New `CoderSpec` case: encode a deliberately multi-fragment input (asserted `isCompact == false`) and check it produces byte-identical output to the compact form, pinning the per-fragment checksum as fragmentation-independent. It is a regression guard rather than a failing-before test, since the previous code was also correct. Writing it turned up a nice subtlety worth knowing: `bs.grouped(n).reduce(_ ++ _)` gives you back a *compact* ByteString, because adjacent slices of one array are merged by `++`; the test copies each chunk into its own array to get a genuine rope. - `sbt http/mimaReportBinaryIssues` - clean. - `scalafmt --mode diff-ref=upstream/main` - clean. ### References None - found while auditing ByteString usage across the code base -- 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]
