Skip to content

perf: optimise UnsynchronizedByteArrayInputStream bulk operations - #3541

Open
pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:optimise-unsync-bais
Open

pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:optimise-unsync-bais

Conversation

@pjfanning

Copy link
Copy Markdown
Member

Motivation

UnsynchronizedByteArrayInputStream (used by ByteString.asInputStream and the Java/Jackson serializers) only overrode the basic read/skip methods. readAllBytes, readNBytes, skipNBytes and transferTo fell through to the generic InputStream defaults, which allocate 16 KiB scratch buffers in a loop and then concatenate — wasteful when the whole payload already sits in a byte[]. skip also advanced offset unconditionally via Math.addExact(offset, Math.toIntExact(n)), so a large n could throw ArithmeticException and offset could end up past eod.

Modification

  • Override readAllBytes, readNBytes(int), readNBytes(byte[],int,int), skipNBytes and transferTo to operate directly on the backing array (one Arrays.copyOfRange / OutputStream.write each) — the same set of overrides java.io.ByteArrayInputStream has.
  • Clamp skip to the remaining bytes so offset <= eod always holds and large skips cannot overflow; simplify available() and readLocal() accordingly.
  • Simplify the (data, offset, length) constructor so offset/eod are clamped to the array using long arithmetic (no int overflow), and use Objects.checkFromIndexSize for the read(byte[],int,int) bounds check.
  • Expand UnsynchronizedByteArrayInputStreamSpec to cover every method and edge case (EOF, zero-length reads, clamping, overflow, negative args).
  • Add readAllBytes and transferTo cases to ByteString_asInputStream_Benchmark.

Result

readAllBytes/readNBytes/transferTo on ByteString.asInputStream copy the data once instead of chunking through temporary buffers. Observable behaviour is otherwise unchanged (negative skip still throws, as before).

Local JMH (-f 1 -wi 2 -i 3, short run so error bars are wide):

Benchmark kb before (ops/s) after (ops/s)
single_bs_as_input_stream_read_all_bytes 10 201,138 502,768
single_bs_as_input_stream_read_all_bytes 1000 3,307 5,460
single_bs_as_input_stream_transfer_to 10 238,211 267,642
single_bs_as_input_stream_transfer_to 1000 2,883 2,747

transferTo is dominated by the ByteArrayOutputStream copy on the receiving side, so the difference there is within noise.

Tests

  • sbt "actor-tests/testOnly org.apache.pekko.util.UnsynchronizedByteArrayInputStreamSpec org.apache.pekko.util.ByteStringSpec" — 226 passed
  • sbt "actor/mimaReportBinaryIssues" — no issues (class is @InternalApi; only methods added)
  • sbt "bench-jmh/Jmh/compile" and Jmh/run of the new benchmarks
  • scalafmt on changed Scala files, sbt actor/javafmtAll on JDK 17
  • git diff --check

References

None - follow-up to #2300 which introduced this class

Motivation:
UnsynchronizedByteArrayInputStream only overrode the basic read/skip
methods, so readAllBytes, readNBytes, skipNBytes and transferTo fell
through to the generic InputStream defaults. Those allocate 16 KiB
scratch buffers in a loop and then concatenate, which is wasteful when
the whole payload already sits in a byte[]. skip also advanced the
offset unconditionally with Math.addExact/toIntExact, so it could
throw ArithmeticException for large values and leave offset past eod.

Modification:
- Override readAllBytes, readNBytes(int), readNBytes(byte[],int,int),
  skipNBytes and transferTo to operate directly on the backing array
  (one Arrays.copyOfRange / OutputStream.write each).
- Clamp skip to the remaining bytes so offset never exceeds eod and
  large skips cannot overflow; simplify available() and readLocal()
  accordingly.
- Simplify the (data, offset, length) constructor so offset/eod are
  clamped to the array with long arithmetic (no int overflow), and use
  Objects.checkFromIndexSize for the read(byte[],int,int) bounds check.
- Expand UnsynchronizedByteArrayInputStreamSpec to cover every method
  and edge case (EOF, zero-length reads, clamping, overflow).
- Add readAllBytes and transferTo cases to
  ByteString_asInputStream_Benchmark.

Result:
readAllBytes/readNBytes/transferTo on ByteString.asInputStream copy the
data once instead of chunking through temporary buffers. Local JMH
(-f 1 -wi 2 -i 3) for single_bs_as_input_stream_read_all_bytes:
10 KB 201k -> 503k ops/s, 1000 KB 3.3k -> 5.5k ops/s.

Tests:
- sbt "actor-tests/testOnly org.apache.pekko.util.UnsynchronizedByteArrayInputStreamSpec org.apache.pekko.util.ByteStringSpec" (226 passed)
- sbt "actor/mimaReportBinaryIssues" (no issues)
- sbt "bench-jmh/Jmh/compile" and Jmh/run of the new benchmarks
- scalafmt on changed Scala files, sbt actor/javafmtAll (JDK 17)
- git diff --check

References:
None - follow-up to apache#2300 which introduced this class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant