From 67da6e4bf945f771607454dff40334990396ce7b Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Wed, 19 Dec 2012 17:35:32 +0900 Subject: [PATCH] Remove the notion of ByteBufAllocator.bufferMaxCapacity() - Allocate the unpooled memory if the requested capacity is greater then the chunkSize - Fixes #834 --- .../buffer/AbstractByteBufAllocator.java | 31 ++++------- .../io/netty/buffer/ByteBufAllocator.java | 2 - .../main/java/io/netty/buffer/PoolArena.java | 31 +++++++++-- .../main/java/io/netty/buffer/PoolChunk.java | 20 ++++++- .../java/io/netty/buffer/PooledByteBuf.java | 52 +++++++++++++------ .../netty/buffer/PooledByteBufAllocator.java | 14 +++-- .../buffer/UnpooledByteBufAllocator.java | 2 +- 7 files changed, 103 insertions(+), 49 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java index 33c80feaf7..b51d4900ea 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java @@ -18,28 +18,18 @@ package io.netty.buffer; public abstract class AbstractByteBufAllocator implements ByteBufAllocator { - private final int bufferMaxCapacity; private final boolean directByDefault; private final ByteBuf emptyBuf; - protected AbstractByteBufAllocator(int bufferMaxCapacity) { - this(bufferMaxCapacity, false); + protected AbstractByteBufAllocator() { + this(false); } - protected AbstractByteBufAllocator(int bufferMaxCapacity, boolean directByDefault) { - if (bufferMaxCapacity <= 0) { - throw new IllegalArgumentException("bufferMaxCapacity: " + bufferMaxCapacity + " (expected: 1+)"); - } + protected AbstractByteBufAllocator(boolean directByDefault) { this.directByDefault = directByDefault; - this.bufferMaxCapacity = bufferMaxCapacity; emptyBuf = new UnpooledHeapByteBuf(this, 0, 0); } - @Override - public int bufferMaxCapacity() { - return bufferMaxCapacity; - } - @Override public ByteBuf buffer() { if (directByDefault) { @@ -66,12 +56,12 @@ public abstract class AbstractByteBufAllocator implements ByteBufAllocator { @Override public ByteBuf heapBuffer() { - return heapBuffer(256, bufferMaxCapacity()); + return heapBuffer(256, Integer.MAX_VALUE); } @Override public ByteBuf heapBuffer(int initialCapacity) { - return heapBuffer(initialCapacity, bufferMaxCapacity()); + return heapBuffer(initialCapacity, Integer.MAX_VALUE); } @Override @@ -85,12 +75,12 @@ public abstract class AbstractByteBufAllocator implements ByteBufAllocator { @Override public ByteBuf directBuffer() { - return directBuffer(256, bufferMaxCapacity()); + return directBuffer(256, Integer.MAX_VALUE); } @Override public ByteBuf directBuffer(int initialCapacity) { - return directBuffer(initialCapacity, bufferMaxCapacity()); + return directBuffer(initialCapacity, Integer.MAX_VALUE); } @Override @@ -138,10 +128,9 @@ public abstract class AbstractByteBufAllocator implements ByteBufAllocator { return new DefaultCompositeByteBuf(this, true, maxNumComponents); } - private void validate(int initialCapacity, int maxCapacity) { - if (maxCapacity > bufferMaxCapacity()) { - throw new IllegalArgumentException( - "maxCapacity: " + maxCapacity + " (expected: not greater than " + bufferMaxCapacity()); + private static void validate(int initialCapacity, int maxCapacity) { + if (initialCapacity < 0) { + throw new IllegalArgumentException("initialCapacity: " + initialCapacity + " (expectd: 0+)"); } if (initialCapacity > maxCapacity) { throw new IllegalArgumentException(String.format( diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufAllocator.java b/buffer/src/main/java/io/netty/buffer/ByteBufAllocator.java index d972948c6c..786bbb18f9 100644 --- a/buffer/src/main/java/io/netty/buffer/ByteBufAllocator.java +++ b/buffer/src/main/java/io/netty/buffer/ByteBufAllocator.java @@ -34,6 +34,4 @@ public interface ByteBufAllocator { CompositeByteBuf compositeHeapBuffer(int maxNumComponents); CompositeByteBuf compositeDirectBuffer(); CompositeByteBuf compositeDirectBuffer(int maxNumComponents); - - int bufferMaxCapacity(); } diff --git a/buffer/src/main/java/io/netty/buffer/PoolArena.java b/buffer/src/main/java/io/netty/buffer/PoolArena.java index 2eaa1e9e35..b8c9e01b00 100644 --- a/buffer/src/main/java/io/netty/buffer/PoolArena.java +++ b/buffer/src/main/java/io/netty/buffer/PoolArena.java @@ -130,6 +130,9 @@ abstract class PoolArena { } } } + } else if (normCapacity > chunkSize) { + allocateHuge(buf, reqCapacity); + return; } allocateNormal(buf, reqCapacity, normCapacity); @@ -150,8 +153,16 @@ abstract class PoolArena { qInit.add(c); } + private void allocateHuge(PooledByteBuf buf, int reqCapacity) { + buf.initUnpooled(newUnpooledChunk(reqCapacity), reqCapacity); + } + synchronized void free(PoolChunk chunk, long handle) { - chunk.parent.free(chunk, handle); + if (chunk.unpooled) { + destroyChunk(chunk); + } else { + chunk.parent.free(chunk, handle); + } } void addSubpage(PoolSubpage subpage) { @@ -175,8 +186,11 @@ abstract class PoolArena { } private int normalizeCapacity(int reqCapacity) { - if (reqCapacity < 0 || reqCapacity > chunkSize) { - throw new IllegalArgumentException("capacity: " + reqCapacity + " (expected: 0-" + chunkSize + ')'); + if (reqCapacity < 0) { + throw new IllegalArgumentException("capacity: " + reqCapacity + " (expected: 0+)"); + } + if (reqCapacity >= chunkSize) { + return reqCapacity; } if ((reqCapacity & 0xFFFFFE00) != 0) { // >= 512 @@ -240,6 +254,7 @@ abstract class PoolArena { } protected abstract PoolChunk newChunk(int pageSize, int maxOrder, int pageShifts, int chunkSize); + protected abstract PoolChunk newUnpooledChunk(int capacity); protected abstract PooledByteBuf newByteBuf(int maxCapacity); protected abstract void memoryCopy(T src, int srcOffset, T dst, int dstOffset, int length); protected abstract void destroyChunk(PoolChunk chunk); @@ -311,6 +326,11 @@ abstract class PoolArena { return new PoolChunk(this, new byte[chunkSize], pageSize, maxOrder, pageShifts, chunkSize); } + @Override + protected PoolChunk newUnpooledChunk(int capacity) { + return new PoolChunk(this, new byte[capacity], capacity); + } + @Override protected void destroyChunk(PoolChunk chunk) { // Rely on GC. @@ -343,6 +363,11 @@ abstract class PoolArena { this, ByteBuffer.allocateDirect(chunkSize), pageSize, maxOrder, pageShifts, chunkSize); } + @Override + protected PoolChunk newUnpooledChunk(int capacity) { + return new PoolChunk(this, ByteBuffer.allocateDirect(capacity), capacity); + } + @Override protected void destroyChunk(PoolChunk chunk) { UnpooledDirectByteBuf.freeDirect(chunk.memory); diff --git a/buffer/src/main/java/io/netty/buffer/PoolChunk.java b/buffer/src/main/java/io/netty/buffer/PoolChunk.java index 3f44f26892..17d29fdcbc 100644 --- a/buffer/src/main/java/io/netty/buffer/PoolChunk.java +++ b/buffer/src/main/java/io/netty/buffer/PoolChunk.java @@ -28,6 +28,7 @@ final class PoolChunk { final PoolArena arena; final T memory; + final boolean unpooled; private final int[] memoryMap; private final PoolSubpage[] subpages; @@ -51,6 +52,7 @@ final class PoolChunk { //private long pad0, pad1, pad2, pad3, pad4, pad5, pad6, pad7; PoolChunk(PoolArena arena, T memory, int pageSize, int maxOrder, int pageShifts, int chunkSize) { + unpooled = false; this.arena = arena; this.memory = memory; this.pageSize = pageSize; @@ -76,6 +78,20 @@ final class PoolChunk { subpages = newSubpageArray(maxSubpageAllocs); } + /** Creates a special chunk that is not pooled. */ + PoolChunk(PoolArena arena, T memory, int size) { + unpooled = true; + this.arena = arena; + this.memory = memory; + memoryMap = null; + subpages = null; + subpageOverflowMask = 0; + pageSize = 0; + pageShifts = 0; + chunkSize = size; + maxSubpageAllocs = 0; + } + @SuppressWarnings("unchecked") private PoolSubpage[] newSubpageArray(int size) { return new PoolSubpage[size]; @@ -266,7 +282,7 @@ final class PoolChunk { if (bitmapIdx == 0) { int val = memoryMap[memoryMapIdx]; assert (val & 3) == ST_ALLOCATED : String.valueOf(val & 3); - buf.init(this, handle, memory, runOffset(val), reqCapacity, runLength(val)); + buf.init(this, handle, runOffset(val), reqCapacity, runLength(val)); } else { initBufWithSubpage(buf, handle, bitmapIdx, reqCapacity); } @@ -288,7 +304,7 @@ final class PoolChunk { assert reqCapacity <= subpage.elemSize; buf.init( - this, handle, memory, + this, handle, runOffset(val) + (bitmapIdx & 0x3FFFFFFF) * subpage.elemSize, reqCapacity, subpage.elemSize); } diff --git a/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java index 3443d69092..287d748b84 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java @@ -37,13 +37,13 @@ abstract class PooledByteBuf extends AbstractByteBuf { super(maxCapacity); } - final void init(PoolChunk chunk, long handle, T memory, int offset, int length, int maxLength) { + final void init(PoolChunk chunk, long handle, int offset, int length, int maxLength) { assert handle >= 0; - assert memory != null; + assert chunk != null; this.chunk = chunk; this.handle = handle; - this.memory = memory; + memory = chunk.memory; this.offset = offset; this.length = length; this.maxLength = maxLength; @@ -51,6 +51,18 @@ abstract class PooledByteBuf extends AbstractByteBuf { tmpNioBuf = null; } + final void initUnpooled(PoolChunk chunk, int length) { + assert chunk != null; + + this.chunk = chunk; + handle = 0; + memory = chunk.memory; + offset = 0; + this.length = maxLength = length; + setIndex(0, 0); + tmpNioBuf = null; + } + @Override public final int capacity() { return length; @@ -61,27 +73,33 @@ abstract class PooledByteBuf extends AbstractByteBuf { checkUnfreed(); // If the request capacity does not require reallocation, just update the length of the memory. - if (newCapacity > length) { - if (newCapacity <= maxLength) { - length = newCapacity; + if (chunk.unpooled) { + if (newCapacity == length) { return this; } - } else if (newCapacity < length) { - if (newCapacity > maxLength >>> 1) { - if (maxLength <= 512) { - if (newCapacity > maxLength - 16) { + } else { + if (newCapacity > length) { + if (newCapacity <= maxLength) { + length = newCapacity; + return this; + } + } else if (newCapacity < length) { + if (newCapacity > maxLength >>> 1) { + if (maxLength <= 512) { + if (newCapacity > maxLength - 16) { + length = newCapacity; + setIndex(Math.min(readerIndex(), newCapacity), Math.min(writerIndex(), newCapacity)); + return this; + } + } else { // > 512 (i.e. >= 1024) length = newCapacity; setIndex(Math.min(readerIndex(), newCapacity), Math.min(writerIndex(), newCapacity)); return this; } - } else { // > 512 (i.e. >= 1024) - length = newCapacity; - setIndex(Math.min(readerIndex(), newCapacity), Math.min(writerIndex(), newCapacity)); - return this; } + } else { + return this; } - } else { - return this; } // Reallocation required. @@ -144,7 +162,7 @@ abstract class PooledByteBuf extends AbstractByteBuf { } for (Allocation a: suspendedDeallocations) { - chunk.arena.free(a.chunk, a.handle); + a.chunk.arena.free(a.chunk, a.handle); } return this; } diff --git a/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java b/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java index 9d68f109a6..eabdbd2f43 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java +++ b/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java @@ -59,7 +59,10 @@ public class PooledByteBufAllocator extends AbstractByteBufAllocator { public PooledByteBufAllocator( boolean directByDefault, int nHeapArena, int nDirectArena, int pageSize, int maxOrder) { - super(validateAndCalculateChunkSize(pageSize, maxOrder), directByDefault); + super(directByDefault); + + final int chunkSize = validateAndCalculateChunkSize(pageSize, maxOrder); + if (nHeapArena <= 0) { throw new IllegalArgumentException("nHeapArena: " + nHeapArena + " (expected: 1+)"); } @@ -68,7 +71,6 @@ public class PooledByteBufAllocator extends AbstractByteBufAllocator { } int pageShifts = validateAndCalculatePageShifts(pageSize); - int chunkSize = bufferMaxCapacity(); heapArenas = newArenaArray(nHeapArena); for (int i = 0; i < heapArenas.length; i ++) { @@ -147,11 +149,17 @@ public class PooledByteBufAllocator extends AbstractByteBufAllocator { public String toString() { StringBuilder buf = new StringBuilder(); buf.append(heapArenas.length); - buf.append(" arena(s):"); + buf.append(" heap arena(s):"); buf.append(StringUtil.NEWLINE); for (PoolArena a: heapArenas) { buf.append(a); } + buf.append(directArenas.length); + buf.append(" direct arena(s):"); + buf.append(StringUtil.NEWLINE); + for (PoolArena a: directArenas) { + buf.append(a); + } return buf.toString(); } } diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledByteBufAllocator.java b/buffer/src/main/java/io/netty/buffer/UnpooledByteBufAllocator.java index 894611658e..2ec014f679 100644 --- a/buffer/src/main/java/io/netty/buffer/UnpooledByteBufAllocator.java +++ b/buffer/src/main/java/io/netty/buffer/UnpooledByteBufAllocator.java @@ -26,7 +26,7 @@ public final class UnpooledByteBufAllocator extends AbstractByteBufAllocator { public static final UnpooledByteBufAllocator DIRECT_BY_DEFAULT = new UnpooledByteBufAllocator(true); private UnpooledByteBufAllocator(boolean directByDefault) { - super(Integer.MAX_VALUE, directByDefault); + super(directByDefault); } @Override