Fix a bug in PoolArena and PoolSubpage where subpage pools are not updated correctly

- Make PoolSubpage a linked list node in the pool
- Now that a subpage is added to and removed from the pool correctly, allocating a subpage from the pool became vastly simpler.

buffer/src/main/java/io/netty/buffer/PoolArena.java

@@ -20,8 +20,6 @@ import io.netty.util.internal.PlatformDependent;
 import io.netty.util.internal.StringUtil;
 
 import java.nio.ByteBuffer;
-import java.util.ArrayDeque;
-import java.util.Deque;
 
 abstract class PoolArena<T> {
 
@@ -33,8 +31,8 @@ abstract class PoolArena<T> {
     private final int chunkSize;
     private final int subpageOverflowMask;
 
-    private final Deque<PoolSubpage<T>>[] tinySubpagePools;
+    private final PoolSubpage<T>[] tinySubpagePools;
-    private final Deque<PoolSubpage<T>>[] smallSubpagePools;
+    private final PoolSubpage<T>[] smallSubpagePools;
 
     private final PoolChunkList<T> q050;
     private final PoolChunkList<T> q025;
@@ -56,12 +54,12 @@ abstract class PoolArena<T> {
 
         tinySubpagePools = newSubpagePoolArray(512 >>> 4);
         for (int i = 0; i < tinySubpagePools.length; i ++) {
-            tinySubpagePools[i] = new ArrayDeque<PoolSubpage<T>>();
+            tinySubpagePools[i] = newSubpagePoolHead(pageSize);
         }
 
         smallSubpagePools = newSubpagePoolArray(pageShifts - 9);
         for (int i = 0; i < smallSubpagePools.length; i ++) {
-            smallSubpagePools[i] = new ArrayDeque<PoolSubpage<T>>();
+            smallSubpagePools[i] = newSubpagePoolHead(pageSize);
         }
 
         q100 = new PoolChunkList<T>(this, null, 100, Integer.MAX_VALUE);
@@ -79,9 +77,16 @@ abstract class PoolArena<T> {
         qInit.prevList = qInit;
     }
 
+    private PoolSubpage<T> newSubpagePoolHead(int pageSize) {
+        PoolSubpage<T> head = new PoolSubpage<T>(pageSize);
+        head.prev = head;
+        head.next = head;
+        return head;
+    }
+
     @SuppressWarnings("unchecked")
-    private Deque<PoolSubpage<T>>[] newSubpagePoolArray(int size) {
+    private PoolSubpage<T>[] newSubpagePoolArray(int size) {
-        return new Deque[size];
+        return new PoolSubpage[size];
     }
 
     PooledByteBuf<T> allocate(PoolThreadCache cache, int reqCapacity, int maxCapacity) {
@@ -94,7 +99,7 @@ abstract class PoolArena<T> {
         final int normCapacity = normalizeCapacity(reqCapacity);
         if ((normCapacity & subpageOverflowMask) == 0) { // capacity < pageSize
             int tableIdx;
-            Deque<PoolSubpage<T>>[] table;
+            PoolSubpage<T>[] table;
             if ((normCapacity & 0xFFFFFE00) == 0) { // < 512
                 tableIdx = normCapacity >>> 4;
                 table = tinySubpagePools;
@@ -109,26 +114,14 @@ abstract class PoolArena<T> {
             }
 
             synchronized (this) {
-                Deque<PoolSubpage<T>> subpages = table[tableIdx];
+                final PoolSubpage<T> head = table[tableIdx];
-                for (;;) {
+                final PoolSubpage<T> s = head.next;
-                    PoolSubpage<T> s = subpages.peekFirst();
+                if (s != head) {
-                    if (s == null) {
+                    assert s.doNotDestroy && s.elemSize == normCapacity;
-                        break;
-                    }
-
-                    if (!s.doNotDestroy || s.elemSize != normCapacity) {
-                        // The subpage has been destroyed or being used for different element size.
-                        subpages.removeFirst();
-                        continue;
-                    }
-
                     long handle = s.allocate();
-                    if (handle < 0) {
+                    assert handle >= 0;
-                        subpages.removeFirst();
+                    s.chunk.initBufWithSubpage(buf, handle, reqCapacity);
-                    } else {
+                    return;
-                        s.chunk.initBufWithSubpage(buf, handle, reqCapacity);
-                        return;
-                    }
                 }
             }
         } else if (normCapacity > chunkSize) {
@@ -142,7 +135,7 @@ abstract class PoolArena<T> {
     private synchronized void allocateNormal(PooledByteBuf<T> buf, int reqCapacity, int normCapacity) {
         if (q050.allocate(buf, reqCapacity, normCapacity) || q025.allocate(buf, reqCapacity, normCapacity) ||
             q000.allocate(buf, reqCapacity, normCapacity) || qInit.allocate(buf, reqCapacity, normCapacity) ||
-            q075.allocate(buf, reqCapacity, normCapacity)) {
+            q075.allocate(buf, reqCapacity, normCapacity) || q100.allocate(buf, reqCapacity, normCapacity)) {
             return;
         }
 
@@ -166,10 +159,9 @@ abstract class PoolArena<T> {
         }
     }
 
-    void addSubpage(PoolSubpage<T> subpage) {
+    PoolSubpage<T> findSubpagePoolHead(int elemSize) {
         int tableIdx;
-        int elemSize = subpage.elemSize;
+        PoolSubpage<T>[] table;
-        Deque<PoolSubpage<T>>[] table;
         if ((elemSize & 0xFFFFFE00) == 0) { // < 512
             tableIdx = elemSize >>> 4;
             table = tinySubpagePools;
@@ -183,7 +175,7 @@ abstract class PoolArena<T> {
             table = smallSubpagePools;
         }
 
-        table[tableIdx].addFirst(subpage);
+        return table[tableIdx];
     }
 
     private int normalizeCapacity(int reqCapacity) {
@@ -288,28 +280,42 @@ abstract class PoolArena<T> {
         buf.append(StringUtil.NEWLINE);
         buf.append("tiny subpages:");
         for (int i = 1; i < tinySubpagePools.length; i ++) {
-            Deque<PoolSubpage<T>> subpages = tinySubpagePools[i];
+            PoolSubpage<T> head = tinySubpagePools[i];
-            if (subpages.isEmpty()) {
+            if (head.next == head) {
                 continue;
             }
 
             buf.append(StringUtil.NEWLINE);
             buf.append(i);
             buf.append(": ");
-            buf.append(subpages);
+            PoolSubpage<T> s = head.next;
+            for (;;) {
+                buf.append(s);
+                s = s.next;
+                if (s == head) {
+                    break;
+                }
+            }
         }
         buf.append(StringUtil.NEWLINE);
         buf.append("small subpages:");
         for (int i = 1; i < smallSubpagePools.length; i ++) {
-            Deque<PoolSubpage<T>> subpages = smallSubpagePools[i];
+            PoolSubpage<T> head = smallSubpagePools[i];
-            if (subpages.isEmpty()) {
+            if (head.next == head) {
                 continue;
             }
 
             buf.append(StringUtil.NEWLINE);
             buf.append(i);
             buf.append(": ");
-            buf.append(subpages);
+            PoolSubpage<T> s = head.next;
+            for (;;) {
+                buf.append(s);
+                s = s.next;
+                if (s == head) {
+                    break;
+                }
+            }
         }
         buf.append(StringUtil.NEWLINE);
 

buffer/src/main/java/io/netty/buffer/PoolChunk.java

@@ -214,7 +214,6 @@ final class PoolChunk<T> {
                 } else {
                     subpage.init(normCapacity);
                 }
-                arena.addSubpage(subpage);
                 return subpage.allocate();
             }
 

buffer/src/main/java/io/netty/buffer/PoolSubpage.java

@@ -24,6 +24,9 @@ final class PoolSubpage<T> {
     final int pageSize;
     final long[] bitmap;
 
+    PoolSubpage<T> prev;
+    PoolSubpage<T> next;
+
     boolean doNotDestroy;
     int elemSize;
     int maxNumElems;
@@ -34,6 +37,16 @@ final class PoolSubpage<T> {
     // TODO: Test if adding padding helps under contention
     //private long pad0, pad1, pad2, pad3, pad4, pad5, pad6, pad7;
 
+    /** Special constructor that creates a linked list head */
+    PoolSubpage(int pageSize) {
+        chunk = null;
+        memoryMapIdx = -1;
+        runOffset = -1;
+        elemSize = -1;
+        this.pageSize = pageSize;
+        bitmap = null;
+    }
+
     PoolSubpage(PoolChunk<T> chunk, int memoryMapIdx, int runOffset, int pageSize, int elemSize) {
         this.chunk = chunk;
         this.memoryMapIdx = memoryMapIdx;
@@ -46,20 +59,20 @@ final class PoolSubpage<T> {
     void init(int elemSize) {
         doNotDestroy = true;
         this.elemSize = elemSize;
-        if (elemSize == 0) {
+        if (elemSize != 0) {
-            return;
+            maxNumElems = numAvail = pageSize / elemSize;
+            nextAvail = 0;
+            bitmapLength = maxNumElems >>> 6;
+            if ((maxNumElems & 63) != 0) {
+                bitmapLength ++;
+            }
+
+            for (int i = 0; i < bitmapLength; i ++) {
+                bitmap[i] = 0;
+            }
         }
 
-        maxNumElems = numAvail = pageSize / elemSize;
+        addToPool();
-        nextAvail = 0;
-        bitmapLength = maxNumElems >>> 6;
-        if ((maxNumElems & 63) != 0) {
-            bitmapLength ++;
-        }
-
-        for (int i = 0; i < bitmapLength; i ++) {
-            bitmap[i] = 0;
-        }
     }
 
     /**
@@ -81,6 +94,7 @@ final class PoolSubpage<T> {
         bitmap[q] |= 1L << r;
 
         if (-- numAvail == 0) {
+            removeFromPool();
             nextAvail = -1;
         } else {
             nextAvail = findNextAvailable();
@@ -105,7 +119,7 @@ final class PoolSubpage<T> {
 
         if (numAvail ++ == 0) {
             nextAvail = bitmapIdx;
-            chunk.arena.addSubpage(this);
+            addToPool();
             return true;
         }
 
@@ -113,10 +127,28 @@ final class PoolSubpage<T> {
             return true;
         } else {
             doNotDestroy = false;
+            removeFromPool();
             return false;
         }
     }
 
+    private void addToPool() {
+        PoolSubpage<T> head = chunk.arena.findSubpagePoolHead(elemSize);
+        assert prev == null && next == null;
+        prev = head;
+        next = head.next;
+        next.prev = this;
+        head.next = this;
+    }
+
+    private void removeFromPool() {
+        assert prev != null && next != null;
+        prev.next = next;
+        next.prev = prev;
+        next = null;
+        prev = null;
+    }
+
     private int findNextAvailable() {
         int newNextAvail = -1;
         loop: