[#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.

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.
+        for (;;) {
-            // This is possible as we called ensureWritable(...) before.
+            if (buf instanceof AbstractByteBuf) {
-            for (int i = 0; i < len; i++) {
+                return writeUtf8((AbstractByteBuf) buf, seq, len);
-                char c = seq.charAt(i);
+            } else if (buf instanceof WrappedByteBuf) {
-                if (c < 0x80) {
+                // Unwrap as the wrapped buffer may be an AbstractByteBuf and so we can use fast-path.
-                    buffer._setByte(writerIndex++, (byte) c);
+                buf = buf.unwrap();
-                } else if (c < 0x800) {
+            } else {
-                    buffer._setByte(writerIndex++, (byte) (0xc0 | (c >> 6)));
+                byte[] bytes = seq.toString().getBytes(CharsetUtil.UTF_8);
-                    buffer._setByte(writerIndex++, (byte) (0x80 | (c & 0x3f)));
+                buf.writeBytes(bytes);
-                } else {
+                return bytes.length;
-                    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;
-        } 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 <a href="http://en.wikipedia.org/wiki/ASCII">ASCII</a> 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) {
+        for (;;) {
-            // Fast-Path
+            if (buf instanceof AbstractByteBuf) {
-            AbstractByteBuf buffer = (AbstractByteBuf) buf;
+                writeAscii((AbstractByteBuf) buf, seq, len);
-            int writerIndex = buffer.writerIndex;
+                break;
-
+            } else if (buf instanceof WrappedByteBuf) {
-            // We can use the _set methods as these not need to do any index checks and reference checks.
+                // Unwrap as the wrapped buffer may be an AbstractByteBuf and so we can use fast-path.
-            // This is possible as we called ensureWritable(...) before.
+                buf = buf.unwrap();
-            for (int i = 0; i < len; i++) {
+            } else {
-                buffer._setByte(writerIndex++, (byte) seq.charAt(i));
+                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}.

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();
     }
 

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);
+    }
 }