From 625981a296842fb10ee6176d3134d13764d5b4dc Mon Sep 17 00:00:00 2001 From: Nick Hill Date: Wed, 6 Nov 2019 03:06:25 -0800 Subject: [PATCH] Introduce ByteBuf#isContiguous() method (#9735) Motivation There's currently no way to determine whether an arbitrary ByteBuf behaves internally like a "singluar" buffer or a composite one, and this can be important to know when making decisions about how to manipulate it in an efficient way. An example of this is the ByteBuf#discardReadBytes() method which increases the writable bytes for a contiguous buffer (by readerIndex) but does not for a composite one. Unfortunately !(buf instanceof CompositeByteBuf) is not reliable, since for example this will be true in the case of a sliced CompositeByteBuf or some third-party composite implementation. isContiguous was chosen over isComposite since we want to assume "not contiguous" in the unknown/default case - the doc will it clear that false does not imply composite. Modifications - Add ByteBuf#isContiguous() which returns true by default - Override the "concrete" ByteBuf impls to return true and ensure wrapped/derived impls delegate it appropriately - Include some basic unit tests Result Better assumptions/decisions possible when manipulating arbitrary ByteBufs, for example when combining/cumulating them. --- .../io/netty/buffer/AbstractDerivedByteBuf.java | 5 +++++ .../netty/buffer/AbstractPooledDerivedByteBuf.java | 5 +++++ buffer/src/main/java/io/netty/buffer/ByteBuf.java | 13 +++++++++++++ .../src/main/java/io/netty/buffer/EmptyByteBuf.java | 5 +++++ .../main/java/io/netty/buffer/PooledByteBuf.java | 5 +++++ .../java/io/netty/buffer/ReadOnlyByteBufferBuf.java | 5 +++++ .../main/java/io/netty/buffer/SwappedByteBuf.java | 5 +++++ .../java/io/netty/buffer/UnpooledDirectByteBuf.java | 5 +++++ .../java/io/netty/buffer/UnpooledHeapByteBuf.java | 5 +++++ .../main/java/io/netty/buffer/WrappedByteBuf.java | 5 +++++ .../netty/buffer/AbstractCompositeByteBufTest.java | 7 +++++++ .../io/netty/buffer/AbstractPooledByteBufTest.java | 7 +++++++ .../io/netty/buffer/BigEndianDirectByteBufTest.java | 9 +++++++++ .../java/io/netty/buffer/DuplicatedByteBufTest.java | 7 +++++++ .../test/java/io/netty/buffer/EmptyByteBufTest.java | 6 ++++++ .../buffer/ReadOnlyDirectByteBufferBufTest.java | 7 +++++++ .../java/io/netty/buffer/SlicedByteBufTest.java | 7 +++++++ 17 files changed, 108 insertions(+) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractDerivedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractDerivedByteBuf.java index 40844020dd..3fd10aacfd 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractDerivedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractDerivedByteBuf.java @@ -117,4 +117,9 @@ public abstract class AbstractDerivedByteBuf extends AbstractByteBuf { public ByteBuffer nioBuffer(int index, int length) { return unwrap().nioBuffer(index, length); } + + @Override + public boolean isContiguous() { + return unwrap().isContiguous(); + } } diff --git a/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java index b106f57887..8cf8295a00 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java @@ -123,6 +123,11 @@ abstract class AbstractPooledDerivedByteBuf extends AbstractReferenceCountedByte return unwrap().hasMemoryAddress(); } + @Override + public boolean isContiguous() { + return unwrap().isContiguous(); + } + @Override public final int nioBufferCount() { return unwrap().nioBufferCount(); diff --git a/buffer/src/main/java/io/netty/buffer/ByteBuf.java b/buffer/src/main/java/io/netty/buffer/ByteBuf.java index a1a7753826..e4db1021d7 100644 --- a/buffer/src/main/java/io/netty/buffer/ByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/ByteBuf.java @@ -2338,6 +2338,19 @@ public abstract class ByteBuf implements ReferenceCounted, Comparable { */ public abstract long memoryAddress(); + /** + * Returns {@code true} if this {@link ByteBuf} implementation is backed by a single memory region. + * Composite buffer implementations must return false even if they currently hold ≤ 1 components. + * For buffers that return {@code true}, it's guaranteed that a successful call to {@link #discardReadBytes()} + * will increase the value of {@link #maxFastWritableBytes()} by the current {@code readerIndex}. + *

