Don't zero non-readable buffer regions when capacity is decreased (#9427)

Motivation

region is preserved when capacity is increased, not just the readable
part. The behaviour is still different however when the capacity is
_decreased_ - data outside the currently-readable region is zeroed.

Modifications

Update ByteBuf capacity(int) implementations to also copy the whole
buffer region when the new capacity is less than the current capacity.

Result

Consistent behaviour of ByteBuf#capacity(int) regardless of whether the
new capacity is greater than or less than the current capacity.
This commit is contained in:
Nick Hill 2019-08-15 23:18:09 -07:00 committed by Norman Maurer
parent 96f92929ab
commit f673ba36a0
6 changed files with 48 additions and 83 deletions

View File

@ -218,6 +218,13 @@ public abstract class AbstractByteBuf extends ByteBuf {
return this;
}
// Called after a capacity reduction
protected final void trimIndicesToCapacity(int newCapacity) {
if (writerIndex() > newCapacity) {
setIndex0(Math.min(readerIndex(), newCapacity), newCapacity);
}
}
@Override
public ByteBuf ensureWritable(int minWritableBytes) {
checkPositiveOrZero(minWritableBytes, "minWritableBytes");

View File

@ -379,9 +379,7 @@ abstract class PoolArena<T> implements PoolArenaMetric {
}
void reallocate(PooledByteBuf<T> buf, int newCapacity, boolean freeOldMemory) {
if (newCapacity < 0 || newCapacity > buf.maxCapacity()) {
throw new IllegalArgumentException("newCapacity: " + newCapacity);
}
assert newCapacity >= 0 && newCapacity <= buf.maxCapacity();
int oldCapacity = buf.length;
if (oldCapacity == newCapacity) {
@ -394,29 +392,17 @@ abstract class PoolArena<T> implements PoolArenaMetric {
T oldMemory = buf.memory;
int oldOffset = buf.offset;
int oldMaxLength = buf.maxLength;
int readerIndex = buf.readerIndex();
int writerIndex = buf.writerIndex();
// This does not touch buf's reader/writer indices
allocate(parent.threadCache(), buf, newCapacity);
int bytesToCopy;
if (newCapacity > oldCapacity) {
memoryCopy(
oldMemory, oldOffset,
buf.memory, buf.offset, oldCapacity);
} else if (newCapacity < oldCapacity) {
if (readerIndex < newCapacity) {
if (writerIndex > newCapacity) {
writerIndex = newCapacity;
}
memoryCopy(
oldMemory, oldOffset + readerIndex,
buf.memory, buf.offset + readerIndex, writerIndex - readerIndex);
} else {
readerIndex = writerIndex = newCapacity;
}
bytesToCopy = oldCapacity;
} else {
buf.trimIndicesToCapacity(newCapacity);
bytesToCopy = newCapacity;
}
buf.setIndex(readerIndex, writerIndex);
memoryCopy(oldMemory, oldOffset, buf.memory, buf.offset, bytesToCopy);
if (freeOldMemory) {
free(oldChunk, oldNioBuffer, oldHandle, oldMaxLength, buf.cache);
}

View File

@ -109,7 +109,7 @@ abstract class PooledByteBuf<T> extends AbstractReferenceCountedByteBuf {
(maxLength > 512 || newCapacity > maxLength - 16)) {
// here newCapacity < length
length = newCapacity;
setIndex(Math.min(readerIndex(), newCapacity), Math.min(writerIndex(), newCapacity));
trimIndicesToCapacity(newCapacity);
return this;
}
}

View File

@ -141,35 +141,23 @@ public class UnpooledDirectByteBuf extends AbstractReferenceCountedByteBuf {
@Override
public ByteBuf capacity(int newCapacity) {
checkNewCapacity(newCapacity);
int readerIndex = readerIndex();
int writerIndex = writerIndex();
int oldCapacity = capacity;
if (newCapacity > oldCapacity) {
ByteBuffer oldBuffer = buffer;
ByteBuffer newBuffer = allocateDirect(newCapacity);
oldBuffer.position(0).limit(oldBuffer.capacity());
newBuffer.position(0).limit(oldBuffer.capacity());
newBuffer.put(oldBuffer);
newBuffer.clear();
setByteBuffer(newBuffer, true);
} else if (newCapacity < oldCapacity) {
ByteBuffer oldBuffer = buffer;
ByteBuffer newBuffer = allocateDirect(newCapacity);
if (readerIndex < newCapacity) {
if (writerIndex > newCapacity) {
writerIndex(writerIndex = newCapacity);
}
oldBuffer.position(readerIndex).limit(writerIndex);
newBuffer.position(readerIndex).limit(writerIndex);
newBuffer.put(oldBuffer);
newBuffer.clear();
} else {
setIndex(newCapacity, newCapacity);
}
setByteBuffer(newBuffer, true);
if (newCapacity == oldCapacity) {
return this;
}
int bytesToCopy;
if (newCapacity > oldCapacity) {
bytesToCopy = oldCapacity;
} else {
trimIndicesToCapacity(newCapacity);
bytesToCopy = newCapacity;
}
ByteBuffer oldBuffer = buffer;
ByteBuffer newBuffer = allocateDirect(newCapacity);
oldBuffer.position(0).limit(bytesToCopy);
newBuffer.position(0).limit(bytesToCopy);
newBuffer.put(oldBuffer).clear();
setByteBuffer(newBuffer, true);
return this;
}

View File

@ -120,29 +120,23 @@ public class UnpooledHeapByteBuf extends AbstractReferenceCountedByteBuf {
@Override
public ByteBuf capacity(int newCapacity) {
checkNewCapacity(newCapacity);
int oldCapacity = array.length;
byte[] oldArray = array;
if (newCapacity > oldCapacity) {
byte[] newArray = allocateArray(newCapacity);
System.arraycopy(oldArray, 0, newArray, 0, oldArray.length);
setArray(newArray);
freeArray(oldArray);
} else if (newCapacity < oldCapacity) {
byte[] newArray = allocateArray(newCapacity);
int readerIndex = readerIndex();
if (readerIndex < newCapacity) {
int writerIndex = writerIndex();
if (writerIndex > newCapacity) {
writerIndex(writerIndex = newCapacity);
}
System.arraycopy(oldArray, readerIndex, newArray, readerIndex, writerIndex - readerIndex);
} else {
setIndex(newCapacity, newCapacity);
}
setArray(newArray);
freeArray(oldArray);
int oldCapacity = oldArray.length;
if (newCapacity == oldCapacity) {
return this;
}
int bytesToCopy;
if (newCapacity > oldCapacity) {
bytesToCopy = oldCapacity;
} else {
trimIndicesToCapacity(newCapacity);
bytesToCopy = newCapacity;
}
byte[] newArray = allocateArray(newCapacity);
System.arraycopy(oldArray, 0, newArray, 0, bytesToCopy);
setArray(newArray);
freeArray(oldArray);
return this;
}

View File

@ -48,18 +48,8 @@ class UnpooledUnsafeNoCleanerDirectByteBuf extends UnpooledUnsafeDirectByteBuf {
return this;
}
ByteBuffer newBuffer = reallocateDirect(buffer, newCapacity);
if (newCapacity < oldCapacity) {
if (readerIndex() < newCapacity) {
if (writerIndex() > newCapacity) {
writerIndex(newCapacity);
}
} else {
setIndex(newCapacity, newCapacity);
}
}
setByteBuffer(newBuffer, false);
trimIndicesToCapacity(newCapacity);
setByteBuffer(reallocateDirect(buffer, newCapacity), false);
return this;
}
}