andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Added configurable ByteBuf bounds checking (#7521)

Browse Source 
Motivation:

The JVM isn't always able to hoist out/reduce bounds checking (due to ref counting operations etc etc) hence making it configurable could improve performances for most CPU intensive use cases.

Modifications:

Each AbstractByteBuf bounds check has been tested against a new static final configuration property similar to checkAccessible ie io.netty.buffer.bytebuf.checkBounds.

Result:

Any user could disable ByteBuf bounds checking in order to get extra performances.
This commit is contained in:
Francesco Nigro 2018-09-03 20:33:47 +02:00 committed by Norman Maurer
parent 3eec66a974
commit c78be33443
3 changed files with 83 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
View File

@ -43,13 +43,22 @@ import static io.netty.util.internal.MathUtil.isOutOfBounds;
*/
public abstract class AbstractByteBuf extends ByteBuf {
private static final InternalLogger logger = InternalLoggerFactory.getInstance(AbstractByteBuf.class);
private static final String PROP_MODE = "io.netty.buffer.bytebuf.checkAccessible";
private static final String LEGACY_PROP_CHECK_ACCESSIBLE = "io.netty.buffer.bytebuf.checkAccessible";
private static final String PROP_CHECK_ACCESSIBLE = "io.netty.buffer.checkAccessible";
private static final boolean checkAccessible;
private static final String PROP_CHECK_BOUNDS = "io.netty.buffer.checkBounds";
private static final boolean checkBounds;
static {
checkAccessible = SystemPropertyUtil.getBoolean(PROP_MODE, true);
if (SystemPropertyUtil.contains(PROP_CHECK_ACCESSIBLE)) {
checkAccessible = SystemPropertyUtil.getBoolean(PROP_CHECK_ACCESSIBLE, true);
} else {
checkAccessible = SystemPropertyUtil.getBoolean(LEGACY_PROP_CHECK_ACCESSIBLE, true);
}
checkBounds = SystemPropertyUtil.getBoolean(PROP_CHECK_BOUNDS, true);
if (logger.isDebugEnabled()) {
logger.debug("-D{}: {}", PROP_MODE, checkAccessible);
logger.debug("-D{}: {}", PROP_CHECK_ACCESSIBLE, checkAccessible);
logger.debug("-D{}: {}", PROP_CHECK_BOUNDS, checkBounds);
}
}
@ -97,11 +106,18 @@ public abstract class AbstractByteBuf extends ByteBuf {
return readerIndex;
}
private static void checkIndexBounds(final int readerIndex, final int writerIndex, final int capacity) {
if (readerIndex < 0 || readerIndex > writerIndex || writerIndex > capacity) {
throw new IndexOutOfBoundsException(String.format(
"readerIndex: %d, writerIndex: %d (expected: 0 <= readerIndex <= writerIndex <= capacity(%d))",
readerIndex, writerIndex, capacity));
}
}
@Override
public ByteBuf readerIndex(int readerIndex) {
if (readerIndex < 0 || readerIndex > writerIndex) {
throw new IndexOutOfBoundsException(String.format(
"readerIndex: %d (expected: 0 <= readerIndex <= writerIndex(%d))", readerIndex, writerIndex));
if (checkBounds) {
checkIndexBounds(readerIndex, writerIndex, capacity());
}
this.readerIndex = readerIndex;
return this;
@ -114,10 +130,8 @@ public abstract class AbstractByteBuf extends ByteBuf {
@Override
public ByteBuf writerIndex(int writerIndex) {
if (writerIndex < readerIndex || writerIndex > capacity()) {
throw new IndexOutOfBoundsException(String.format(
"writerIndex: %d (expected: readerIndex(%d) <= writerIndex <= capacity(%d))",
writerIndex, readerIndex, capacity()));
if (checkBounds) {
checkIndexBounds(readerIndex, writerIndex, capacity());
}
this.writerIndex = writerIndex;
return this;
@ -125,10 +139,8 @@ public abstract class AbstractByteBuf extends ByteBuf {
@Override
public ByteBuf setIndex(int readerIndex, int writerIndex) {
if (readerIndex < 0 || readerIndex > writerIndex || writerIndex > capacity()) {
throw new IndexOutOfBoundsException(String.format(
"readerIndex: %d, writerIndex: %d (expected: 0 <= readerIndex <= writerIndex <= capacity(%d))",
readerIndex, writerIndex, capacity()));
if (checkBounds) {
checkIndexBounds(readerIndex, writerIndex, capacity());
}
setIndex0(readerIndex, writerIndex);
return this;
@ -271,12 +283,13 @@ public abstract class AbstractByteBuf extends ByteBuf {
if (minWritableBytes <= writableBytes()) {
return;
}
if (checkBounds) {
if (minWritableBytes > maxCapacity - writerIndex) {
throw new IndexOutOfBoundsException(String.format(
"writerIndex(%d) + minWritableBytes(%d) exceeds maxCapacity(%d): %s",
writerIndex, minWritableBytes, maxCapacity, this));
}
}
// Normalize the current capacity to the power of 2.
int newCapacity = alloc().calculateNewCapacity(writerIndex + minWritableBytes, maxCapacity);
@ -618,15 +631,21 @@ public abstract class AbstractByteBuf extends ByteBuf {
return this;
}
private static void checkReadableBounds(final ByteBuf src, final int length) {
if (length > src.readableBytes()) {
throw new IndexOutOfBoundsException(String.format(
"length(%d) exceeds src.readableBytes(%d) where src is: %s", length, src.readableBytes(), src));
}
}
@Override
public ByteBuf setBytes(int index, ByteBuf src, int length) {
checkIndex(index, length);
if (src == null) {
throw new NullPointerException("src");
}
if (length > src.readableBytes()) {
throw new IndexOutOfBoundsException(String.format(
"length(%d) exceeds src.readableBytes(%d) where src is: %s", length, src.readableBytes(), src));
if (checkBounds) {
checkReadableBounds(src, length);
}
setBytes(index, src, src.readerIndex(), length);
@ -889,10 +908,12 @@ public abstract class AbstractByteBuf extends ByteBuf {
@Override
public ByteBuf readBytes(ByteBuf dst, int length) {
if (checkBounds) {
if (length > dst.writableBytes()) {
throw new IndexOutOfBoundsException(String.format(
"length(%d) exceeds dst.writableBytes(%d) where dst is: %s", length, dst.writableBytes(), dst));
}
}
readBytes(dst, dst.writerIndex(), length);
dst.writerIndex(dst.writerIndex() + length);
return this;
@ -1065,9 +1086,8 @@ public abstract class AbstractByteBuf extends ByteBuf {
@Override
public ByteBuf writeBytes(ByteBuf src, int length) {
if (length > src.readableBytes()) {
throw new IndexOutOfBoundsException(String.format(
"length(%d) exceeds src.readableBytes(%d) where src is: %s", length, src.readableBytes(), src));
if (checkBounds) {
checkReadableBounds(src, length);
}
writeBytes(src, src.readerIndex(), length);
src.readerIndex(src.readerIndex() + length);
@ -1357,26 +1377,30 @@ public abstract class AbstractByteBuf extends ByteBuf {
checkIndex0(index, fieldLength);
}
final void checkIndex0(int index, int fieldLength) {
if (isOutOfBounds(index, fieldLength, capacity())) {
private static void checkRangeBounds(final int index, final int fieldLength, final int capacity) {
if (isOutOfBounds(index, fieldLength, capacity)) {
throw new IndexOutOfBoundsException(String.format(
"index: %d, length: %d (expected: range(0, %d))", index, fieldLength, capacity()));
"index: %d, length: %d (expected: range(0, %d))", index, fieldLength, capacity));
}
}
final void checkIndex0(int index, int fieldLength) {
if (checkBounds) {
checkRangeBounds(index, fieldLength, capacity());
}
}
protected final void checkSrcIndex(int index, int length, int srcIndex, int srcCapacity) {
checkIndex(index, length);
if (isOutOfBounds(srcIndex, length, srcCapacity)) {
throw new IndexOutOfBoundsException(String.format(
"srcIndex: %d, length: %d (expected: range(0, %d))", srcIndex, length, srcCapacity));
if (checkBounds) {
checkRangeBounds(srcIndex, length, srcCapacity);
}
}
protected final void checkDstIndex(int index, int length, int dstIndex, int dstCapacity) {
checkIndex(index, length);
if (isOutOfBounds(dstIndex, length, dstCapacity)) {
throw new IndexOutOfBoundsException(String.format(
"dstIndex: %d, length: %d (expected: range(0, %d))", dstIndex, length, dstCapacity));
if (checkBounds) {
checkRangeBounds(dstIndex, length, dstCapacity);
}
}
@ -1394,19 +1418,24 @@ public abstract class AbstractByteBuf extends ByteBuf {
protected final void checkNewCapacity(int newCapacity) {
ensureAccessible();
if (checkBounds) {
if (newCapacity < 0 || newCapacity > maxCapacity()) {
throw new IllegalArgumentException("newCapacity: " + newCapacity + " (expected: 0-" + maxCapacity() + ')');
throw new IllegalArgumentException("newCapacity: " + newCapacity +
" (expected: 0-" + maxCapacity() + ')');
}
}
}
private void checkReadableBytes0(int minimumReadableBytes) {
ensureAccessible();
if (checkBounds) {
if (readerIndex > writerIndex - minimumReadableBytes) {
throw new IndexOutOfBoundsException(String.format(
"readerIndex(%d) + length(%d) exceeds writerIndex(%d): %s",
readerIndex, minimumReadableBytes, writerIndex, this));
}
}
}
/**
* Should be called by every method that tries to access the buffers content to check

7
microbench/src/main/java/io/netty/microbench/buffer/ByteBufBenchmark.java
View File

@ -20,6 +20,7 @@ import io.netty.buffer.PooledByteBufAllocator;
import io.netty.buffer.Unpooled;
import io.netty.microbench.util.AbstractMicrobenchmark;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.TearDown;
@ -27,10 +28,13 @@ import java.nio.ByteBuffer;
public class ByteBufBenchmark extends AbstractMicrobenchmark {
static {
System.setProperty("io.netty.buffer.bytebuf.checkAccessible", "false");
System.setProperty("io.netty.buffer.checkAccessible", "false");
}
private static final byte BYTE = '0';
@Param({ "true", "false" })
public String checkBounds;
private ByteBuffer byteBuffer;
private ByteBuffer directByteBuffer;
private ByteBuf buffer;
@ -39,6 +43,7 @@ public class ByteBufBenchmark extends AbstractMicrobenchmark {
@Setup
public void setup() {
System.setProperty("io.netty.buffer.checkBounds", checkBounds);
byteBuffer = ByteBuffer.allocate(8);
directByteBuffer = ByteBuffer.allocateDirect(8);
buffer = Unpooled.buffer(8);

5
microbench/src/main/java/io/netty/microbench/buffer/HeapByteBufBenchmark.java
View File

@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufAllocator;
import io.netty.microbench.util.AbstractMicrobenchmark;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.TearDown;
@ -26,6 +27,9 @@ import java.lang.reflect.Constructor;
public class HeapByteBufBenchmark extends AbstractMicrobenchmark {
@Param({ "true", "false" })
public String checkBounds;
private ByteBuf unsafeBuffer;
private ByteBuf buffer;
@ -39,6 +43,7 @@ public class HeapByteBufBenchmark extends AbstractMicrobenchmark {
@Setup
public void setup() throws Exception {
System.setProperty("io.netty.buffer.bytebuf.checkBounds", checkBounds);
unsafeBuffer = newBuffer("io.netty.buffer.UnpooledUnsafeHeapByteBuf");
buffer = newBuffer("io.netty.buffer.UnpooledHeapByteBuf");
unsafeBuffer.writeLong(1L);