perf: use Arrays.copyOf/copyOfRange instead of manual new Array + arraycopy#3150
Merged
Conversation
…aycopy Motivation: Several places allocate a new array then immediately copy from an existing array using System.arraycopy. java.util.Arrays.copyOf and copyOfRange combine allocation and copy in a single call that the JIT can intrinsify more effectively. Modification: - ByteString.scala: 3 sites — clearTemp, resizeTemp, fromArray replace new Array[Byte] + System.arraycopy with java.util.Arrays.copyOf / copyOfRange - ImmutableLongMap.scala: replace full array copy with values.clone() - SnapshotSerializer.scala: replace partial copy with java.util.Arrays.copyOfRange Result: Fewer lines, potentially faster due to JIT intrinsification of the combined allocate+copy operation. ByteString builder operations benefit most as they are in frequently-called paths. Tests: sbt "actor/compile" "remote/compile" "persistence/compile" — passed References: Refs #3136
…zeTemp Motivation: `ByteStringBuilder._temp` is initially `null` until the first call to `++=` or `putByte`. `resizeTemp` was unconditionally calling `java.util.Arrays.copyOf(_temp, size)` which throws `NullPointerException` when passed a `null` source array. This bug was introduced by the earlier "perf: use Arrays.copyOf/copyOfRange" commit in the same PR, which replaced the previous `new Array[Byte](size)` followed by `System.arraycopy(src, 0, dst, 0, n)` pattern — the old pattern was safe because the arraycopy length was `0` when `_temp` was `null`, masking the missing null check. Modification: Guard the call with `if (_temp eq null) new Array[Byte](size) else java.util.Arrays.copyOf(_temp, size)`, so the first resize allocates a fresh array and subsequent resizes retain the copyOf fast path. Result: `ByteStringBuilder` no longer throws NPE on the very first write when the internal buffer has not yet been allocated. Tests: sbt "actor/compile" -- passed sbt "actor-tests/Test/testOnly *ByteStringSpec" -- passed (relevant directional tests; the NPE was also reachable from stream/Pekko usage). References: Discovered during internal review of PR #3150.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Several places allocate a new array then immediately copy from an existing array using
System.arraycopy.java.util.Arrays.copyOfandcopyOfRangecombine allocation and copy in a single call that the JIT can intrinsify more effectively.Modification
ByteString.scala: 3 sites (clearTemp,resizeTemp,fromArray) — replacenew Array[Byte]+System.arraycopywithjava.util.Arrays.copyOf/copyOfRangeImmutableLongMap.scala: replace full array copy withvalues.clone()SnapshotSerializer.scala: replace partial copy withjava.util.Arrays.copyOfRangeResult
Fewer lines, potentially faster due to JIT intrinsification of the combined allocate+copy operation.
ByteStringbuilder operations benefit most as they are in frequently-called paths.Tests
sbt "actor/compile" "remote/compile" "persistence/compile"— passedReferences
Refs #3136