pjfanning opened a new pull request, #3533: URL: https://github.com/apache/pekko/pull/3533
### Motivation `ByteString.apply(idx)` bounds-checks before delegating to `byteAtUnchecked`, and for the `ByteStrings` composite it also resolves the absolute offset to a fragment. Two call sites under `src/main` pay that check when the caller has already established the bound: - `ByteStringParser.ByteReader.readByte` guards with `off < input.length` and then calls `input(off)`, so the bound is tested twice. Its siblings `readShortLE` / `readIntLE` / `readLongLE` already use the `*Unchecked` variants after their own guard, so `readByte` was the odd one out. `ByteReader` backs gzip/deflate header parsing, TLS and framing. - `TcpDnsClient.decodeLength` read the two length-prefix bytes with two `apply` calls: two bounds checks, and for a composite `ByteString` two fragment lookups. These are the only two places under `src/main` that index a `ByteString` outside `ByteString.scala` itself and `bench-jmh`. ### Modification `readByte` now calls `input.byteAtUnchecked(off)`. `off` starts at 0 and only ever moves forward, so the existing guard is sufficient. `decodeLength` now uses `data.readShortBE(0) & 0xFFFF`. `readShortBE` keeps the bounds check, so short input still throws `IndexOutOfBoundsException` as before, and it is SWAR-optimised for the single-array `ByteString1C` / `ByteString1` that the common single-chunk read path produces. Both are internal (`private[pekko]` / `private[internal]`); no public API or binary shape change. ### Result One bounds check per byte instead of two in the stream parser, and one checked 16-bit read instead of two indexed reads for the DNS length prefix. Behaviour is unchanged. Worth noting for reviewers: `ByteStrings.apply` is already cheap for sequential access thanks to the `fragmentHint` memoisation, so this is a small constant-factor cleanup rather than a complexity fix. I did not switch `ByteReader` to an iterator — it needs `off` as a random-access cursor for `take`, `slice` and `fromStartToHere`. ### Tests - `sbt "stream-tests/testOnly org.apache.pekko.stream.io.ByteStringParserSpec org.apache.pekko.stream.scaladsl.CompressionSpec org.apache.pekko.stream.io.compression.*"` — 106 succeeded, 0 failed - `sbt "actor-tests/testOnly org.apache.pekko.io.dns.internal.TcpDnsClientSpec"` — 8 succeeded, 0 failed - `sbt "actor/mimaReportBinaryIssues" "stream/mimaReportBinaryIssues"` — success - Native `scalafmt` run on the two changed files. - No new tests: both changes are behaviour-preserving refactors of internal code, covered by the existing specs above. - Not benchmarked. The change is a strict reduction in work per read, not an algorithmic change — say the word if you want JMH numbers before merging. ### References None - found by reviewing `ByteString` indexed access under `src/main` -- 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]
