Replace ChannelBuffer.toByteBuffer() with hasNioBuffer() and nioBuffer()

... just like we do with byte arrays. toByteBuffer() and toByteBuffers() had an indeterministic behavior and thus it could not tell when the returned NIO buffer is shared or not. nioBuffer() always returns a view buffer of the Netty buffer. The only case where hasNioBuffer() returns false and nioBuffer() fails is the CompositeChannelBuffer, which is not very commonly used and *slow*.
rajdavies · Jun 2, 2012 · cc4f705 · cc4f705
1 parent 14e68ac
commit cc4f705
Show file tree

Hide file tree

Showing 21 changed files with 168 additions and 156 deletions.
diff --git a/buffer/src/main/java/io/netty/buffer/AbstractChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/AbstractChannelBuffer.java
@@ -600,18 +600,8 @@ public ChannelBuffer slice() {
     }
 
     @Override
-    public ByteBuffer toByteBuffer() {
-        return toByteBuffer(readerIndex, readableBytes());
-    }
-
-    @Override
-    public ByteBuffer[] toByteBuffers() {
-        return toByteBuffers(readerIndex, readableBytes());
-    }
-
-    @Override
-    public ByteBuffer[] toByteBuffers(int index, int length) {
-        return new ByteBuffer[] { toByteBuffer(index, length) };
+    public ByteBuffer nioBuffer() {
+        return nioBuffer(readerIndex, readableBytes());
     }
 
     @Override
@@ -625,8 +615,16 @@ public String toString(int index, int length, Charset charset) {
             return "";
         }
 
-        return ChannelBuffers.decodeString(
-                toByteBuffer(index, length), charset);
+        ByteBuffer nioBuffer;
+        if (hasNioBuffer()) {
+            nioBuffer = nioBuffer(index, length);
+        } else {
+            nioBuffer = ByteBuffer.allocate(length);
+            getBytes(index, nioBuffer);
+            nioBuffer.flip();
+        }
+
+        return ChannelBuffers.decodeString(nioBuffer, charset);
     }
 
     @Override

diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufferBackedChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/ByteBufferBackedChannelBuffer.java
@@ -212,6 +212,10 @@ public void setBytes(int index, byte[] src, int srcIndex, int length) {
 
     @Override
     public void setBytes(int index, ByteBuffer src) {
+        if (src == tmpBuf) {
+            src = src.duplicate();
+        }
+
         tmpBuf.clear().position(index).limit(index + src.remaining());
         tmpBuf.put(src);
     }
@@ -273,7 +277,12 @@ public int setBytes(int index, ScatteringByteChannel in, int length)
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
+    public boolean hasNioBuffer() {
+        return true;
+    }
+
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
         if (index == 0 && length == capacity()) {
             return buffer.duplicate().order(order());
         } else {

diff --git a/buffer/src/main/java/io/netty/buffer/ChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/ChannelBuffer.java
@@ -213,10 +213,9 @@
  *
  * <h4>NIO Buffers</h4>
  *
- * Various {@link #toByteBuffer()} and {@link #toByteBuffers()} methods convert
- * a {@link ChannelBuffer} into one or more NIO buffers.  These methods avoid
- * buffer allocation and memory copy whenever possible, but there's no
- * guarantee that memory copy will not be involved.
+ * If a {@link ChannelBuffer} can be converted into an NIO {@link ByteBuffer} which shares its
+ * content (i.e. view buffer), you can get it via the {@link #nioBuffer()} method.  To determine
+ * if a buffer can be converted into an NIO buffer, use {@link #nioBuffer()}.
  *
  * <h4>Strings</h4>
  *
@@ -1651,42 +1650,37 @@ public interface ChannelBuffer extends Comparable<ChannelBuffer> {
     ChannelBuffer duplicate();
 
     /**
-     * Converts this buffer's readable bytes into a NIO buffer.  The returned
-     * buffer might or might not share the content with this buffer, while
-     * they have separate indexes and marks.  This method is identical to
-     * {@code buf.toByteBuffer(buf.readerIndex(), buf.readableBytes())}.
-     * This method does not modify {@code readerIndex} or {@code writerIndex} of
-     * this buffer.
-     */
-    ByteBuffer toByteBuffer();
-
-    /**
-     * Converts this buffer's sub-region into a NIO buffer.  The returned
-     * buffer might or might not share the content with this buffer, while
-     * they have separate indexes and marks.
-     * This method does not modify {@code readerIndex} or {@code writerIndex} of
-     * this buffer.
+     * Returns {@code true} if and only if {@link #nioBuffer()} method will not fail.
      */
-    ByteBuffer toByteBuffer(int index, int length);
+    boolean hasNioBuffer();
 
     /**
-     * Converts this buffer's readable bytes into an array of NIO buffers.
-     * The returned buffers might or might not share the content with this
-     * buffer, while they have separate indexes and marks.  This method is
-     * identical to {@code buf.toByteBuffers(buf.readerIndex(), buf.readableBytes())}.
-     * This method does not modify {@code readerIndex} or {@code writerIndex} of
-     * this buffer.
+     * Exposes this buffer's readable bytes as an NIO {@link ByteBuffer}.  The returned buffer
+     * shares the content with this buffer, while changing the position and limit of the returned
+     * NIO buffer does not affect the indexes and marks of this buffer.  This method is identical
+     * to {@code buf.asByteBuffer(buf.readerIndex(), buf.readableBytes())}.  This method does not
+     * modify {@code readerIndex} or {@code writerIndex} of this buffer.  Please note that the
+     * returned NIO buffer will not see the changes of this buffer if this buffer is a dynamic
+     * buffer and it adjusted its capacity.
+     *
+     *
+     * @throws UnsupportedOperationException
+     *         if this buffer cannot create a {@link ByteBuffer} that shares the content with itself
      */
-    ByteBuffer[] toByteBuffers();
+    ByteBuffer nioBuffer();
 
     /**
-     * Converts this buffer's sub-region into an array of NIO buffers.
-     * The returned buffers might or might not share the content with this
-     * buffer, while they have separate indexes and marks.
-     * This method does not modify {@code readerIndex} or {@code writerIndex} of
-     * this buffer.
+     * Exposes this buffer's sub-region as an NIO {@link ByteBuffer}.  The returned buffer
+     * shares the content with this buffer, while changing the position and limit of the returned
+     * NIO buffer does not affect the indexes and marks of this buffer.  This method does not
+     * modify {@code readerIndex} or {@code writerIndex} of this buffer.  Please note that the
+     * returned NIO buffer will not see the changes of this buffer if this buffer is a dynamic
+     * buffer and it adjusted its capacity.
+     *
+     * @throws UnsupportedOperationException
+     *         if this buffer cannot create a {@link ByteBuffer} that shares the content with itself
      */
-    ByteBuffer[] toByteBuffers(int index, int length);
+    ByteBuffer nioBuffer(int index, int length);
 
     /**
      * Returns {@code true} if and only if this buffer has a backing byte array.

diff --git a/buffer/src/main/java/io/netty/buffer/CompositeChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/CompositeChannelBuffer.java
@@ -15,6 +15,8 @@
  */
 package io.netty.buffer;
 
+import io.netty.util.internal.DetectionUtil;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -297,10 +299,18 @@ public void getBytes(int index, ChannelBuffer dst, int dstIndex, int length) {
     @Override
     public int getBytes(int index, GatheringByteChannel out, int length)
             throws IOException {
-        // XXX Gathering write is not supported because of a known issue.
-        //     See http://bugs.sun.com/view_bug.do?bug_id=6210541
-        //     This issue appeared in 2004 and is still unresolved!?
-        return out.write(toByteBuffer(index, length));
+        if (DetectionUtil.javaVersion() < 7) {
+            // XXX Gathering write is not supported because of a known issue.
+            //     See http://bugs.sun.com/view_bug.do?bug_id=6210541
+            return out.write(copiedNioBuffer(index, length));
+        } else {
+            long writtenBytes = out.write(nioBuffers(index, length));
+            if (writtenBytes > Integer.MAX_VALUE) {
+                return Integer.MAX_VALUE;
+            } else {
+                return (int) writtenBytes;
+            }
+        }
     }
 
     @Override
@@ -593,12 +603,20 @@ public ChannelBuffer slice(int index, int length) {
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
+    public boolean hasNioBuffer() {
+        return false;
+    }
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        throw new UnsupportedOperationException();
+    }
+
+    private ByteBuffer copiedNioBuffer(int index, int length) {
         if (components.length == 1) {
-            return components[0].toByteBuffer(index, length);
+            return toNioBuffer(components[0], index, length);
         }
 
-        ByteBuffer[] buffers = toByteBuffers(index, length);
+        ByteBuffer[] buffers = nioBuffers(index, length);
         ByteBuffer merged = ByteBuffer.allocate(length).order(order());
         for (ByteBuffer b: buffers) {
             merged.put(b);
@@ -607,8 +625,7 @@ public ByteBuffer toByteBuffer(int index, int length) {
         return merged;
     }
 
-    @Override
-    public ByteBuffer[] toByteBuffers(int index, int length) {
+    private ByteBuffer[] nioBuffers(int index, int length) {
         int componentId = componentId(index);
         if (index + length > capacity()) {
             throw new IndexOutOfBoundsException("Too many bytes to convert - Needs"
@@ -619,10 +636,10 @@ public ByteBuffer[] toByteBuffers(int index, int length) {
 
         int i = componentId;
         while (length > 0) {
-            ChannelBuffer s = components[i];
+            ChannelBuffer c = components[i];
             int adjustment = indices[i];
-            int localLength = Math.min(length, s.capacity() - (index - adjustment));
-            buffers.add(s.toByteBuffer(index - adjustment, localLength));
+            int localLength = Math.min(length, c.capacity() - (index - adjustment));
+            buffers.add(toNioBuffer(c, index - adjustment, localLength));
             index += localLength;
             length -= localLength;
             i ++;
@@ -631,6 +648,14 @@ public ByteBuffer[] toByteBuffers(int index, int length) {
         return buffers.toArray(new ByteBuffer[buffers.size()]);
     }
 
+    private static ByteBuffer toNioBuffer(ChannelBuffer buf, int index, int length) {
+        if (buf.hasNioBuffer()) {
+            return buf.nioBuffer(index, length);
+        } else {
+            return buf.copy(index, length).nioBuffer(0, length);
+        }
+    }
+
     private int componentId(int index) {
         int lastComponentId = lastAccessedComponentId;
         if (index >= indices[lastComponentId]) {

diff --git a/buffer/src/main/java/io/netty/buffer/DuplicatedChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/DuplicatedChannelBuffer.java
@@ -206,7 +206,12 @@ public int setBytes(int index, ScatteringByteChannel in, int length)
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
-        return buffer.toByteBuffer(index, length);
+    public boolean hasNioBuffer() {
+        return buffer.hasNioBuffer();
+    }
+
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        return buffer.nioBuffer(index, length);
     }
 }
diff --git a/buffer/src/main/java/io/netty/buffer/DynamicChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/DynamicChannelBuffer.java
@@ -322,7 +322,12 @@ public ChannelBuffer slice(int index, int length) {
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
-        return buffer.toByteBuffer(index, length);
+    public boolean hasNioBuffer() {
+        return buffer.hasNioBuffer();
+    }
+
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        return buffer.nioBuffer(index, length);
     }
 }
diff --git a/buffer/src/main/java/io/netty/buffer/HeapChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/HeapChannelBuffer.java
@@ -190,7 +190,12 @@ public ChannelBuffer slice(int index, int length) {
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
+    public boolean hasNioBuffer() {
+        return true;
+    }
+
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
         return ByteBuffer.wrap(array, index, length).order(order());
     }
 }
diff --git a/buffer/src/main/java/io/netty/buffer/ReadOnlyChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/ReadOnlyChannelBuffer.java
@@ -206,17 +206,13 @@ public long getLong(int index) {
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
-        return buffer.toByteBuffer(index, length).asReadOnlyBuffer();
+    public boolean hasNioBuffer() {
+        return buffer.hasNioBuffer();
     }
 
     @Override
-    public ByteBuffer[] toByteBuffers(int index, int length) {
-        ByteBuffer[] bufs = buffer.toByteBuffers(index, length);
-        for (int i = 0; i < bufs.length; i ++) {
-            bufs[i] = bufs[i].asReadOnlyBuffer();
-        }
-        return bufs;
+    public ByteBuffer nioBuffer(int index, int length) {
+        return buffer.nioBuffer(index, length).asReadOnlyBuffer();
     }
 
     @Override

diff --git a/buffer/src/main/java/io/netty/buffer/SlicedChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/SlicedChannelBuffer.java
@@ -240,9 +240,14 @@ public int setBytes(int index, ScatteringByteChannel in, int length)
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
+    public boolean hasNioBuffer() {
+        return buffer.hasNioBuffer();
+    }
+
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
         checkIndex(index, length);
-        return buffer.toByteBuffer(index + adjustment, length);
+        return buffer.nioBuffer(index + adjustment, length);
     }
 
     private void checkIndex(int index) {

diff --git a/buffer/src/main/java/io/netty/buffer/TruncatedChannelBuffer.java b/buffer/src/main/java/io/netty/buffer/TruncatedChannelBuffer.java
@@ -233,9 +233,14 @@ public int setBytes(int index, ScatteringByteChannel in, int length)
     }
 
     @Override
-    public ByteBuffer toByteBuffer(int index, int length) {
+    public boolean hasNioBuffer() {
+        return buffer.hasNioBuffer();
+    }
+
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
         checkIndex(index, length);
-        return buffer.toByteBuffer(index, length);
+        return buffer.nioBuffer(index, length);
     }
 
     private void checkIndex(int index) {