From d7fa7be67fb3cd5020ab89b64b311ff3dc7c82bb Mon Sep 17 00:00:00 2001 From: Nick Hill Date: Sat, 27 Oct 2018 08:43:28 -0700 Subject: [PATCH] Exploit PlatformDependent.allocateUninitializedArray() in more places (#8393) Motivation: There are currently many more places where this could be used which were possibly not considered when the method was added. If https://github.com/netty/netty/pull/8388 is included in its current form, a number of these places could additionally make use of the same BYTE_ARRAYS threadlocal. There's also a couple of adjacent places where an optimistically-pooled heap buffer is used for temp byte storage which could use the threadlocal too in preference to allocating a temp heap bytebuf wrapper. For example https://github.com/netty/netty/blob/4.1/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java#L1417. Modifications: Replace new byte[] with PlatformDependent.allocateUninitializedArray() where appropriate; make use of ByteBufUtil.getBytes() in some places which currently perform the equivalent logic, including avoiding copy of backing array if possible (although would be rare). Result: Further potential speed-up with java9+ and appropriate compile flags. Many of these places could be on latency-sensitive code paths. --- .../main/java/io/netty/buffer/ByteBufUtil.java | 5 +++-- .../io/netty/buffer/PooledDirectByteBuf.java | 3 ++- .../io/netty/buffer/ReadOnlyByteBufferBuf.java | 3 ++- .../src/main/java/io/netty/buffer/Unpooled.java | 10 +++++----- .../io/netty/buffer/UnpooledDirectByteBuf.java | 2 +- .../io/netty/buffer/UnpooledHeapByteBuf.java | 2 +- .../java/io/netty/buffer/UnsafeByteBufUtil.java | 3 ++- .../handler/codec/bytes/ByteArrayDecoder.java | 6 ++---- .../handler/codec/protobuf/ProtobufDecoder.java | 4 ++-- .../codec/protobuf/ProtobufDecoderNano.java | 4 ++-- .../src/main/java/io/netty/util/AsciiString.java | 16 ++++++++-------- .../channel/socket/oio/OioDatagramChannel.java | 5 ++--- 12 files changed, 32 insertions(+), 31 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java index 5afa5e9852..500e5e9c67 100644 --- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java +++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java @@ -837,7 +837,7 @@ public final class ByteBufUtil { } } - byte[] v = new byte[length]; + byte[] v = PlatformDependent.allocateUninitializedArray(length); buf.getBytes(start, v); return v; } @@ -1403,7 +1403,8 @@ public final class ByteBufUtil { tmpBuf.release(); } } else { - getBytes(buffer, new byte[chunkLen], 0, chunkLen, out, length); + byte[] tmp = PlatformDependent.allocateUninitializedArray(length); + getBytes(buffer, tmp, 0, chunkLen, out, length); } } } diff --git a/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java index 3c43509c05..8b326ccef7 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java @@ -17,6 +17,7 @@ package io.netty.buffer; import io.netty.util.Recycler; +import io.netty.util.internal.PlatformDependent; import java.io.IOException; import java.io.InputStream; @@ -351,7 +352,7 @@ final class PooledDirectByteBuf extends PooledByteBuf { @Override public int setBytes(int index, InputStream in, int length) throws IOException { checkIndex(index, length); - byte[] tmp = new byte[length]; + byte[] tmp = PlatformDependent.allocateUninitializedArray(length); int readBytes = in.read(tmp); if (readBytes <= 0) { return readBytes; diff --git a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java index 406514f384..ec34930ee1 100644 --- a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java +++ b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java @@ -15,6 +15,7 @@ */ package io.netty.buffer; +import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.StringUtil; import java.io.IOException; @@ -355,7 +356,7 @@ class ReadOnlyByteBufferBuf extends AbstractReferenceCountedByteBuf { if (buffer.hasArray()) { out.write(buffer.array(), index + buffer.arrayOffset(), length); } else { - byte[] tmp = new byte[length]; + byte[] tmp = PlatformDependent.allocateUninitializedArray(length); ByteBuffer tmpBuf = internalNioBuffer(); tmpBuf.clear().position(index); tmpBuf.get(tmp); diff --git a/buffer/src/main/java/io/netty/buffer/Unpooled.java b/buffer/src/main/java/io/netty/buffer/Unpooled.java index f6bec336dd..1c917b42c2 100644 --- a/buffer/src/main/java/io/netty/buffer/Unpooled.java +++ b/buffer/src/main/java/io/netty/buffer/Unpooled.java @@ -400,7 +400,7 @@ public final class Unpooled { if (length == 0) { return EMPTY_BUFFER; } - byte[] copy = new byte[length]; + byte[] copy = PlatformDependent.allocateUninitializedArray(length); System.arraycopy(array, offset, copy, 0, length); return wrappedBuffer(copy); } @@ -416,7 +416,7 @@ public final class Unpooled { if (length == 0) { return EMPTY_BUFFER; } - byte[] copy = new byte[length]; + byte[] copy = PlatformDependent.allocateUninitializedArray(length); // Duplicate the buffer so we not adjust the position during our get operation. // See https://github.com/netty/netty/issues/3896 ByteBuffer duplicate = buffer.duplicate(); @@ -473,7 +473,7 @@ public final class Unpooled { return EMPTY_BUFFER; } - byte[] mergedArray = new byte[length]; + byte[] mergedArray = PlatformDependent.allocateUninitializedArray(length); for (int i = 0, j = 0; i < arrays.length; i ++) { byte[] a = arrays[i]; System.arraycopy(a, 0, mergedArray, j, a.length); @@ -527,7 +527,7 @@ public final class Unpooled { return EMPTY_BUFFER; } - byte[] mergedArray = new byte[length]; + byte[] mergedArray = PlatformDependent.allocateUninitializedArray(length); for (int i = 0, j = 0; i < buffers.length; i ++) { ByteBuf b = buffers[i]; int bLen = b.readableBytes(); @@ -582,7 +582,7 @@ public final class Unpooled { return EMPTY_BUFFER; } - byte[] mergedArray = new byte[length]; + byte[] mergedArray = PlatformDependent.allocateUninitializedArray(length); for (int i = 0, j = 0; i < buffers.length; i ++) { // Duplicate the buffer so we not adjust the position during our get operation. // See https://github.com/netty/netty/issues/3896 diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java index 74c0509a76..71ab3df86a 100644 --- a/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java @@ -561,7 +561,7 @@ public class UnpooledDirectByteBuf extends AbstractReferenceCountedByteBuf { if (buffer.hasArray()) { return in.read(buffer.array(), buffer.arrayOffset() + index, length); } else { - byte[] tmp = new byte[length]; + byte[] tmp = PlatformDependent.allocateUninitializedArray(length); int readBytes = in.read(tmp); if (readBytes <= 0) { return readBytes; diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java index 0133845f82..f37ceb0559 100644 --- a/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java @@ -536,7 +536,7 @@ public class UnpooledHeapByteBuf extends AbstractReferenceCountedByteBuf { @Override public ByteBuf copy(int index, int length) { checkIndex(index, length); - byte[] copiedArray = new byte[length]; + byte[] copiedArray = PlatformDependent.allocateUninitializedArray(length); System.arraycopy(array, index, copiedArray, 0, length); return new UnpooledHeapByteBuf(alloc(), copiedArray, maxCapacity()); } diff --git a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java index 7d866d5b43..ead07253bd 100644 --- a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java +++ b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java @@ -595,7 +595,8 @@ final class UnsafeByteBufUtil { tmpBuf.release(); } } else { - getBytes(addr, new byte[len], 0, len, out, length); + byte[] tmp = PlatformDependent.allocateUninitializedArray(len); + getBytes(addr, tmp, 0, len, out, length); } } } diff --git a/codec/src/main/java/io/netty/handler/codec/bytes/ByteArrayDecoder.java b/codec/src/main/java/io/netty/handler/codec/bytes/ByteArrayDecoder.java index 1fe78c5051..21429d1867 100644 --- a/codec/src/main/java/io/netty/handler/codec/bytes/ByteArrayDecoder.java +++ b/codec/src/main/java/io/netty/handler/codec/bytes/ByteArrayDecoder.java @@ -16,6 +16,7 @@ package io.netty.handler.codec.bytes; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; import io.netty.handler.codec.LengthFieldBasedFrameDecoder; @@ -52,9 +53,6 @@ public class ByteArrayDecoder extends MessageToMessageDecoder { @Override protected void decode(ChannelHandlerContext ctx, ByteBuf msg, List out) throws Exception { // copy the ByteBuf content to a byte array - byte[] array = new byte[msg.readableBytes()]; - msg.getBytes(0, array); - - out.add(array); + out.add(ByteBufUtil.getBytes(msg)); } } diff --git a/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoder.java b/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoder.java index d48bfbf7a9..9ef56f11d4 100644 --- a/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoder.java +++ b/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoder.java @@ -20,6 +20,7 @@ import com.google.protobuf.ExtensionRegistryLite; import com.google.protobuf.Message; import com.google.protobuf.MessageLite; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; import io.netty.channel.ChannelHandler.Sharable; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; @@ -111,8 +112,7 @@ public class ProtobufDecoder extends MessageToMessageDecoder { array = msg.array(); offset = msg.arrayOffset() + msg.readerIndex(); } else { - array = new byte[length]; - msg.getBytes(msg.readerIndex(), array, 0, length); + array = ByteBufUtil.getBytes(msg, msg.readerIndex(), length, false); offset = 0; } diff --git a/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoderNano.java b/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoderNano.java index 5144b5fa3d..0d6685c979 100644 --- a/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoderNano.java +++ b/codec/src/main/java/io/netty/handler/codec/protobuf/ProtobufDecoderNano.java @@ -20,6 +20,7 @@ import com.google.protobuf.nano.MessageNano; import java.util.List; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; import io.netty.channel.ChannelHandler.Sharable; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; @@ -78,8 +79,7 @@ public class ProtobufDecoderNano extends MessageToMessageDecoder { array = msg.array(); offset = msg.arrayOffset() + msg.readerIndex(); } else { - array = new byte[length]; - msg.getBytes(msg.readerIndex(), array, 0, length); + array = ByteBufUtil.getBytes(msg, msg.readerIndex(), length, false); offset = 0; } MessageNano prototype = clazz.getConstructor().newInstance(); diff --git a/common/src/main/java/io/netty/util/AsciiString.java b/common/src/main/java/io/netty/util/AsciiString.java index 2730923f8d..3d3e1fbc21 100644 --- a/common/src/main/java/io/netty/util/AsciiString.java +++ b/common/src/main/java/io/netty/util/AsciiString.java @@ -146,7 +146,7 @@ public final class AsciiString implements CharSequence, Comparable this.offset = start; } } else { - this.value = new byte[length]; + this.value = PlatformDependent.allocateUninitializedArray(length); int oldPos = value.position(); value.get(this.value, 0, length); value.position(oldPos); @@ -172,7 +172,7 @@ public final class AsciiString implements CharSequence, Comparable + ") <= " + "value.length(" + value.length + ')'); } - this.value = new byte[length]; + this.value = PlatformDependent.allocateUninitializedArray(length); for (int i = 0, j = start; i < length; i++, j++) { this.value[i] = c2b(value[j]); } @@ -219,7 +219,7 @@ public final class AsciiString implements CharSequence, Comparable + ") <= " + "value.length(" + value.length() + ')'); } - this.value = new byte[length]; + this.value = PlatformDependent.allocateUninitializedArray(length); for (int i = 0, j = start; i < length; i++, j++) { this.value[i] = c2b(value.charAt(j)); } @@ -483,7 +483,7 @@ public final class AsciiString implements CharSequence, Comparable return that; } - byte[] newValue = new byte[thisLen + thatLen]; + byte[] newValue = PlatformDependent.allocateUninitializedArray(thisLen + thatLen); System.arraycopy(value, arrayOffset(), newValue, 0, thisLen); System.arraycopy(that.value, that.arrayOffset(), newValue, thisLen, thatLen); return new AsciiString(newValue, false); @@ -493,7 +493,7 @@ public final class AsciiString implements CharSequence, Comparable return new AsciiString(string); } - byte[] newValue = new byte[thisLen + thatLen]; + byte[] newValue = PlatformDependent.allocateUninitializedArray(thisLen + thatLen); System.arraycopy(value, arrayOffset(), newValue, 0, thisLen); for (int i = thisLen, j = 0; i < newValue.length; i++, j++) { newValue[i] = c2b(string.charAt(j)); @@ -881,7 +881,7 @@ public final class AsciiString implements CharSequence, Comparable final int len = offset + length; for (int i = offset; i < len; ++i) { if (value[i] == oldCharAsByte) { - byte[] buffer = new byte[length()]; + byte[] buffer = PlatformDependent.allocateUninitializedArray(length()); System.arraycopy(value, offset, buffer, 0, i - offset); buffer[i - offset] = newCharAsByte; ++i; @@ -942,7 +942,7 @@ public final class AsciiString implements CharSequence, Comparable return this; } - final byte[] newValue = new byte[length()]; + final byte[] newValue = PlatformDependent.allocateUninitializedArray(length()); for (i = 0, j = arrayOffset(); i < newValue.length; ++i, ++j) { newValue[i] = toLowerCase(value[j]); } @@ -972,7 +972,7 @@ public final class AsciiString implements CharSequence, Comparable return this; } - final byte[] newValue = new byte[length()]; + final byte[] newValue = PlatformDependent.allocateUninitializedArray(length()); for (i = 0, j = arrayOffset(); i < newValue.length; ++i, ++j) { newValue[i] = toUpperCase(value[j]); } diff --git a/transport/src/main/java/io/netty/channel/socket/oio/OioDatagramChannel.java b/transport/src/main/java/io/netty/channel/socket/oio/OioDatagramChannel.java index 3e588d5a36..a283db7423 100644 --- a/transport/src/main/java/io/netty/channel/socket/oio/OioDatagramChannel.java +++ b/transport/src/main/java/io/netty/channel/socket/oio/OioDatagramChannel.java @@ -16,6 +16,7 @@ package io.netty.channel.socket.oio; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; import io.netty.channel.AddressedEnvelope; import io.netty.channel.Channel; import io.netty.channel.ChannelException; @@ -276,9 +277,7 @@ public class OioDatagramChannel extends AbstractOioMessageChannel if (data.hasArray()) { tmpPacket.setData(data.array(), data.arrayOffset() + data.readerIndex(), length); } else { - byte[] tmp = new byte[length]; - data.getBytes(data.readerIndex(), tmp); - tmpPacket.setData(tmp); + tmpPacket.setData(ByteBufUtil.getBytes(data, data.readerIndex(), length)); } socket.send(tmpPacket); in.remove();