+ * This method will return {@code false} by default, and a {@code false} return value does not necessarily + * mean that the implementation is composite or that it is not backed by a single memory region. + */ + public boolean isContiguous() { + return false; + } + /** * Decodes this buffer's readable bytes into a string with the specified * character set name. This method is identical to diff --git a/buffer/src/main/java/io/netty/buffer/EmptyByteBuf.java b/buffer/src/main/java/io/netty/buffer/EmptyByteBuf.java index dac071ddd3..70216ac449 100644 --- a/buffer/src/main/java/io/netty/buffer/EmptyByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/EmptyByteBuf.java @@ -939,6 +939,11 @@ public final class EmptyByteBuf extends ByteBuf { } } + @Override + public boolean isContiguous() { + return true; + } + @Override public String toString(Charset charset) { return ""; diff --git a/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java index 680d2067e0..65bfe489f0 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java @@ -213,6 +213,11 @@ abstract class PooledByteBuf extends AbstractReferenceCountedByteBuf { return new ByteBuffer[] { nioBuffer(index, length) }; } + @Override + public final boolean isContiguous() { + return true; + } + @Override public final int getBytes(int index, GatheringByteChannel out, int length) throws IOException { return out.write(duplicateInternalNioBuffer(index, length)); diff --git a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java index 9dcaf61105..eaac786adc 100644 --- a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java +++ b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java @@ -453,6 +453,11 @@ class ReadOnlyByteBufferBuf extends AbstractReferenceCountedByteBuf { return (ByteBuffer) internalNioBuffer().clear().position(index).limit(index + length); } + @Override + public final boolean isContiguous() { + return true; + } + @Override public boolean hasArray() { return buffer.hasArray(); diff --git a/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java b/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java index f16ae9087e..f29a568cbd 100644 --- a/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java @@ -956,6 +956,11 @@ public class SwappedByteBuf extends ByteBuf { return buf.hasMemoryAddress(); } + @Override + public boolean isContiguous() { + return buf.isContiguous(); + } + @Override public long memoryAddress() { return buf.memoryAddress(); diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java index d3e277367d..542bb4f838 100644 --- a/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java @@ -595,6 +595,11 @@ public class UnpooledDirectByteBuf extends AbstractReferenceCountedByteBuf { return new ByteBuffer[] { nioBuffer(index, length) }; } + @Override + public final boolean isContiguous() { + return true; + } + @Override public ByteBuf copy(int index, int length) { ensureAccessible(); diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java index 8dfab10ce0..954da9d518 100644 --- a/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java @@ -320,6 +320,11 @@ public class UnpooledHeapByteBuf extends AbstractReferenceCountedByteBuf { return (ByteBuffer) internalNioBuffer().clear().position(index).limit(index + length); } + @Override + public final boolean isContiguous() { + return true; + } + @Override public byte getByte(int index) { ensureAccessible(); diff --git a/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java b/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java index c7c1eec63e..fe1bc30148 100644 --- a/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/WrappedByteBuf.java @@ -52,6 +52,11 @@ class WrappedByteBuf extends ByteBuf { return buf.hasMemoryAddress(); } + @Override + public boolean isContiguous() { + return buf.isContiguous(); + } + @Override public final long memoryAddress() { return buf.memoryAddress(); diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java index 8fa6687791..9ea6b1d9b8 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java @@ -109,6 +109,13 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { return false; } + @Test + public void testIsContiguous() { + ByteBuf buf = newBuffer(4); + assertFalse(buf.isContiguous()); + buf.release(); + } + /** * Tests the "getBufferFor" method */ diff --git a/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java index c3b2a104d0..7b49dc8372 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java @@ -114,4 +114,11 @@ public abstract class AbstractPooledByteBufTest extends AbstractByteBufTest { assertEquals(buffer.writableBytes(), buffer.maxFastWritableBytes()); buffer.release(); } + + @Test + public void testIsContiguous() { + ByteBuf buf = newBuffer(4); + assertTrue(buf.isContiguous()); + buf.release(); + } } diff --git a/buffer/src/test/java/io/netty/buffer/BigEndianDirectByteBufTest.java b/buffer/src/test/java/io/netty/buffer/BigEndianDirectByteBufTest.java index 6943c2f58e..873129503c 100644 --- a/buffer/src/test/java/io/netty/buffer/BigEndianDirectByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/BigEndianDirectByteBufTest.java @@ -19,6 +19,8 @@ import static org.junit.Assert.*; import java.nio.ByteOrder; +import org.junit.Test; + /** * Tests big-endian direct channel buffers */ @@ -35,4 +37,11 @@ public class BigEndianDirectByteBufTest extends AbstractByteBufTest { protected ByteBuf newDirectBuffer(int length, int maxCapacity) { return new UnpooledDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, maxCapacity); } + + @Test + public void testIsContiguous() { + ByteBuf buf = newBuffer(4); + assertTrue(buf.isContiguous()); + buf.release(); + } } diff --git a/buffer/src/test/java/io/netty/buffer/DuplicatedByteBufTest.java b/buffer/src/test/java/io/netty/buffer/DuplicatedByteBufTest.java index 4b3001c307..bb55a8307e 100644 --- a/buffer/src/test/java/io/netty/buffer/DuplicatedByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/DuplicatedByteBufTest.java @@ -33,6 +33,13 @@ public class DuplicatedByteBufTest extends AbstractByteBufTest { return buffer; } + @Test + public void testIsContiguous() { + ByteBuf buf = newBuffer(4); + assertEquals(buf.unwrap().isContiguous(), buf.isContiguous()); + buf.release(); + } + @Test(expected = NullPointerException.class) public void shouldNotAllowNullInConstructor() { new DuplicatedByteBuf(null); diff --git a/buffer/src/test/java/io/netty/buffer/EmptyByteBufTest.java b/buffer/src/test/java/io/netty/buffer/EmptyByteBufTest.java index c2d5764cb1..0d9fc254c5 100644 --- a/buffer/src/test/java/io/netty/buffer/EmptyByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/EmptyByteBufTest.java @@ -22,6 +22,12 @@ import static org.junit.Assert.*; public class EmptyByteBufTest { + @Test + public void testIsContiguous() { + EmptyByteBuf empty = new EmptyByteBuf(UnpooledByteBufAllocator.DEFAULT); + assertTrue(empty.isContiguous()); + } + @Test public void testIsWritable() { EmptyByteBuf empty = new EmptyByteBuf(UnpooledByteBufAllocator.DEFAULT); diff --git a/buffer/src/test/java/io/netty/buffer/ReadOnlyDirectByteBufferBufTest.java b/buffer/src/test/java/io/netty/buffer/ReadOnlyDirectByteBufferBufTest.java index 73fb7c8262..f4a26ce821 100644 --- a/buffer/src/test/java/io/netty/buffer/ReadOnlyDirectByteBufferBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/ReadOnlyDirectByteBufferBufTest.java @@ -37,6 +37,13 @@ public class ReadOnlyDirectByteBufferBufTest { return ByteBuffer.allocateDirect(size); } + @Test + public void testIsContiguous() { + ByteBuf buf = buffer(allocate(4).asReadOnlyBuffer()); + Assert.assertTrue(buf.isContiguous()); + buf.release(); + } + @Test(expected = IllegalArgumentException.class) public void testConstructWithWritable() { buffer(allocate(1)); diff --git a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java index 66bb4af96d..a5b31454ec 100644 --- a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java @@ -47,6 +47,13 @@ public class SlicedByteBufTest extends AbstractByteBufTest { return buffer.slice(offset, length); } + @Test + public void testIsContiguous() { + ByteBuf buf = newBuffer(4); + assertEquals(buf.unwrap().isContiguous(), buf.isContiguous()); + buf.release(); + } + @Test(expected = NullPointerException.class) public void shouldNotAllowNullInConstructor() { new SlicedByteBuf(null, 0, 0);