From f88dfd043030fd7ef68cd02d71894127cea35eab Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 14 Jul 2014 21:02:07 +0200 Subject: [PATCH] [#2653] Remove unnecessary ensureAccessible() calls Motivation: I introduced ensureAccessible() class as part of 6c47cc97111146396d2daf1a97051135d2eaf69e in some places. Unfortunally I also added some where these are not needed and so caused a performance regression. Modification: Remove calls where not needed. Result: Fixed performance regression. --- .../main/java/io/netty/buffer/AbstractByteBuf.java | 13 +++++++++++-- .../netty/buffer/UnsafeDirectSwappedByteBuf.java | 3 +++ .../java/io/netty/buffer/SlicedByteBufTest.java | 14 ++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java index 2e64e97075..8b69427bb3 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java @@ -229,7 +229,6 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf ensureWritable(int minWritableBytes) { - ensureAccessible(); if (minWritableBytes < 0) { throw new IllegalArgumentException(String.format( "minWritableBytes: %d (expected: >= 0)", minWritableBytes)); @@ -731,13 +730,15 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf writeByte(int value) { + ensureAccessible(); ensureWritable(1); - setByte(writerIndex++, value); + _setByte(writerIndex++, value); return this; } @Override public ByteBuf writeShort(int value) { + ensureAccessible(); ensureWritable(2); _setShort(writerIndex, value); writerIndex += 2; @@ -746,6 +747,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf writeMedium(int value) { + ensureAccessible(); ensureWritable(3); _setMedium(writerIndex, value); writerIndex += 3; @@ -754,6 +756,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf writeInt(int value) { + ensureAccessible(); ensureWritable(4); _setInt(writerIndex, value); writerIndex += 4; @@ -762,6 +765,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf writeLong(long value) { + ensureAccessible(); ensureWritable(8); _setLong(writerIndex, value); writerIndex += 8; @@ -788,6 +792,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf writeBytes(byte[] src, int srcIndex, int length) { + ensureAccessible(); ensureWritable(length); setBytes(writerIndex, src, srcIndex, length); writerIndex += length; @@ -819,6 +824,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf writeBytes(ByteBuf src, int srcIndex, int length) { + ensureAccessible(); ensureWritable(length); setBytes(writerIndex, src, srcIndex, length); writerIndex += length; @@ -827,6 +833,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf writeBytes(ByteBuffer src) { + ensureAccessible(); int length = src.remaining(); ensureWritable(length); setBytes(writerIndex, src); @@ -837,6 +844,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public int writeBytes(InputStream in, int length) throws IOException { + ensureAccessible(); ensureWritable(length); int writtenBytes = setBytes(writerIndex, in, length); if (writtenBytes > 0) { @@ -847,6 +855,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public int writeBytes(ScatteringByteChannel in, int length) throws IOException { + ensureAccessible(); ensureWritable(length); int writtenBytes = setBytes(writerIndex, in, length); if (writtenBytes > 0) { diff --git a/buffer/src/main/java/io/netty/buffer/UnsafeDirectSwappedByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnsafeDirectSwappedByteBuf.java index e777c80578..76cb60b928 100644 --- a/buffer/src/main/java/io/netty/buffer/UnsafeDirectSwappedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/UnsafeDirectSwappedByteBuf.java @@ -129,6 +129,7 @@ final class UnsafeDirectSwappedByteBuf extends SwappedByteBuf { @Override public ByteBuf writeShort(int value) { + wrapped.ensureAccessible(); wrapped.ensureWritable(2); _setShort(wrapped.writerIndex, value); wrapped.writerIndex += 2; @@ -137,6 +138,7 @@ final class UnsafeDirectSwappedByteBuf extends SwappedByteBuf { @Override public ByteBuf writeInt(int value) { + wrapped.ensureAccessible(); wrapped.ensureWritable(4); _setInt(wrapped.writerIndex, value); wrapped.writerIndex += 4; @@ -145,6 +147,7 @@ final class UnsafeDirectSwappedByteBuf extends SwappedByteBuf { @Override public ByteBuf writeLong(long value) { + wrapped.ensureAccessible(); wrapped.ensureWritable(8); _setLong(wrapped.writerIndex, value); wrapped.writerIndex += 8; diff --git a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java index ab5fa344fb..5922f8a92a 100644 --- a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java @@ -15,8 +15,10 @@ */ package io.netty.buffer; +import io.netty.util.IllegalReferenceCountException; import org.junit.Test; +import java.io.IOException; import java.util.Random; import static org.junit.Assert.*; @@ -95,6 +97,18 @@ public class SlicedByteBufTest extends AbstractByteBufTest { super.testNioBufferExposeOnlyRegion(); } + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testEnsureWritableAfterRelease() { + super.testEnsureWritableAfterRelease(); + } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteZeroAfterRelease() throws IOException { + super.testWriteZeroAfterRelease(); + } + @Test @Override public void testLittleEndianWithExpand() {