andreacavalli
/
netty-incubator-buffer-api
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix test failures coming from removal of `slice` and introduction of `copy`.

This commit is contained in:
Chris Vest 2021-05-27 17:06:30 +02:00
parent 707e5e2afb
commit 1c25fa88b7
12 changed files with 92 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
buffer-api/src/main/java/io/netty/buffer/api/Buffer.java
View File

@ -74,7 +74,7 @@ import java.nio.ByteOrder;
* 0 <= readerOffset <= writerOffset <= capacity
* </pre>
*
* <h3 name="slice-split">Splitting buffers</h3>
* <h3 name="split">Splitting buffers</h3>
*
* The {@link #split()} method break a buffer into two.
* The two buffers will share the underlying memory, but their regions will not overlap, ensuring that the memory is
@ -185,9 +185,8 @@ public interface Buffer extends Resource<Buffer>, BufferAccessors {
long nativeAddress();
/**
* Make this buffer read-only. This is irreversible.
* Unless a writable slice has previously been obtained from this buffer, there will no longer be any way to modify
* the data contained in this buffer.
* Make this buffer read-only.
* This is irreversible.
*
* @return this buffer.
*/
@ -437,7 +436,9 @@ public interface Buffer extends Resource<Buffer>, BufferAccessors {
* that contains a copy of the readable region of this buffer.
*/
default Buffer copy() {
return copy(readerOffset(), readableBytes());
int offset = readerOffset();
int length = readableBytes();
return copy(offset, length);
}
/**
@ -494,8 +495,7 @@ public interface Buffer extends Resource<Buffer>, BufferAccessors {
* <p>
* Split buffers support all operations that normal buffers do, including {@link #ensureWritable(int)}.
* <p>
* See the <a href="#slice-split">Slice vs. Split</a> section for details on the difference between slice
* and split.
* See the <a href="#split">Splitting buffers</a> section for details.
*
* @return A new buffer with independent and exclusive ownership over the read and readable bytes from this buffer.
*/
@ -543,8 +543,7 @@ public interface Buffer extends Resource<Buffer>, BufferAccessors {
* <p>
* Split buffers support all operations that normal buffers do, including {@link #ensureWritable(int)}.
* <p>
* See the <a href="#slice-split">Slice vs. Split</a> section for details on the difference between slice
* and split.
* See the <a href="#split">Splitting buffers</a> section for details.
*
* @return A new buffer with independent and exclusive ownership over the bytes from the beginning to the given
* offset of this buffer.

2
buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java
View File

@ -438,7 +438,7 @@ public final class CompositeBuffer extends ResourceSupport<Buffer, CompositeBuff
}
copies = Arrays.copyOf(copies, i);
} else {
// Specialize for length == 0, since we must slice from at least one constituent buffer.
// Specialize for length == 0, since we must copy from at least one constituent buffer.
copies = new Buffer[] { choice.copy(subOffset, 0) };
}

13
buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java
View File

@ -195,10 +195,12 @@ class NioBuffer extends ResourceSupport<Buffer, NioBuffer> implements Buffer, Re
if (!isAccessible()) {
throw new IllegalStateException("This buffer is closed: " + this + '.');
}
AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length);
ByteBuffer byteBuffer = memory.memory();
Buffer copy = new NioBuffer(byteBuffer, byteBuffer, control, memory.drop());
copyInto(0, copy, 0, length);
int allocSize = Math.max(length, 1); // Allocators don't support allocating zero-sized buffers.
AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, allocSize);
ByteBuffer base = memory.memory();
ByteBuffer buffer = length == 0? bbslice(base, 0, 0) : base;
Buffer copy = new NioBuffer(base, buffer, control, memory.drop());
copyInto(offset, copy, 0, length);
copy.writerOffset(length).order(order());
if (readOnly()) {
copy = copy.makeReadOnly();
@ -465,8 +467,7 @@ class NioBuffer extends ResourceSupport<Buffer, NioBuffer> implements Buffer, Re
if (readOnly) {
splitBuffer.makeReadOnly();
}
// Note that split, unlike slice, does not deconstify, because data changes in either buffer are not visible
// in the other. The split buffers can later deconstify independently if needed.
// Split preserves const-state.
splitBuffer.constBuffer = constBuffer;
rmem = bbslice(rmem, splitOffset, rmem.capacity() - splitOffset);
if (!readOnly) {

5
buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java
View File

@ -64,6 +64,11 @@ public interface Statics {
}
}
@SuppressWarnings("unchecked")
static <T extends Buffer> Drop<T> noOpDrop() {
return (Drop<T>) NO_OP_DROP;
}
static VarHandle findVarHandle(Lookup lookup, Class<?> recv, String name, Class<?> type) {
try {
return lookup.findVarHandle(recv, name, type);

10
buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java
View File

@ -170,9 +170,12 @@ class UnsafeBuffer extends ResourceSupport<Buffer, UnsafeBuffer> implements Buff
throw new IllegalArgumentException("Length cannot be negative: " + length + '.');
}
checkGet(offset, length);
AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length);
int allocSize = Math.max(length, 1); // Allocators don't support allocating zero-sized buffers.
AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, allocSize);
UnsafeMemory unsafeMemory = memory.memory();
Buffer copy = new UnsafeBuffer(unsafeMemory, unsafeMemory.address, length, control, memory.drop());
Buffer copy = new UnsafeBuffer(unsafeMemory, 0, length, control, memory.drop());
copyInto(offset, copy, 0, length);
copy.writerOffset(length).order(order);
if (readOnly) {
copy = copy.makeReadOnly();
@ -509,8 +512,7 @@ class UnsafeBuffer extends ResourceSupport<Buffer, UnsafeBuffer> implements Buff
if (readOnly) {
splitBuffer.makeReadOnly();
}
// Note that split, unlike slice, does not deconstify, because data changes in either buffer are not visible
// in the other. The split buffers can later deconstify independently if needed.
// Split preserves const-state.
splitBuffer.constBuffer = constBuffer;
rsize -= splitOffset;
baseOffset += splitOffset;

22
buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java
View File

@ -56,6 +56,8 @@ import static jdk.incubator.foreign.MemoryAccess.setShortAtOffset;
class MemSegBuffer extends ResourceSupport<Buffer, MemSegBuffer> implements Buffer, ReadableComponent,
WritableComponent, BufferIntegratable {
private static final MemorySegment CLOSED_SEGMENT;
private static final MemorySegment ZERO_OFFHEAP_SEGMENT;
private static final MemorySegment ZERO_ONHEAP_SEGMENT;
static final Drop<MemSegBuffer> SEGMENT_CLOSE;
static {
@ -66,6 +68,8 @@ class MemSegBuffer extends ResourceSupport<Buffer, MemSegBuffer> implements Buff
MemorySegment segment = MemorySegment.allocateNative(1, scope);
CLOSED_SEGMENT = segment.asSlice(0, 0);
}
ZERO_OFFHEAP_SEGMENT = MemorySegment.allocateNative(1, ResourceScope.newImplicitScope()).asSlice(0, 0);
ZERO_ONHEAP_SEGMENT = MemorySegment.ofArray(new byte[0]);
SEGMENT_CLOSE = new Drop<MemSegBuffer>() {
@Override
public void drop(MemSegBuffer buf) {
@ -294,16 +298,29 @@ class MemSegBuffer extends ResourceSupport<Buffer, MemSegBuffer> implements Buff
@Override
public Buffer copy(int offset, int length) {
checkGet(offset, length);
if (length < 0) {
throw new IllegalArgumentException("Length cannot be negative: " + length + '.');
}
if (!isAccessible()) {
throw new IllegalStateException("This buffer is closed: " + this + '.');
}
if (length == 0) {
// Special case zero-length segments, since allocators don't support allocating empty buffers.
final MemorySegment zero;
if (nativeAddress() == 0) {
zero = ZERO_ONHEAP_SEGMENT;
} else {
zero = ZERO_OFFHEAP_SEGMENT;
}
return new MemSegBuffer(zero, zero, Statics.noOpDrop(), control);
}
AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length);
MemorySegment segment = memory.memory();
Buffer copy = new MemSegBuffer(segment, segment, memory.drop(), control);
copyInto(0, copy, 0, length);
copyInto(offset, copy, 0, length);
copy.writerOffset(length).order(order());
if (readOnly()) {
copy = copy.makeReadOnly();
@ -576,8 +593,7 @@ class MemSegBuffer extends ResourceSupport<Buffer, MemSegBuffer> implements Buff
if (readOnly) {
splitBuffer.makeReadOnly();
}
// Note that split, unlike slice, does not deconstify, because data changes in either buffer are not visible
// in the other. The split buffers can later deconstify independently if needed.
// Split preserves const-state.
splitBuffer.constBuffer = constBuffer;
seg = seg.asSlice(splitOffset, seg.byteSize() - splitOffset);
if (!readOnly) {

3
buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java
View File

@ -100,7 +100,8 @@ public class BufferEnsureWritableTest extends BufferTestSupport {
assertThat(buf.capacity()).isGreaterThanOrEqualTo(8);
buf.writeLong(0x0102030405060708L);
try (Buffer copy = buf.copy()) {
assertEquals(0x0102030405060708L, copy.readLong());
long actual = copy.readLong();
assertEquals(0x0102030405060708L, actual);
}
}
}

4
buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java
View File

@ -197,8 +197,8 @@ public class BufferReadOnlyTest extends BufferTestSupport {
assertThat(b.capacity()).isEqualTo(8);
try (Buffer c = b.copy()) {
assertTrue(c.readOnly());
assertFalse(asRS(c).isOwned());
assertFalse(asRS(b).isOwned());
assertTrue(asRS(c).isOwned());
assertTrue(asRS(b).isOwned());
assertThat(c.capacity()).isEqualTo(8);
}
}

72
buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java
View File

@ -116,19 +116,19 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
}
assertEquals(0x01, buf.readByte());
buf.writerOffset(buf.writerOffset() - 1);
try (Buffer slice = buf.copy()) {
assertThat(toByteArray(slice)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07);
assertEquals(0, slice.readerOffset());
assertEquals(6, slice.readableBytes());
assertEquals(6, slice.writerOffset());
assertEquals(6, slice.capacity());
assertEquals(0x02, slice.readByte());
assertEquals(0x03, slice.readByte());
assertEquals(0x04, slice.readByte());
assertEquals(0x05, slice.readByte());
assertEquals(0x06, slice.readByte());
assertEquals(0x07, slice.readByte());
assertThrows(IndexOutOfBoundsException.class, slice::readByte);
try (Buffer copy = buf.copy()) {
assertThat(toByteArray(copy)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07);
assertEquals(0, copy.readerOffset());
assertEquals(6, copy.readableBytes());
assertEquals(6, copy.writerOffset());
assertEquals(6, copy.capacity());
assertEquals(0x02, copy.readByte());
assertEquals(0x03, copy.readByte());
assertEquals(0x04, copy.readByte());
assertEquals(0x05, copy.readByte());
assertEquals(0x06, copy.readByte());
assertEquals(0x07, copy.readByte());
assertThrows(IndexOutOfBoundsException.class, copy::readByte);
}
}
}
@ -143,19 +143,19 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
}
buf.readerOffset(3); // Reader and writer offsets must be ignored.
buf.writerOffset(6);
try (Buffer slice = buf.copy(1, 6)) {
assertThat(toByteArray(slice)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07);
assertEquals(0, slice.readerOffset());
assertEquals(6, slice.readableBytes());
assertEquals(6, slice.writerOffset());
assertEquals(6, slice.capacity());
assertEquals(0x02, slice.readByte());
assertEquals(0x03, slice.readByte());
assertEquals(0x04, slice.readByte());
assertEquals(0x05, slice.readByte());
assertEquals(0x06, slice.readByte());
assertEquals(0x07, slice.readByte());
assertThrows(IndexOutOfBoundsException.class, slice::readByte);
try (Buffer copy = buf.copy(1, 6)) {
assertThat(toByteArray(copy)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07);
assertEquals(0, copy.readerOffset());
assertEquals(6, copy.readableBytes());
assertEquals(6, copy.writerOffset());
assertEquals(6, copy.capacity());
assertEquals(0x02, copy.readByte());
assertEquals(0x03, copy.readByte());
assertEquals(0x04, copy.readByte());
assertEquals(0x05, copy.readByte());
assertEquals(0x06, copy.readByte());
assertEquals(0x07, copy.readByte());
assertThrows(IndexOutOfBoundsException.class, copy::readByte);
}
}
}
@ -167,11 +167,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
Buffer buf = allocator.allocate(8)) {
try (Buffer copy = buf.copy()) {
assertTrue(asRS(buf).isOwned());
buf.send().discard();
assertTrue(asRS(copy).isOwned());
copy.send().discard();
}
assertTrue(asRS(buf).isOwned());
buf.send().discard();
}
}
@ -182,11 +182,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
Buffer buf = allocator.allocate(8)) {
try (Buffer copy = buf.copy(0, 8)) {
assertTrue(asRS(buf).isOwned());
buf.send().discard();
assertTrue(asRS(copy).isOwned());
copy.send().discard();
}
assertTrue(asRS(buf).isOwned());
buf.send().discard();
}
}
@ -230,10 +230,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
try (BufferAllocator allocator = fixture.createAllocator();
Buffer buf = allocator.allocate(8)) {
try (Buffer copy = buf.copy()) {
assertFalse(asRS(buf).isOwned());
assertTrue(asRS(buf).isOwned());
copy.send().discard();
}
// Verify that the slice is closed properly afterwards.
// Verify that the copy is closed properly afterwards.
assertTrue(asRS(buf).isOwned());
buf.send().receive().close();
}
@ -245,10 +245,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
try (BufferAllocator allocator = fixture.createAllocator();
Buffer buf = allocator.allocate(8)) {
try (Buffer copy = buf.copy(0, 8)) {
assertFalse(asRS(buf).isOwned());
assertTrue(asRS(buf).isOwned());
copy.send().discard();
}
// Verify that the slice is closed properly afterwards.
// Verify that the copy is closed properly afterwards.
assertTrue(asRS(buf).isOwned());
}
}
@ -259,7 +259,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
try (BufferAllocator allocator = fixture.createAllocator();
Buffer buf = allocator.allocate(8)) {
assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(-1, 1));
// Verify that the slice is closed properly afterwards.
// Verify that the copy is closed properly afterwards.
assertTrue(asRS(buf).isOwned());
}
}
@ -271,7 +271,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
Buffer buf = allocator.allocate(8)) {
assertThrows(IllegalArgumentException.class, () -> buf.copy(0, -1));
assertThrows(IllegalArgumentException.class, () -> buf.copy(2, -1));
// Verify that the slice is closed properly afterwards.
// Verify that the copy is closed properly afterwards.
assertTrue(asRS(buf).isOwned());
}
}
@ -284,7 +284,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(0, 9));
buf.copy(0, 8).close(); // This is still fine.
assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(1, 8));
// Verify that the slice is closed properly afterwards.
// Verify that the copy is closed properly afterwards.
assertTrue(asRS(buf).isOwned());
}
}
@ -295,7 +295,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport {
try (BufferAllocator allocator = fixture.createAllocator();
Buffer buf = allocator.allocate(8)) {
buf.copy(0, 0).close(); // This is fine.
// Verify that the slice is closed properly afterwards.
// Verify that the copy is closed properly afterwards.
assertTrue(asRS(buf).isOwned());
}
}

