RFR: 8310843: Reimplement ByteArray and ByteArrayLittleEndian with Unsafe [v10]
Maurizio Cimadamore
mcimadamore at openjdk.org
Thu Jul 20 09:28:46 UTC 2023
On Thu, 13 Jul 2023 23:05:48 GMT, Glavo <duke at openjdk.org> wrote:
>> `ByteArray` and `ByteArrayLittleEndian` are very useful tool classes that can be used in many places to performance tuning.
>>
>> Currently they are implemented by `VarHandle`, so using them may have some impact on startup time.
>>
>> This PR reimplements them using `Unsafe`, which reduces the impact on startup time.
>
> Glavo has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
>
> - Merge branch 'openjdk:master' into unsafe
> - add 8310843 to @bug
> - Merge branch 'openjdk:master' into unsafe
> - Merge branch 'openjdk:master' into unsafe
> - delete incorrect comments
> - delete extraneous whitespace
> - add javadoc
> - delete extraneous whitespace
> - fix test
> - update tests
> - ... and 5 more: https://git.openjdk.org/jdk/compare/7d6c3848...cb56e736
src/java.base/share/classes/jdk/internal/util/ByteArray.java line 54:
> 52:
> 53: @ForceInline
> 54: static long arrayOffset(byte[] array, int typeBytes, int offset) {
IMHO, this is the really interesting thing that this class does - e.g. it introduces a way to translate a (logical) offset into a byte array into a physical offset that can be used for unsafe. After you have an helper method like this, it seems like the client can just do what it wants by using Unsafe directly (which would remove the need for having this class) ? Was some experiment of that kind done (e.g. replacing usage of ByteArray with Unsafe + helpers) - or does it lead to code that is too cumbersome on the client?
Also, were ByteBuffers considered as an alternative? (I'm not suggesting MemorySegment as those depend on VarHandle again, but a heap ByteBuffer is just a thin wrapper around an array which uses Unsafe). ByteBuffer will have a bound check, but so does your code (which call checkIndex). I believe that, at least in hot code, wrapping a ByteBuffer around a byte array should be routinely scalarized, as there's no control flow inside these little methods.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14636#discussion_r1269195526
More information about the core-libs-dev
mailing list