On Thu, 20 Jul 2023 14:49:48 GMT, Glavo <d...@openjdk.org> wrote:
>> Also, note that ByteBuffer exposes its backing array (at least if the buffer
>> is not read only) via ByteBuffer::array. This does no copy. So in all the
>> various stream implementations, I believe we can really just use a
>> ByteBuffer instead of an array - and use ByteBuffer::array when we really
>> need an array.
>>
>> This makes it very easy to migrate some of the classes that use ByteArray.
>> Here's the patch for DataInputStream:
>>
>>
>> diff --git a/src/java.base/share/classes/java/io/DataInputStream.java
>> b/src/java.base/share/classes/java/io/DataInputStream.java
>> index 7192b30d5f2..b5b013cdd50 100644
>> --- a/src/java.base/share/classes/java/io/DataInputStream.java
>> +++ b/src/java.base/share/classes/java/io/DataInputStream.java
>> @@ -27,6 +27,8 @@ package java.io;
>>
>> import jdk.internal.util.ByteArray;
>>
>> +import java.nio.ByteBuffer;
>> +import java.nio.ByteOrder;
>> import java.util.Objects;
>>
>> /**
>> @@ -59,7 +61,7 @@ public class DataInputStream extends FilterInputStream
>> implements DataInput {
>> super(in);
>> }
>>
>> - private final byte[] readBuffer = new byte[8];
>> + private final ByteBuffer readBuffer = ByteBuffer.allocate(8); //
>> BIG_ENDIAN
>>
>> /**
>> * working arrays initialized on demand by readUTF
>> @@ -316,8 +318,8 @@ public class DataInputStream extends FilterInputStream
>> implements DataInput {
>> * @see java.io.FilterInputStream#in
>> */
>> public final short readShort() throws IOException {
>> - readFully(readBuffer, 0, 2);
>> - return ByteArray.getShort(readBuffer, 0);
>> + readFully(readBuffer.array(), 0, 2);
>> + return readBuffer.getShort(0);
>> }
>>
>> /**
>> @@ -338,8 +340,8 @@ public class DataInputStream extends FilterInputStream
>> implements DataInput {
>> * @see java.io.FilterInputStream#in
>> */
>> public final int readUnsignedShort() throws IOException {
>> - readFully(readBuffer, 0, 2);
>> - return ByteArray.getUnsignedShort(readBuffer, 0);
>> + readFully(readBuffer.array(), 0, 2);
>> + return Short.toUnsignedInt(readBuffer.getShort(0));
>> }
>>
>> /**
>> @@ -360,8 +362,8 @@ public class DataInputStream extends FilterInputStream
>> implements DataInput {
>> * @see java.io.FilterInputStream#in
>> */
>> public final char readChar() throws IOException {
>> - readFully(readBuffer, 0, 2);
>> - return ByteArray.getChar(readBuffer, 0);
>> + readFully(readBuffer.arra...
>
> @mcimadamore I compared the performance of `ByteBuffer` and `VarHandle` using
> a JMH benchmark:
>
>
> public class ByteArray {
>
> private byte[] array;
> private ByteBuffer byteBuffer;
>
> private static final VarHandle INT =
> MethodHandles.byteArrayViewVarHandle(int[].class, LITTLE_ENDIAN);
> private static final VarHandle LONG =
> MethodHandles.byteArrayViewVarHandle(long[].class, LITTLE_ENDIAN);
>
> @Setup
> public void setup() {
> array = new byte[8];
> byteBuffer = ByteBuffer.wrap(array).order(LITTLE_ENDIAN);
>
> new Random(0).nextBytes(array);
> }
>
> @Benchmark
> public byte readByte() {
> return array[0];
> }
>
> @Benchmark
> public byte readByteFromBuffer() {
> return byteBuffer.get(0);
> }
>
> @Benchmark
> public int readInt() {
> return (int) INT.get(array, 0);
> }
>
> @Benchmark
> public int readIntFromBuffer() {
> return byteBuffer.getInt(0);
> }
>
>
> @Benchmark
> public long readLong() {
> return (long) LONG.get(array, 0);
> }
>
> @Benchmark
> public long readLongFromBuffer() {
> return byteBuffer.getLong(0);
> }
> }
>
>
> Result:
>
> Benchmark Mode Cnt Score Error Units
> ByteArray.readByte thrpt 5 1270230.180 ± 29172.551 ops/ms
> ByteArray.readByteFromBuffer thrpt 5 623862.080 ± 12167.410 ops/ms
> ByteArray.readInt thrpt 5 1252719.463 ± 77598.672 ops/ms
> ByteArray.readIntFromBuffer thrpt 5 571070.474 ± 1500.426 ops/ms
> ByteArray.readLong thrpt 5 1262720.686 ± 728.100 ops/ms
> ByteArray.readLongFromBuffer thrpt 5 571594.800 ± 3376.735 ops/ms
>
>
> In this result, ByteBuffer is much slower than VarHandle. Am I doing
> something wrong? What conditions are needed to make the performance of
> ByteBuffer close to that of Unsafe?
I tried a few more. It looks like the JIT is able to optimize the ByteBuffer
away pretty well by keeping it only as a local variable without escaping.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14636#discussion_r1269582991