1
buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java
View File

@ -76,6 +76,7 @@ public class BufferWriteBytesCombinationsTest extends BufferTestSupport {
assertThat(target.writerOffset()).isEqualTo(35);
assertThat(source.readerOffset()).isEqualTo(35);
assertThat(source.writerOffset()).isEqualTo(35);
source.readerOffset(0);
assertReadableEquals(source, target);
}
}

8
buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java
View File

@ -32,13 +32,13 @@ public final class ComposingAndSplittingExample {
buf.writeByte((byte) tlr.nextInt());
}
try (Buffer slice = buf.split()) {
slice.send();
try (Buffer split = buf.split()) {
split.send();
System.out.println("buf.capacity() = " + buf.capacity());
System.out.println("buf.readableBytes() = " + buf.readableBytes());
System.out.println("---");
System.out.println("slice.capacity() = " + slice.capacity());
System.out.println("slice.readableBytes() = " + slice.readableBytes());
System.out.println("split.capacity() = " + split.capacity());
System.out.println("split.readableBytes() = " + split.readableBytes());
}
}
}

8
buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java
View File

@ -314,13 +314,7 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter {
protected final void discardSomeReadBytes() {
if (cumulation != null && !first) {
// discard some bytes if possible to make more room in the
// buffer but only if the refCnt == 1 as otherwise the user may have
// used slice().retain() or duplicate().retain().
//
// See:
// - https://github.com/netty/netty/issues/2327
// - https://github.com/netty/netty/issues/1764
// Discard some bytes if possible to make more room in the buffer.
cumulation.compact();
}
}