Rehabilitate quadratic pool

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java

@@ -36,6 +36,7 @@
 import org.eclipse.jetty.io.internal.QueuedPool;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.ConcurrentPool;
+import org.eclipse.jetty.util.MathUtils;
 import org.eclipse.jetty.util.Pool;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
@@ -723,15 +724,12 @@ protected void acquire()
      * A variant of the {@link ArrayByteBufferPool} that
      * uses buckets of buffers that increase in size by a power of
      * 2 (e.g. 1k, 2k, 4k, 8k, etc.).
-     * @deprecated Usage of {@code Quadratic} is often wasteful of additional space and can increase contention on
-     * the larger buffers.
      */
-    @Deprecated(forRemoval = true, since = "12.1.0")
     public static class Quadratic extends ArrayByteBufferPool
     {
         public Quadratic()
         {
-            this(0, -1, Integer.MAX_VALUE);
+            this(-1, -1, Integer.MAX_VALUE);
         }
 
         public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize)
@@ -742,15 +740,28 @@ public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize)
         public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize, long maxHeapMemory, long maxDirectMemory)
         {
             super(minCapacity,
-                -1,
-                maxCapacity,
+                computeMinCapacity(minCapacity),
+                computeMaxCapacity(maxCapacity),
                 maxBucketSize,
                 maxHeapMemory,
                 maxDirectMemory,
-                c -> 32 - Integer.numberOfLeadingZeros(c - 1),
-                i -> 1 << i
+                // The bucket indices are the powers of 2, but those powers up to minCapacity are skipped so they must be
+                // substracted when computing the index and added when computing the capacity; so if minCapacity is 1024, any
+                // number from 0 to 1024 must return index 0, and index 0 must return capacity 1024.
+                c -> Integer.SIZE - Integer.numberOfLeadingZeros(c - 1) - MathUtils.log2Ceiled(computeMinCapacity(minCapacity)),
+                i -> 1 << i + MathUtils.log2Ceiled(computeMinCapacity(minCapacity))
             );
         }
