Correctly calculate the elementSize when cache alignment is configured (#11987)

normanmaurer · web-flow · commit ef69732a42d0 · 2022-01-11T08:55:07.000+01:00
Motivation: We need to take the cache alignment into account when we pre-calculate the ementSize otherwise we fail later. Modifications: - Fix pre-calculation - Add unit test - Fix incorrect unit test Result: Fixes #11955
diff --git a/buffer/src/main/java/io/netty/buffer/PoolArena.java b/buffer/src/main/java/io/netty/buffer/PoolArena.java
@@ -557,9 +557,9 @@ private void destroyPoolChunkLists(PoolChunkList<T>... chunkLists) {
     static final class HeapArena extends PoolArena<byte[]> {
 
         HeapArena(PooledByteBufAllocator parent, int pageSize, int pageShifts,
-                  int chunkSize, int directMemoryCacheAlignment) {
+                  int chunkSize) {
             super(parent, pageSize, pageShifts, chunkSize,
-                  directMemoryCacheAlignment);
+                  0);
         }
 
         private static byte[] newByteArray(int size) {
diff --git a/buffer/src/main/java/io/netty/buffer/PoolChunk.java b/buffer/src/main/java/io/netty/buffer/PoolChunk.java
@@ -571,7 +571,7 @@ void initBufWithSubpage(PooledByteBuf<T> buf, ByteBuffer nioBuffer, long handle,
 
         PoolSubpage<T> s = subpages[runOffset];
         assert s.doNotDestroy;
-        assert reqCapacity <= s.elemSize;
+        assert reqCapacity <= s.elemSize : reqCapacity + "<=" + s.elemSize;
 
         int offset = (runOffset << pageShifts) + bitmapIdx * s.elemSize;
         buf.init(this, nioBuffer, handle, offset, reqCapacity, s.elemSize, threadCache);
diff --git a/buffer/src/main/java/io/netty/buffer/PoolSubpage.java b/buffer/src/main/java/io/netty/buffer/PoolSubpage.java
@@ -25,6 +25,7 @@
 final class PoolSubpage<T> implements PoolSubpageMetric {
 
     final PoolChunk<T> chunk;
+    final int elemSize;
     private final int pageShifts;
     private final int runOffset;
     private final int runSize;
@@ -34,7 +35,6 @@ final class PoolSubpage<T> implements PoolSubpageMetric {
     PoolSubpage<T> next;
 
     boolean doNotDestroy;
-    int elemSize;
     private int maxNumElems;
     private int bitmapLength;
     private int nextAvail;
diff --git a/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java b/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java
@@ -300,8 +300,7 @@ public PooledByteBufAllocator(boolean preferDirect, int nHeapArena, int nDirectA
             List<PoolArenaMetric> metrics = new ArrayList<PoolArenaMetric>(heapArenas.length);
             for (int i = 0; i < heapArenas.length; i ++) {
                 PoolArena.HeapArena arena = new PoolArena.HeapArena(this,
-                        pageSize, pageShifts, chunkSize,
-                        directMemoryCacheAlignment);
+                        pageSize, pageShifts, chunkSize);
                 heapArenas[i] = arena;
                 metrics.add(arena);
             }
diff --git a/buffer/src/main/java/io/netty/buffer/SizeClasses.java b/buffer/src/main/java/io/netty/buffer/SizeClasses.java
@@ -237,6 +237,12 @@ private void idx2SizeTab(int[] sizeIdx2sizeTab, int[] pageIdx2sizeTab) {
             int nDelta = sizeClass[NDELTA_IDX];
 
             int size = (1 << log2Group) + (nDelta << log2Delta);
+            if (directMemoryCacheAlignment > 0) {
+                // We need to ensure we align the size before storing it as otherwise we will use the incorrect element
+                // size when creating the PoolSubPage
+                size = alignSize(size);
+            }
+
             sizeIdx2sizeTab[i] = size;
 
             if (sizeClass[PAGESIZE_IDX] == yes) {
diff --git a/buffer/src/test/java/io/netty/buffer/AlignedPooledByteBufAllocatorTest.java b/buffer/src/test/java/io/netty/buffer/AlignedPooledByteBufAllocatorTest.java
@@ -15,6 +15,11 @@
  */
 package io.netty.buffer;
 
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 public class AlignedPooledByteBufAllocatorTest extends PooledByteBufAllocatorTest {
     @Override
     protected PooledByteBufAllocator newAllocator(boolean preferDirect) {
@@ -30,4 +35,32 @@ protected PooledByteBufAllocator newAllocator(boolean preferDirect) {
                 PooledByteBufAllocator.defaultUseCacheForAllThreads(),
                 directMemoryCacheAlignment);
     }
+
+    // https://github.com/netty/netty/issues/11955
+    @Test
+    public void testCorrectElementSize() {
+        ByteBufAllocator allocator = new PooledByteBufAllocator(
+                true,
+                PooledByteBufAllocator.defaultNumHeapArena(),
+                PooledByteBufAllocator.defaultNumDirectArena(),
+                PooledByteBufAllocator.defaultPageSize(),
+                11,
+                PooledByteBufAllocator.defaultSmallCacheSize(),
+                64,
+                PooledByteBufAllocator.defaultUseCacheForAllThreads(),
+                64);
+
+        ByteBuf a = allocator.directBuffer(0, 16384);
+        ByteBuf b = allocator.directBuffer(0, 16384);
+        a.capacity(16);
+        assertEquals(16, a.capacity());
+        b.capacity(16);
+        assertEquals(16, b.capacity());
+        a.capacity(17);
+        assertEquals(17, a.capacity());
+        b.capacity(18);
+        assertEquals(18, b.capacity());
+        assertTrue(a.release());
+        assertTrue(b.release());
+    }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/PoolArenaTest.java b/buffer/src/test/java/io/netty/buffer/PoolArenaTest.java
@@ -45,7 +45,7 @@ public void testNormalizeCapacity() {
     public void testNormalizeAlignedCapacity() {
         PoolArena<ByteBuffer> arena = new PoolArena.DirectArena(null, PAGE_SIZE, PAGE_SHIFTS, CHUNK_SIZE, 64);
         int[] reqCapacities = {0, 15, 510, 1024, 1023, 1025};
-        int[] expectedResult = {16, 64, 512, 1024, 1024, 1280};
+        int[] expectedResult = {64, 64, 512, 1024, 1024, 1280};
         for (int i = 0; i < reqCapacities.length; i ++) {
             assertEquals(expectedResult[i], arena.sizeIdx2size(arena.size2SizeIdx(reqCapacities[i])));
         }

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ public void testNormalizeCapacity() {`
`45`	`45`	`public void testNormalizeAlignedCapacity() {`
`46`	`46`	`PoolArena<ByteBuffer> arena = new PoolArena.DirectArena(null, PAGE_SIZE, PAGE_SHIFTS, CHUNK_SIZE, 64);`
`47`	`47`	`int[] reqCapacities = {0, 15, 510, 1024, 1023, 1025};`
`48`		`- int[] expectedResult = {16, 64, 512, 1024, 1024, 1280};`
	`48`	`+ int[] expectedResult = {64, 64, 512, 1024, 1024, 1280};`
`49`	`49`	`for (int i = 0; i < reqCapacities.length; i ++) {`
`50`	`50`	`assertEquals(expectedResult[i], arena.sizeIdx2size(arena.size2SizeIdx(reqCapacities[i])));`
`51`	`51`	`}`