pjfanning opened a new pull request, #3471: URL: https://github.com/apache/pekko/pull/3471
### Motivation `ByteStringBuilder` accumulates writes from `putByte`/`putBytes`/`putInt`/`asOutputStream` into a temporary array and flushes it in `clearTemp`, which always does `java.util.Arrays.copyOf(_temp, _tempLength)`. On the final flush performed by `result()` that copy is avoidable: the buffer is not needed for further writes at that point, so every builder result pays a full copy of its tail chunk for nothing. ### Modification Add a private `clearTempForResult` used only by `result()`: - when more than half of the temp buffer holds data, the buffer is handed over to the resulting `ByteString1` instead of being copied, and `_temp`/`_tempCapacity` are reset so that a later write to the builder allocates a fresh buffer rather than mutating the bytes that were handed over (every `_temp` write in the class goes through `ensureTempSize` first, and `shouldResizeTempFor` returns true on `_tempCapacity == 0`); - when only a small part of the buffer is used it is still copied, so a small result does not retain a much larger array. The mid-stream `clearTemp` calls made by `addAll` are unchanged, since the buffer is still reused there. ### Result No copy of the tail chunk on `result()` for builders that are at least half full. A result produced by the hand-over path can be a non-compact `ByteString1`. That is copy-neutral: a consumer calling `toArrayUnsafe` on it pays exactly the copy that used to happen inside `clearTemp`, and it is avoided entirely for consumers that do not need an array. `ByteStringUtils.toProtoByteStringUnsafe` still takes its zero-copy `asByteBuffers` path for such a value. ### Tests - `sbt "actor-tests/testOnly org.apache.pekko.util.ByteStringSpec"` - 218 tests succeeded, 0 failed - `ByteStringSpec` gains cases for the mostly-used, exactly-filled and small-write-in-a-large-buffer flush paths, that a returned `ByteString` is unaffected by later writes to the builder, and that the content is correct for every fill level from 0 to 200 bytes - `scalafmt --list --mode diff-ref=upstream/main` - no files reported - `git diff --check` - clean - A broader run of `actor-tests/testOnly org.apache.pekko.util.*`, the stream io/compression/framing suites and `actor/mimaReportBinaryIssues` was started but stopped before completing, so it is not recorded as a pass. MiMa is expected to be unaffected: the change adds one private method and alters no signature. ### References None - follows the same no-copy approach as apache/pekko-http#1235 -- 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]