+
+        private static int computeMinCapacity(int minCapacity)
+        {
+            return minCapacity <= 0 ? 1024 : minCapacity;
+        }
+
+        private static int computeMaxCapacity(int maxCapacity)
+        {
+            return maxCapacity <= 0 ? 65536 : maxCapacity;
+        }
     }
 
     /**

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java

@@ -16,7 +16,7 @@
 import java.nio.ByteBuffer;
 
 import org.eclipse.jetty.util.BufferUtil;
-import org.eclipse.jetty.util.TypeUtil;
+import org.eclipse.jetty.util.MathUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -106,7 +106,7 @@ private void tryExpandBufferCapacity(int remaining)
         if (remaining <= capacityLeft)
             return;
         int need = remaining - capacityLeft;
-        _currentSize = Math.min(_maxSize, TypeUtil.ceilToNextPowerOfTwo(_currentSize + need));
+        _currentSize = Math.min(_maxSize, MathUtils.ceilToNextPowerOfTwo(_currentSize + need));
 
         if (_retainableByteBuffer != null)
         {

jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java

@@ -391,44 +391,39 @@ public void testAcquireRelease()
     }
 
     @Test
-    @Deprecated(forRemoval = true)
-    public void testQuadraticPool()
+    public void testQuadraticPoolBucketSizes()
     {
-        ArrayByteBufferPool pool = new ArrayByteBufferPool.Quadratic();
-
-        RetainableByteBuffer retain5 = pool.acquire(5, false);
-        retain5.release();
-        RetainableByteBuffer retain6 = pool.acquire(6, false);
-        assertThat(retain6, not(sameInstance(retain5)));
-        assertThat(retain6.getByteBuffer(), sameInstance(retain5.getByteBuffer()));
-        retain6.release();
-        RetainableByteBuffer retain9 = pool.acquire(9, false);
-        assertThat(retain9, not(sameInstance(retain5)));
-        retain9.release();
-
-        assertThat(pool.acquire(1, false).capacity(), is(1));
-        assertThat(pool.acquire(2, false).capacity(), is(2));
-        RetainableByteBuffer b3 = pool.acquire(3, false);
-        assertThat(b3.capacity(), is(4));
-        RetainableByteBuffer b4 = pool.acquire(4, false);
-        assertThat(b4.capacity(), is(4));
-
-        int capacity = 4;
-        while (true)
-        {
-            RetainableByteBuffer b = pool.acquire(capacity - 1, false);
-            assertThat(b.capacity(), Matchers.is(capacity));
-            b = pool.acquire(capacity, false);
-            assertThat(b.capacity(), Matchers.is(capacity));
-
-            if (capacity >= pool.getMaxCapacity())
-                break;
-
-            b = pool.acquire(capacity + 1, false);
-            assertThat(b.capacity(), Matchers.is(capacity * 2));
-
-            capacity = capacity * 2;
-        }
+        ArrayByteBufferPool pool1 = new ArrayByteBufferPool.Quadratic();
+        String dump1 = pool1.dump();
+        assertThat(dump1, containsString("direct size=7\n"));
+        assertThat(dump1, containsString("{capacity=1024,"));
+        assertThat(dump1, containsString("{capacity=2048,"));
+        assertThat(dump1, containsString("{capacity=4096,"));
+        assertThat(dump1, containsString("{capacity=8192,"));
+        assertThat(dump1, containsString("{capacity=16384,"));
+        assertThat(dump1, containsString("{capacity=32768,"));
+        assertThat(dump1, containsString("{capacity=65536,"));
+
+        ArrayByteBufferPool pool2 = new ArrayByteBufferPool.Quadratic(100, 800, Integer.MAX_VALUE);
+        String dump2 = pool2.dump();
+        assertThat(dump2, containsString("direct size=4\n"));
+        assertThat(dump2, containsString("{capacity=128,"));
+        assertThat(dump2, containsString("{capacity=256,"));
+        assertThat(dump2, containsString("{capacity=512,"));
+        assertThat(dump2, containsString("{capacity=800,"));
+
+        ArrayByteBufferPool pool3 = new ArrayByteBufferPool.Quadratic(1, 200, Integer.MAX_VALUE);
+        String dump3 = pool3.dump();
+        assertThat(dump3, containsString("direct size=9\n"));
+        assertThat(dump3, containsString("{capacity=1,"));
+        assertThat(dump3, containsString("{capacity=2,"));
+        assertThat(dump3, containsString("{capacity=4,"));
+        assertThat(dump3, containsString("{capacity=8,"));
+        assertThat(dump3, containsString("{capacity=16,"));
+        assertThat(dump3, containsString("{capacity=32,"));
+        assertThat(dump3, containsString("{capacity=64,"));
+        assertThat(dump3, containsString("{capacity=128,"));
+        assertThat(dump3, containsString("{capacity=200,"));
     }
 
     @Test

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java

@@ -77,4 +77,30 @@ public static int cappedAdd(int a, int b, int maxValue)
             return maxValue;
         }
     }
+
+    /**
+     * Get the next highest power of two
+     * @param value An integer
+     * @return a power of two that is greater than or equal to {@code value}
+     */
+    public static int ceilToNextPowerOfTwo(int value)
+    {
+        if (value < 0)
+            throw new IllegalArgumentException("value must not be negative");
+        int result = 1 << (Integer.SIZE - Integer.numberOfLeadingZeros(value - 1));
+        return result > 0 ? result : Integer.MAX_VALUE;
+    }
+
+    /**
+     * Computes binary logarithm of the given number ceiled to the next power of two.
+     * If the given number is 800, it is ceiled to 1024 which is the closest power of 2 greater than or equal to 800,
+     * then 1024 is 10_000_000_000 in binary, i.e.: 1 followed by 10 zeros, and it's also 2 to the power of 10.
+     * So for the numbers between 513 and 1024 the returned value is 10.
+     * @param value An integer
+     * @return the binary logarithm of the given number ceiled to the next power of two.
+     */
+    public static int log2Ceiled(int value)
+    {
+        return Integer.numberOfTrailingZeros(Integer.highestOneBit(ceilToNextPowerOfTwo(value)));
+    }
 }

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java

@@ -832,19 +832,6 @@ private TypeUtil()
         // prevents instantiation
     }
 
