RFR: 8310843: Reimplement ByteArray and ByteArrayLittleEndian with Unsafe [v10]

[Maurizio Cimadamore]

On Thu, 20 Jul 2023 09:33:15 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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.
>
> Actually, a byte buffer is big endian, so some extra code would be required. But maybe that's another helper function:
> 
> 
> @ForceInline
> ByteBuffer asBuffer(byte[] array) { return ByteBuffer.wrap(array).order(ByteOrder.nativeOrder()); }
> 
> 
> And then replace:
> 
> 
> ByteArray.getChar(array, 42)
> 
> With
> 
> asBuffer(array).getChar(42);

Also... in a lot of cases where ByteArray is used (DataXYZStream, ObjectXYZStream) the array being used is a field in the class. So the byte buffer creation can definitively be amortized (or the code changed to work on buffers instead of arrays).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14636#discussion_r1269210990