From 1aba6001b4d9babb3824b9b6fa7bc5feddb9930a Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 21 Oct 2015 10:04:49 +0200 Subject: [PATCH] Share code between Unsafe ByteBuf implementations Motiviation: We have a lot of duplicated code which makes it hard to maintain. Modification: Move shared code to UnsafeByteBufUtil and use it in the implementations. Result: Less duplicated code and so easier to maintain. --- .../java/io/netty/buffer/AbstractByteBuf.java | 2 +- .../buffer/PooledUnsafeDirectByteBuf.java | 113 ++------------ .../netty/buffer/UnpooledDirectByteBuf.java | 5 - .../buffer/UnpooledUnsafeDirectByteBuf.java | 116 ++------------ .../io/netty/buffer/UnsafeByteBufUtil.java | 141 ++++++++++++++++++ 5 files changed, 160 insertions(+), 217 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java index 0e29175c35..8ca303bfcd 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java @@ -1132,7 +1132,7 @@ public abstract class AbstractByteBuf extends ByteBuf { } } - private static boolean isInvalid(int index, int length, int capacity) { + static boolean isInvalid(int index, int length, int capacity) { return (index | length | (index + length) | (capacity - (index + length))) < 0; } diff --git a/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java index 3ceb4ac780..fca08e40a3 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java @@ -101,59 +101,19 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf { @Override public ByteBuf getBytes(int index, ByteBuf dst, int dstIndex, int length) { - checkIndex(index, length); - if (dst == null) { - throw new NullPointerException("dst"); - } - if (dstIndex < 0 || dstIndex > dst.capacity() - length) { - throw new IndexOutOfBoundsException("dstIndex: " + dstIndex); - } - - if (length != 0) { - if (dst.hasMemoryAddress()) { - PlatformDependent.copyMemory(addr(index), dst.memoryAddress() + dstIndex, length); - } else if (dst.hasArray()) { - PlatformDependent.copyMemory(addr(index), dst.array(), dst.arrayOffset() + dstIndex, length); - } else { - dst.setBytes(dstIndex, this, index, length); - } - } + UnsafeByteBufUtil.getBytes(this, addr(index), index, dst, dstIndex, length); return this; } @Override public ByteBuf getBytes(int index, byte[] dst, int dstIndex, int length) { - checkIndex(index, length); - if (dst == null) { - throw new NullPointerException("dst"); - } - if (dstIndex < 0 || dstIndex > dst.length - length) { - throw new IndexOutOfBoundsException("dstIndex: " + dstIndex); - } - if (length != 0) { - PlatformDependent.copyMemory(addr(index), dst, dstIndex, length); - } + UnsafeByteBufUtil.getBytes(this, addr(index), index, dst, dstIndex, length); return this; } @Override public ByteBuf getBytes(int index, ByteBuffer dst) { - checkIndex(index); - int bytesToCopy = Math.min(capacity() - index, dst.remaining()); - if (bytesToCopy == 0) { - return this; - } - - if (dst.isDirect()) { - // Copy to direct memory - long dstAddress = PlatformDependent.directBufferAddress(dst); - PlatformDependent.copyMemory(addr(index), dstAddress + dst.position(), bytesToCopy); - } else { - // Copy to array - PlatformDependent.copyMemory(addr(index), dst.array(), dst.arrayOffset() + dst.position(), bytesToCopy); - } - - dst.position(dst.position() + bytesToCopy); + UnsafeByteBufUtil.getBytes(this, addr(index), index, dst); return this; } @@ -168,12 +128,7 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf { @Override public ByteBuf getBytes(int index, OutputStream out, int length) throws IOException { - checkIndex(index, length); - if (length != 0) { - byte[] tmp = new byte[length]; - PlatformDependent.copyMemory(addr(index), tmp, 0, length); - out.write(tmp); - } + UnsafeByteBufUtil.getBytes(this, addr(index), index, out, length); return this; } @@ -235,65 +190,25 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf { @Override public ByteBuf setBytes(int index, ByteBuf src, int srcIndex, int length) { - checkIndex(index, length); - if (src == null) { - throw new NullPointerException("src"); - } - if (srcIndex < 0 || srcIndex > src.capacity() - length) { - throw new IndexOutOfBoundsException("srcIndex: " + srcIndex); - } - - if (length != 0) { - if (src.hasMemoryAddress()) { - PlatformDependent.copyMemory(src.memoryAddress() + srcIndex, addr(index), length); - } else if (src.hasArray()) { - PlatformDependent.copyMemory(src.array(), src.arrayOffset() + srcIndex, addr(index), length); - } else { - src.getBytes(srcIndex, this, index, length); - } - } + UnsafeByteBufUtil.setBytes(this, addr(index), index, src, srcIndex, length); return this; } @Override public ByteBuf setBytes(int index, byte[] src, int srcIndex, int length) { - checkIndex(index, length); - if (length != 0) { - PlatformDependent.copyMemory(src, srcIndex, addr(index), length); - } + UnsafeByteBufUtil.setBytes(this, addr(index), index, src, srcIndex, length); return this; } @Override public ByteBuf setBytes(int index, ByteBuffer src) { - checkIndex(index, src.remaining()); - - int length = src.remaining(); - if (length == 0) { - return this; - } - - if (src.isDirect()) { - // Copy from direct memory - long srcAddress = PlatformDependent.directBufferAddress(src); - PlatformDependent.copyMemory(srcAddress + src.position(), addr(index), src.remaining()); - } else { - // Copy from array - PlatformDependent.copyMemory(src.array(), src.arrayOffset() + src.position(), addr(index), length); - } - src.position(src.position() + length); + UnsafeByteBufUtil.setBytes(this, addr(index), index, src); return this; } @Override public int setBytes(int index, InputStream in, int length) throws IOException { - checkIndex(index, length); - byte[] tmp = new byte[length]; - int readBytes = in.read(tmp); - if (readBytes > 0) { - PlatformDependent.copyMemory(tmp, 0, addr(index), readBytes); - } - return readBytes; + return UnsafeByteBufUtil.setBytes(this, addr(index), index, in, length); } @Override @@ -311,17 +226,7 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf { @Override public ByteBuf copy(int index, int length) { - checkIndex(index, length); - ByteBuf copy = alloc().directBuffer(length, maxCapacity()); - if (length != 0) { - if (copy.hasMemoryAddress()) { - PlatformDependent.copyMemory(addr(index), copy.memoryAddress(), length); - copy.setIndex(0, length); - } else { - copy.writeBytes(this, index, length); - } - } - return copy; + return UnsafeByteBufUtil.copy(this, addr(index), index, length); } @Override diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java index 1414c8ee8e..de95ea4eb1 100644 --- a/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java @@ -291,11 +291,6 @@ public class UnpooledDirectByteBuf extends AbstractReferenceCountedByteBuf { private void getBytes(int index, byte[] dst, int dstIndex, int length, boolean internal) { checkDstIndex(index, length, dstIndex, dst.length); - if (dstIndex < 0 || dstIndex > dst.length - length) { - throw new IndexOutOfBoundsException(String.format( - "dstIndex: %d, length: %d (expected: range(0, %d))", dstIndex, length, dst.length)); - } - ByteBuffer tmpBuf; if (internal) { tmpBuf = internalNioBuffer(); diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java index 7f5f598c58..f6d65150ec 100644 --- a/buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java @@ -240,63 +240,19 @@ public class UnpooledUnsafeDirectByteBuf extends AbstractReferenceCountedByteBuf @Override public ByteBuf getBytes(int index, ByteBuf dst, int dstIndex, int length) { - checkIndex(index, length); - if (dst == null) { - throw new NullPointerException("dst"); - } - if (dstIndex < 0 || dstIndex > dst.capacity() - length) { - throw new IndexOutOfBoundsException("dstIndex: " + dstIndex); - } - - if (dst.hasMemoryAddress()) { - PlatformDependent.copyMemory(addr(index), dst.memoryAddress() + dstIndex, length); - } else if (dst.hasArray()) { - PlatformDependent.copyMemory(addr(index), dst.array(), dst.arrayOffset() + dstIndex, length); - } else { - dst.setBytes(dstIndex, this, index, length); - } + UnsafeByteBufUtil.getBytes(this, addr(index), index, dst, dstIndex, length); return this; } @Override public ByteBuf getBytes(int index, byte[] dst, int dstIndex, int length) { - checkIndex(index, length); - if (dst == null) { - throw new NullPointerException("dst"); - } - if (dstIndex < 0 || dstIndex > dst.length - length) { - throw new IndexOutOfBoundsException(String.format( - "dstIndex: %d, length: %d (expected: range(0, %d))", dstIndex, length, dst.length)); - } - - if (length != 0) { - PlatformDependent.copyMemory(addr(index), dst, dstIndex, length); - } + UnsafeByteBufUtil.getBytes(this, addr(index), index, dst, dstIndex, length); return this; } @Override public ByteBuf getBytes(int index, ByteBuffer dst) { - checkIndex(index); - if (dst == null) { - throw new NullPointerException("dst"); - } - - int bytesToCopy = Math.min(capacity() - index, dst.remaining()); - if (bytesToCopy == 0) { - return this; - } - - if (dst.isDirect()) { - // Copy to direct memory - long dstAddress = PlatformDependent.directBufferAddress(dst); - PlatformDependent.copyMemory(addr(index), dstAddress + dst.position(), bytesToCopy); - } else { - // Copy to array - PlatformDependent.copyMemory(addr(index), dst.array(), dst.arrayOffset() + dst.position(), bytesToCopy); - } - - dst.position(dst.position() + bytesToCopy); + UnsafeByteBufUtil.getBytes(this, addr(index), index, dst); return this; } @@ -336,63 +292,25 @@ public class UnpooledUnsafeDirectByteBuf extends AbstractReferenceCountedByteBuf @Override public ByteBuf setBytes(int index, ByteBuf src, int srcIndex, int length) { - checkIndex(index, length); - if (src == null) { - throw new NullPointerException("src"); - } - if (srcIndex < 0 || srcIndex > src.capacity() - length) { - throw new IndexOutOfBoundsException("srcIndex: " + srcIndex); - } - - if (length != 0) { - if (src.hasMemoryAddress()) { - PlatformDependent.copyMemory(src.memoryAddress() + srcIndex, addr(index), length); - } else if (src.hasArray()) { - PlatformDependent.copyMemory(src.array(), src.arrayOffset() + srcIndex, addr(index), length); - } else { - src.getBytes(srcIndex, this, index, length); - } - } + UnsafeByteBufUtil.setBytes(this, addr(index), index, src, srcIndex, length); return this; } @Override public ByteBuf setBytes(int index, byte[] src, int srcIndex, int length) { - checkIndex(index, length); - if (length != 0) { - PlatformDependent.copyMemory(src, srcIndex, addr(index), length); - } + UnsafeByteBufUtil.setBytes(this, addr(index), index, src, srcIndex, length); return this; } @Override public ByteBuf setBytes(int index, ByteBuffer src) { - ensureAccessible(); - int length = src.remaining(); - if (length == 0) { - return this; - } - - if (src.isDirect()) { - // Copy from direct memory - long srcAddress = PlatformDependent.directBufferAddress(src); - PlatformDependent.copyMemory(srcAddress + src.position(), addr(index), src.remaining()); - } else { - // Copy from array - PlatformDependent.copyMemory(src.array(), src.arrayOffset() + src.position(), addr(index), length); - } - src.position(src.position() + length); + UnsafeByteBufUtil.setBytes(this, addr(index), index, src); return this; } @Override public ByteBuf getBytes(int index, OutputStream out, int length) throws IOException { - ensureAccessible(); - if (length != 0) { - byte[] tmp = new byte[length]; - PlatformDependent.copyMemory(addr(index), tmp, 0, length); - out.write(tmp); - } + UnsafeByteBufUtil.getBytes(this, addr(index), index, out, length); return this; } @@ -427,13 +345,7 @@ public class UnpooledUnsafeDirectByteBuf extends AbstractReferenceCountedByteBuf @Override public int setBytes(int index, InputStream in, int length) throws IOException { - checkIndex(index, length); - byte[] tmp = new byte[length]; - int readBytes = in.read(tmp); - if (readBytes > 0) { - PlatformDependent.copyMemory(tmp, 0, addr(index), readBytes); - } - return readBytes; + return UnsafeByteBufUtil.setBytes(this, addr(index), index, in, length); } @Override @@ -460,17 +372,7 @@ public class UnpooledUnsafeDirectByteBuf extends AbstractReferenceCountedByteBuf @Override public ByteBuf copy(int index, int length) { - checkIndex(index, length); - ByteBuf copy = alloc().directBuffer(length, maxCapacity()); - if (length != 0) { - if (copy.hasMemoryAddress()) { - PlatformDependent.copyMemory(addr(index), copy.memoryAddress(), length); - copy.setIndex(0, length); - } else { - copy.writeBytes(this, index, length); - } - } - return copy; + return UnsafeByteBufUtil.copy(this, addr(index), index, length); } @Override diff --git a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java index 623651923a..9fa3fbf7bf 100644 --- a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java +++ b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java @@ -17,8 +17,14 @@ package io.netty.buffer; import io.netty.util.internal.PlatformDependent; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.ByteBuffer; import java.nio.ByteOrder; +import static io.netty.util.internal.ObjectUtil.checkNotNull; + /** * All operations get and set as {@link ByteOrder#BIG_ENDIAN}. */ @@ -243,5 +249,140 @@ final class UnsafeByteBufUtil { } } + static ByteBuf copy(AbstractByteBuf buf, long addr, int index, int length) { + buf.checkIndex(index, length); + ByteBuf copy = buf.alloc().directBuffer(length, buf.maxCapacity()); + if (length != 0) { + if (copy.hasMemoryAddress()) { + PlatformDependent.copyMemory(addr, copy.memoryAddress(), length); + copy.setIndex(0, length); + } else { + copy.writeBytes(buf, index, length); + } + } + return copy; + } + + static int setBytes(AbstractByteBuf buf, long addr, int index, InputStream in, int length) throws IOException { + buf.checkIndex(index, length); + ByteBuf tmpBuf = buf.alloc().heapBuffer(length); + try { + byte[] tmp = tmpBuf.array(); + int offset = tmpBuf.arrayOffset(); + int readBytes = in.read(tmp, offset, length); + if (readBytes > 0) { + PlatformDependent.copyMemory(tmp, offset, addr, readBytes); + } + return readBytes; + } finally { + tmpBuf.release(); + } + } + + static void getBytes(AbstractByteBuf buf, long addr, int index, ByteBuf dst, int dstIndex, int length) { + buf.checkIndex(index, length); + checkNotNull(dst, "dst"); + if (AbstractByteBuf.isInvalid(dstIndex, length, dst.capacity())) { + throw new IndexOutOfBoundsException("dstIndex: " + dstIndex); + } + + if (dst.hasMemoryAddress()) { + PlatformDependent.copyMemory(addr, dst.memoryAddress() + dstIndex, length); + } else if (dst.hasArray()) { + PlatformDependent.copyMemory(addr, dst.array(), dst.arrayOffset() + dstIndex, length); + } else { + dst.setBytes(dstIndex, buf, index, length); + } + } + + static void getBytes(AbstractByteBuf buf, long addr, int index, byte[] dst, int dstIndex, int length) { + buf.checkIndex(index, length); + checkNotNull(dst, "dst"); + if (AbstractByteBuf.isInvalid(dstIndex, length, dst.length)) { + throw new IndexOutOfBoundsException("dstIndex: " + dstIndex); + } + if (length != 0) { + PlatformDependent.copyMemory(addr, dst, dstIndex, length); + } + } + + static void getBytes(AbstractByteBuf buf, long addr, int index, ByteBuffer dst) { + buf.checkIndex(index); + int bytesToCopy = Math.min(buf.capacity() - index, dst.remaining()); + if (bytesToCopy == 0) { + return; + } + + if (dst.isDirect()) { + // Copy to direct memory + long dstAddress = PlatformDependent.directBufferAddress(dst); + PlatformDependent.copyMemory(addr, dstAddress + dst.position(), bytesToCopy); + } else { + // Copy to array + PlatformDependent.copyMemory(addr, dst.array(), dst.arrayOffset() + dst.position(), bytesToCopy); + } + + dst.position(dst.position() + bytesToCopy); + } + + static void setBytes(AbstractByteBuf buf, long addr, int index, ByteBuf src, int srcIndex, int length) { + buf.checkIndex(index, length); + checkNotNull(src, "src"); + if (AbstractByteBuf.isInvalid(srcIndex, length, src.capacity())) { + throw new IndexOutOfBoundsException("srcIndex: " + srcIndex); + } + + if (length != 0) { + if (src.hasMemoryAddress()) { + PlatformDependent.copyMemory(src.memoryAddress() + srcIndex, addr, length); + } else if (src.hasArray()) { + PlatformDependent.copyMemory(src.array(), src.arrayOffset() + srcIndex, addr, length); + } else { + src.getBytes(srcIndex, buf, index, length); + } + } + } + + static void setBytes(AbstractByteBuf buf, long addr, int index, byte[] src, int srcIndex, int length) { + buf.checkIndex(index, length); + if (length != 0) { + PlatformDependent.copyMemory(src, srcIndex, addr, length); + } + } + + static void setBytes(AbstractByteBuf buf, long addr, int index, ByteBuffer src) { + buf.checkIndex(index, src.remaining()); + + int length = src.remaining(); + if (length == 0) { + return; + } + + if (src.isDirect()) { + // Copy from direct memory + long srcAddress = PlatformDependent.directBufferAddress(src); + PlatformDependent.copyMemory(srcAddress + src.position(), addr, src.remaining()); + } else { + // Copy from array + PlatformDependent.copyMemory(src.array(), src.arrayOffset() + src.position(), addr, length); + } + src.position(src.position() + length); + } + + static void getBytes(AbstractByteBuf buf, long addr, int index, OutputStream out, int length) throws IOException { + buf.checkIndex(index, length); + if (length != 0) { + ByteBuf tmpBuf = buf.alloc().heapBuffer(length); + try { + byte[] tmp = tmpBuf.array(); + int offset = tmpBuf.arrayOffset(); + PlatformDependent.copyMemory(addr, tmp, offset, length); + out.write(tmp, offset, length); + } finally { + tmpBuf.release(); + } + } + } + private UnsafeByteBufUtil() { } }