-    /**
-     * Get the next highest power of two
-     * @param value An integer
-     * @return a power of two that is greater than or equal to {@code value}
-     */
-    public static int ceilToNextPowerOfTwo(int value)
-    {
-        if (value < 0)
-            throw new IllegalArgumentException("value must not be negative");
-        int result = 1 << (Integer.SIZE - Integer.numberOfLeadingZeros(value - 1));
-        return result > 0 ? result : Integer.MAX_VALUE;
-    }
-
     /**
      * Test is a method has been declared on the class of an instance
      * @param object The object to check

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java

@@ -20,8 +20,8 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.eclipse.jetty.util.MathUtils;
 import org.eclipse.jetty.util.ProcessorUtils;
-import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.VirtualThreads;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
@@ -120,7 +120,7 @@ public static int reservedThreads(Executor executor, int capacity)
         if (executor instanceof ThreadPool.SizedThreadPool)
         {
             int threads = ((ThreadPool.SizedThreadPool)executor).getMaxThreads();
-            return Math.max(1, TypeUtil.ceilToNextPowerOfTwo(Math.min(cpus, threads / 8)));
+            return Math.max(1, MathUtils.ceilToNextPowerOfTwo(Math.min(cpus, threads / 8)));
         }
         return cpus;
     }

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadIdPool.java

@@ -21,9 +21,9 @@
 import java.util.function.Function;
 import java.util.function.Supplier;
 
+import org.eclipse.jetty.util.MathUtils;
 import org.eclipse.jetty.util.MemoryUtils;
 import org.eclipse.jetty.util.ProcessorUtils;
-import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.component.Dumpable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -65,7 +65,7 @@ private static int calcCapacity(int capacity)
     {
         if (capacity >= 0)
             return capacity;
-        return 2 * TypeUtil.ceilToNextPowerOfTwo(ProcessorUtils.availableProcessors());
+        return 2 * MathUtils.ceilToNextPowerOfTwo(ProcessorUtils.availableProcessors());
     }
 
     private static int toSlot(int index)

jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java

@@ -0,0 +1,46 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.util;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class MathUtilTest
+{
+    @Test
+    public void testCeilNextPowerOfTwo()
+    {
+        assertThrows(IllegalArgumentException.class, () -> MathUtils.ceilToNextPowerOfTwo(-1));
+        assertThat(MathUtils.ceilToNextPowerOfTwo(0), is(1));
+        assertThat(MathUtils.ceilToNextPowerOfTwo(1), is(1));
+        assertThat(MathUtils.ceilToNextPowerOfTwo(2), is(2));
+        assertThat(MathUtils.ceilToNextPowerOfTwo(3), is(4));
+        assertThat(MathUtils.ceilToNextPowerOfTwo(4), is(4));
+        assertThat(MathUtils.ceilToNextPowerOfTwo(5), is(8));
+        assertThat(MathUtils.ceilToNextPowerOfTwo(Integer.MAX_VALUE - 1), is(Integer.MAX_VALUE));
+    }
+
+    @Test
+    public void testLog2Ceiled()
+    {
+        assertThrows(IllegalArgumentException.class, () -> MathUtils.log2Ceiled(-1));
+        assertThat(MathUtils.log2Ceiled(0), is(0));
+        assertThat(MathUtils.log2Ceiled(800), is(10));
+        assertThat(MathUtils.log2Ceiled(1024), is(10));
+        assertThat(MathUtils.log2Ceiled(Integer.MAX_VALUE), is(30));
+    }
+}

jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java

@@ -31,7 +31,6 @@
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
@@ -255,19 +254,6 @@ public void testToShortName(Class<?> clazz, String shortName)
         assertThat(TypeUtil.toShortName(clazz), is(shortName));
     }
 
-    @Test
-    public void testCeilNextPowerOfTwo()
-    {
-        assertThrows(IllegalArgumentException.class, () -> TypeUtil.ceilToNextPowerOfTwo(-1));
-        assertThat(TypeUtil.ceilToNextPowerOfTwo(0), is(1));
-        assertThat(TypeUtil.ceilToNextPowerOfTwo(1), is(1));
-        assertThat(TypeUtil.ceilToNextPowerOfTwo(2), is(2));
-        assertThat(TypeUtil.ceilToNextPowerOfTwo(3), is(4));
-        assertThat(TypeUtil.ceilToNextPowerOfTwo(4), is(4));
-        assertThat(TypeUtil.ceilToNextPowerOfTwo(5), is(8));
-        assertThat(TypeUtil.ceilToNextPowerOfTwo(Integer.MAX_VALUE - 1), is(Integer.MAX_VALUE));
-    }
-
     public static class Base
     {
         protected String methodA(String arg)