pjfanning opened a new pull request, #3464: URL: https://github.com/apache/pekko/pull/3464
Adds two `ByteString` APIs for callers that repeatedly read sub-ranges of a larger buffer. Both come from an analysis of the pekko-http parsers, where the slice-then-use pattern is common in header and line parsing. ## `copyToArray(srcOffset, dest, destOffset, len)` The existing `copyToArray(dest, destOffset, len)` can only copy from index 0, so copying from an offset means `drop(n)` or `slice(...)` first — which allocates a new ByteString, and for a fragmented one a new fragment vector too. ## `decodeString(charset, from, until)` Decodes a sub-range directly. The array-backed layouts pass the offset and length straight to `new String(bytes, offset, len, charset)`, so neither an intermediate ByteString nor an intermediate array is created. ## Implementation Both are defined on `ByteString` with a correct generic fallback and overridden in `ByteString1C`, `ByteString1`, `ByteString2` and `ByteStrings`. The single-array layouts use `System.arraycopy` and the offset-taking `String` constructor; `ByteStrings` locates the starting fragment and copies whole runs out of consecutive fragments. Out of range arguments are **clamped rather than throwing**, matching the existing `copyToArray` overloads: the copy is reduced to what is available in the source and what fits in the destination, and an empty or inverted range decodes to `""`. This is a deliberate choice for consistency — worth a reviewer's opinion if you would rather they threw. ## Measurements Allocation is the reliable signal here and is the point of both APIs. Copying 4096 bytes from offset 2048 of an 8192-byte ByteString: | layout | `drop(n).copyToArray` | `copyToArray(srcOffset, …)` | |---|---|---| | 128 × 64B fragments | 202 B/op | **0 B/op** | | 16 × 512B fragments | 144 B/op | **0 B/op** | Decoding a 45-byte header out of a fragmented buffer: `slice(…).decodeString` 328 B/op → `decodeString(cs, from, until)` 152 B/op. **One case where the new API is slower**, which I want to flag rather than bury: with very small fragments (1024 × 8B), `copyToArray(srcOffset, …)` runs slower than `drop(n).copyToArray` (~15µs vs ~11µs) even though it allocates nothing. The reason is that `drop` produces a ByteString whose existing `copyToArray` walks `bytestrings.iterator` once, whereas the new implementation indexes `bytestrings(fragIdx)` per fragment, and `Vector.apply` is O(log n). I tried an iterator-based walk; it fixed the timing but reintroduced allocation, which defeats the purpose, so I kept the allocation-free version. 8-byte fragments are also not a realistic shape for the parser use case this targets. Happy to revisit if a reviewer disagrees with that trade. Timings on this machine were noisy run to run; the allocation figures are stable and reproducible, so I would treat the timing numbers as indicative only. ## Relationship to the other ByteString PRs Independent of #3463. I stacked this branch on top of it to check: the two merge cleanly, all tests pass, and the small-fragment gap above persists, because `copyToArray` copies via `Array.copy` per fragment and never goes through the memoised `byteAtUnchecked` that #3463 adds. The one place they do interact is the generic `copyToArray` fallback on the abstract class, which reads `byteAtUnchecked` per byte — that path gets faster if #3463 lands, but no built-in layout uses it. Neither PR needs the other, and they can merge in either order. ## Tests 8 new tests in `ByteStringSpec`: - both APIs compared against the `slice(...)`-based expression they replace, across compacted, sliced, two-fragment and multi-fragment representations, over all combinations of in and out of range offsets and lengths - clamping of every out of range argument - not writing past the end of the destination - empty ByteString and empty/inverted ranges - a multi-byte UTF-8 character split across two fragments `actor-tests/testOnly org.apache.pekko.util.ByteStringSpec` passes (221 tests). `sbt actor/mimaReportBinaryIssues` is clean — both APIs are additive. `scalafmt` run on both modules. I did not add a JMH benchmark here, since the win is allocation rather than throughput and the existing ByteString benchmarks are throughput oriented. Happy to add one if preferred. -- 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]
