From d6a00d06427a5215c113f8d3eee8e75fa2e7f99b Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 8 Oct 2015 13:54:48 +0200 Subject: [PATCH] [#4313] ByteBufUtil.writeUtf8 should use fast-path for WrappedByteBuf Motivation: ByteBufUtil.writeUtf8(...) / writeUsAscii(...) can use a fast-path when writing into AbstractByteBuf. We should try to unwrap WrappedByteBuf implementations so we are able to do the same on wrapped AbstractByteBuf instances. Modifications: - Try to unwrap WrappedByteBuf to use the fast-path Result: Faster writing of utf8 and usascii for WrappedByteBuf instances. --- .../java/io/netty/buffer/ByteBufUtil.java | 100 ++++++++++-------- .../java/io/netty/buffer/WrappedByteBuf.java | 57 +++++----- .../java/io/netty/buffer/ByteBufUtilTest.java | 30 ++++++ 3 files changed, 119 insertions(+), 68 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java index 7dc375fbb4..08c747dfcf 100644 --- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java +++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java @@ -395,39 +395,46 @@ public final class ByteBufUtil { final int len = seq.length(); final int maxSize = len * 3; buf.ensureWritable(maxSize); - if (buf instanceof AbstractByteBuf) { - // Fast-Path - AbstractByteBuf buffer = (AbstractByteBuf) buf; - int oldWriterIndex = buffer.writerIndex; - int writerIndex = oldWriterIndex; - // We can use the _set methods as these not need to do any index checks and reference checks. - // This is possible as we called ensureWritable(...) before. - for (int i = 0; i < len; i++) { - char c = seq.charAt(i); - if (c < 0x80) { - buffer._setByte(writerIndex++, (byte) c); - } else if (c < 0x800) { - buffer._setByte(writerIndex++, (byte) (0xc0 | (c >> 6))); - buffer._setByte(writerIndex++, (byte) (0x80 | (c & 0x3f))); - } else { - buffer._setByte(writerIndex++, (byte) (0xe0 | (c >> 12))); - buffer._setByte(writerIndex++, (byte) (0x80 | ((c >> 6) & 0x3f))); - buffer._setByte(writerIndex++, (byte) (0x80 | (c & 0x3f))); - } + for (;;) { + if (buf instanceof AbstractByteBuf) { + return writeUtf8((AbstractByteBuf) buf, seq, len); + } else if (buf instanceof WrappedByteBuf) { + // Unwrap as the wrapped buffer may be an AbstractByteBuf and so we can use fast-path. + buf = buf.unwrap(); + } else { + byte[] bytes = seq.toString().getBytes(CharsetUtil.UTF_8); + buf.writeBytes(bytes); + return bytes.length; } - // update the writerIndex without any extra checks for performance reasons - buffer.writerIndex = writerIndex; - return writerIndex - oldWriterIndex; - } else { - // Maybe we could also check if we can unwrap() to access the wrapped buffer which - // may be an AbstractByteBuf. But this may be overkill so let us keep it simple for now. - byte[] bytes = seq.toString().getBytes(CharsetUtil.UTF_8); - buf.writeBytes(bytes); - return bytes.length; } } + // Fast-Path implementation + private static int writeUtf8(AbstractByteBuf buffer, CharSequence seq, int len) { + int oldWriterIndex = buffer.writerIndex; + int writerIndex = oldWriterIndex; + + // We can use the _set methods as these not need to do any index checks and reference checks. + // This is possible as we called ensureWritable(...) before. + for (int i = 0; i < len; i++) { + char c = seq.charAt(i); + if (c < 0x80) { + buffer._setByte(writerIndex++, (byte) c); + } else if (c < 0x800) { + buffer._setByte(writerIndex++, (byte) (0xc0 | (c >> 6))); + buffer._setByte(writerIndex++, (byte) (0x80 | (c & 0x3f))); + } else { + buffer._setByte(writerIndex++, (byte) (0xe0 | (c >> 12))); + buffer._setByte(writerIndex++, (byte) (0x80 | ((c >> 6) & 0x3f))); + buffer._setByte(writerIndex++, (byte) (0x80 | (c & 0x3f))); + } + } + // update the writerIndex without any extra checks for performance reasons + buffer.writerIndex = writerIndex; + return writerIndex - oldWriterIndex; + } + /** * Encode a {@link CharSequence} in ASCII and write it * to a {@link ByteBuf}. @@ -444,26 +451,33 @@ public final class ByteBufUtil { // ASCII uses 1 byte per char final int len = seq.length(); buf.ensureWritable(len); - if (buf instanceof AbstractByteBuf) { - // Fast-Path - AbstractByteBuf buffer = (AbstractByteBuf) buf; - int writerIndex = buffer.writerIndex; - - // We can use the _set methods as these not need to do any index checks and reference checks. - // This is possible as we called ensureWritable(...) before. - for (int i = 0; i < len; i++) { - buffer._setByte(writerIndex++, (byte) seq.charAt(i)); + for (;;) { + if (buf instanceof AbstractByteBuf) { + writeAscii((AbstractByteBuf) buf, seq, len); + break; + } else if (buf instanceof WrappedByteBuf) { + // Unwrap as the wrapped buffer may be an AbstractByteBuf and so we can use fast-path. + buf = buf.unwrap(); + } else { + buf.writeBytes(seq.toString().getBytes(CharsetUtil.US_ASCII)); } - // update the writerIndex without any extra checks for performance reasons - buffer.writerIndex = writerIndex; - } else { - // Maybe we could also check if we can unwrap() to access the wrapped buffer which - // may be an AbstractByteBuf. But this may be overkill so let us keep it simple for now. - buf.writeBytes(seq.toString().getBytes(CharsetUtil.US_ASCII)); } return len; } + // Fast-Path implementation + private static void writeAscii(AbstractByteBuf buffer, CharSequence seq, int len) { + int writerIndex = buffer.writerIndex; + + // We can use the _set methods as these not need to do any index checks and reference checks. + // This is possible as we called ensureWritable(...) before. + for (int i = 0; i < len; i++) { + buffer._setByte(writerIndex++, (byte) seq.charAt(i)); + } + // update the writerIndex without any extra checks for performance reasons + buffer.writerIndex = writerIndex; + } + /** * Encode the given {@link CharBuffer} using the given {@link Charset} into a new {@link ByteBuf} which * is allocated via the {@link ByteBufAllocator}. diff --git a/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java b/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java index e8eb093aa2..de9f92cb19 100644 --- a/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java @@ -27,6 +27,13 @@ import java.nio.channels.GatheringByteChannel; import java.nio.channels.ScatteringByteChannel; import java.nio.charset.Charset; +/** + * Wraps another {@link ByteBuf}. + * + * It's important that the {@link #readerIndex()} and {@link #writerIndex()} will not do any adjustments on the + * indices on the fly because of internal optimizations made by {@link ByteBufUtil#writeAscii(ByteBuf, CharSequence)} + * and {@link ByteBufUtil#writeUtf8(ByteBuf, CharSequence)}. + */ class WrappedByteBuf extends ByteBuf { protected final ByteBuf buf; @@ -39,17 +46,17 @@ class WrappedByteBuf extends ByteBuf { } @Override - public boolean hasMemoryAddress() { + public final boolean hasMemoryAddress() { return buf.hasMemoryAddress(); } @Override - public long memoryAddress() { + public final long memoryAddress() { return buf.memoryAddress(); } @Override - public int capacity() { + public final int capacity() { return buf.capacity(); } @@ -60,17 +67,17 @@ class WrappedByteBuf extends ByteBuf { } @Override - public int maxCapacity() { + public final int maxCapacity() { return buf.maxCapacity(); } @Override - public ByteBufAllocator alloc() { + public final ByteBufAllocator alloc() { return buf.alloc(); } @Override - public ByteOrder order() { + public final ByteOrder order() { return buf.order(); } @@ -80,33 +87,33 @@ class WrappedByteBuf extends ByteBuf { } @Override - public ByteBuf unwrap() { + public final ByteBuf unwrap() { return buf; } @Override - public boolean isDirect() { + public final boolean isDirect() { return buf.isDirect(); } @Override - public int readerIndex() { + public final int readerIndex() { return buf.readerIndex(); } @Override - public ByteBuf readerIndex(int readerIndex) { + public final ByteBuf readerIndex(int readerIndex) { buf.readerIndex(readerIndex); return this; } @Override - public int writerIndex() { + public final int writerIndex() { return buf.writerIndex(); } @Override - public ByteBuf writerIndex(int writerIndex) { + public final ByteBuf writerIndex(int writerIndex) { buf.writerIndex(writerIndex); return this; } @@ -118,56 +125,56 @@ class WrappedByteBuf extends ByteBuf { } @Override - public int readableBytes() { + public final int readableBytes() { return buf.readableBytes(); } @Override - public int writableBytes() { + public final int writableBytes() { return buf.writableBytes(); } @Override - public int maxWritableBytes() { + public final int maxWritableBytes() { return buf.maxWritableBytes(); } @Override - public boolean isReadable() { + public final boolean isReadable() { return buf.isReadable(); } @Override - public boolean isWritable() { + public final boolean isWritable() { return buf.isWritable(); } @Override - public ByteBuf clear() { + public final ByteBuf clear() { buf.clear(); return this; } @Override - public ByteBuf markReaderIndex() { + public final ByteBuf markReaderIndex() { buf.markReaderIndex(); return this; } @Override - public ByteBuf resetReaderIndex() { + public final ByteBuf resetReaderIndex() { buf.resetReaderIndex(); return this; } @Override - public ByteBuf markWriterIndex() { + public final ByteBuf markWriterIndex() { buf.markWriterIndex(); return this; } @Override - public ByteBuf resetWriterIndex() { + public final ByteBuf resetWriterIndex() { buf.resetWriterIndex(); return this; } @@ -801,17 +808,17 @@ class WrappedByteBuf extends ByteBuf { } @Override - public boolean isReadable(int size) { + public final boolean isReadable(int size) { return buf.isReadable(size); } @Override - public boolean isWritable(int size) { + public final boolean isWritable(int size) { return buf.isWritable(size); } @Override - public int refCnt() { + public final int refCnt() { return buf.refCnt(); } diff --git a/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java b/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java index fb04c78ce7..1725fb525a 100644 --- a/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java +++ b/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java @@ -33,6 +33,19 @@ public class ByteBufUtilTest { Assert.assertEquals(buf, buf2); } + @Test + public void testWriteUsAsciiWrapped() { + String usAscii = "NettyRocks"; + ByteBuf buf = Unpooled.unreleasableBuffer(ReferenceCountUtil.releaseLater(Unpooled.buffer(16))); + assertWrapped(buf); + buf.writeBytes(usAscii.getBytes(CharsetUtil.US_ASCII)); + ByteBuf buf2 = Unpooled.unreleasableBuffer(ReferenceCountUtil.releaseLater(Unpooled.buffer(16))); + assertWrapped(buf2); + ByteBufUtil.writeAscii(buf2, usAscii); + + Assert.assertEquals(buf, buf2); + } + @Test public void testWriteUtf8() { String usAscii = "Some UTF-8 like äÄ∏ŒŒ"; @@ -43,4 +56,21 @@ public class ByteBufUtilTest { Assert.assertEquals(buf, buf2); } + + @Test + public void testWriteUtf8Wrapped() { + String usAscii = "Some UTF-8 like äÄ∏ŒŒ"; + ByteBuf buf = Unpooled.unreleasableBuffer(ReferenceCountUtil.releaseLater(Unpooled.buffer(16))); + assertWrapped(buf); + buf.writeBytes(usAscii.getBytes(CharsetUtil.UTF_8)); + ByteBuf buf2 = Unpooled.unreleasableBuffer(ReferenceCountUtil.releaseLater(Unpooled.buffer(16))); + assertWrapped(buf2); + ByteBufUtil.writeUtf8(buf2, usAscii); + + Assert.assertEquals(buf, buf2); + } + + private static void assertWrapped(ByteBuf buf) { + Assert.assertTrue(buf instanceof WrappedByteBuf); + } }