From c1c115399882c3b36cf3e3bc9ee62a80cde38e96 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 14 Jan 2025 12:01:06 +0100 Subject: [PATCH 01/11] Un-deprecate quadratic pool + made it respect minCapacity + add heuristic for factor Signed-off-by: Ludovic Orban --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 12 ++-- .../jetty/io/ArrayByteBufferPoolTest.java | 67 +++++++++---------- 2 files changed, 35 insertions(+), 44 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 8d44072014c..e07e7aea6d8 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -37,6 +37,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.ConcurrentPool; import org.eclipse.jetty.util.Pool; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; @@ -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(4096, 65536, Integer.MAX_VALUE); } public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize) @@ -742,13 +740,13 @@ public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize) public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize, long maxHeapMemory, long maxDirectMemory) { super(minCapacity, - -1, + minCapacity <= 0 ? 1 : minCapacity, maxCapacity, maxBucketSize, maxHeapMemory, maxDirectMemory, - c -> 32 - Integer.numberOfLeadingZeros(c - 1), - i -> 1 << i + c -> 32 - Integer.numberOfLeadingZeros(c - 1) - Integer.numberOfTrailingZeros(Integer.highestOneBit(TypeUtil.ceilToNextPowerOfTwo(minCapacity))), + i -> 1 << i + Integer.numberOfTrailingZeros(Integer.highestOneBit(TypeUtil.ceilToNextPowerOfTwo(minCapacity))) ); } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index cc228c08109..62824e22353 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -391,44 +391,37 @@ 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=5\n")); + 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(0, 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 From 62225cb27d6a14d67fa12b94a127eee3d4abf956 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 23 Jan 2025 14:48:23 +0100 Subject: [PATCH 02/11] handle review comments Signed-off-by: Ludovic Orban --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 34 ++++++++++++++++--- .../jetty/io/ArrayByteBufferPoolTest.java | 6 ++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index e07e7aea6d8..490b7857e39 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -729,7 +729,7 @@ public static class Quadratic extends ArrayByteBufferPool { public Quadratic() { - this(4096, 65536, Integer.MAX_VALUE); + this(-1, -1, Integer.MAX_VALUE); } public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize) @@ -740,15 +740,39 @@ public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize) public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize, long maxHeapMemory, long maxDirectMemory) { super(minCapacity, - minCapacity <= 0 ? 1 : minCapacity, - maxCapacity, + computeMinCapacity(minCapacity), + computeMaxCapacity(maxCapacity), maxBucketSize, maxHeapMemory, maxDirectMemory, - c -> 32 - Integer.numberOfLeadingZeros(c - 1) - Integer.numberOfTrailingZeros(Integer.highestOneBit(TypeUtil.ceilToNextPowerOfTwo(minCapacity))), - i -> 1 << i + Integer.numberOfTrailingZeros(Integer.highestOneBit(TypeUtil.ceilToNextPowerOfTwo(minCapacity))) + // 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) - powerOfTwo(computeMinCapacity(minCapacity)), + i -> 1 << i + powerOfTwo(computeMinCapacity(minCapacity)) ); } + + private static int computeMinCapacity(int minCapacity) + { + return minCapacity <= 0 ? 1024 : minCapacity; + } + + private static int computeMaxCapacity(int maxCapacity) + { + return maxCapacity <= 0 ? 65536 : maxCapacity; + } + + /** + * Computes the power of two 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. + */ + private static int powerOfTwo(int val) + { + return Integer.numberOfTrailingZeros(Integer.highestOneBit(TypeUtil.ceilToNextPowerOfTwo(val))); + } } /** diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index 62824e22353..dcdc7bf53b7 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -395,7 +395,9 @@ public void testQuadraticPoolBucketSizes() { ArrayByteBufferPool pool1 = new ArrayByteBufferPool.Quadratic(); String dump1 = pool1.dump(); - assertThat(dump1, containsString("direct size=5\n")); + 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,")); @@ -410,7 +412,7 @@ public void testQuadraticPoolBucketSizes() assertThat(dump2, containsString("{capacity=512,")); assertThat(dump2, containsString("{capacity=800,")); - ArrayByteBufferPool pool3 = new ArrayByteBufferPool.Quadratic(0, 200, Integer.MAX_VALUE); + 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,")); From 357fae63fbb2945d235fa5b52749570cfa0a7fd5 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 23 Jan 2025 18:43:07 +0100 Subject: [PATCH 03/11] handle review comments + move testCeilNextPowerOfTwo to MathUtils Signed-off-by: Ludovic Orban --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 17 ++----- .../jetty/io/ByteBufferAggregator.java | 4 +- .../org/eclipse/jetty/util/MathUtils.java | 26 +++++++++++ .../java/org/eclipse/jetty/util/TypeUtil.java | 13 ------ .../util/thread/ReservedThreadExecutor.java | 4 +- .../jetty/util/thread/ThreadIdPool.java | 4 +- .../org/eclipse/jetty/util/MathUtilTest.java | 46 +++++++++++++++++++ .../org/eclipse/jetty/util/TypeUtilTest.java | 14 ------ 8 files changed, 81 insertions(+), 47 deletions(-) create mode 100644 jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 490b7857e39..d4290b62929 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -36,8 +36,8 @@ 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.TypeUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; @@ -748,8 +748,8 @@ public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize, long maxHe // 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) - powerOfTwo(computeMinCapacity(minCapacity)), - i -> 1 << i + powerOfTwo(computeMinCapacity(minCapacity)) + c -> Integer.SIZE - Integer.numberOfLeadingZeros(c - 1) - MathUtils.log2Ceiled(computeMinCapacity(minCapacity)), + i -> 1 << i + MathUtils.log2Ceiled(computeMinCapacity(minCapacity)) ); } @@ -762,17 +762,6 @@ private static int computeMaxCapacity(int maxCapacity) { return maxCapacity <= 0 ? 65536 : maxCapacity; } - - /** - * Computes the power of two 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. - */ - private static int powerOfTwo(int val) - { - return Integer.numberOfTrailingZeros(Integer.highestOneBit(TypeUtil.ceilToNextPowerOfTwo(val))); - } } /** diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java index 3469c367ee5..38ded84aef9 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java +++ b/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) { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java index 5c71b5459b4..eb1006fbf92 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java +++ b/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))); + } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 995f5107285..9727b79ace5 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/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 diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java index 8a0cbeddb8a..ce24bb6d196 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java +++ b/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; } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadIdPool.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadIdPool.java index 46b0756f18a..93b27e3c16b 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadIdPool.java +++ b/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) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java new file mode 100644 index 00000000000..cb66856ec0d --- /dev/null +++ b/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)); + } +} diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java index c57c079c937..8c82772fea3 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java +++ b/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) From 0aa5474de75b9e2a42fa4daecdcf1109baf0898a Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Fri, 24 Jan 2025 10:59:24 +0100 Subject: [PATCH 04/11] handle review comments Signed-off-by: Ludovic Orban --- .../org/eclipse/jetty/io/ArrayByteBufferPool.java | 4 ++-- .../main/java/org/eclipse/jetty/util/MathUtils.java | 2 +- .../java/org/eclipse/jetty/util/MathUtilTest.java | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index d4290b62929..be96dad9442 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -748,8 +748,8 @@ public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize, long maxHe // 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)) + c -> Integer.SIZE - Integer.numberOfLeadingZeros(c - 1) - MathUtils.ceilLog2(computeMinCapacity(minCapacity)), + i -> 1 << i + MathUtils.ceilLog2(computeMinCapacity(minCapacity)) ); } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java index eb1006fbf92..e48f3623faa 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java @@ -99,7 +99,7 @@ public static int ceilToNextPowerOfTwo(int value) * @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) + public static int ceilLog2(int value) { return Integer.numberOfTrailingZeros(Integer.highestOneBit(ceilToNextPowerOfTwo(value))); } diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java index cb66856ec0d..0bb1e0f1a67 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java @@ -35,12 +35,12 @@ public void testCeilNextPowerOfTwo() } @Test - public void testLog2Ceiled() + public void testCeilLog2() { - 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)); + assertThrows(IllegalArgumentException.class, () -> MathUtils.ceilLog2(-1)); + assertThat(MathUtils.ceilLog2(0), is(0)); + assertThat(MathUtils.ceilLog2(800), is(10)); + assertThat(MathUtils.ceilLog2(1024), is(10)); + assertThat(MathUtils.ceilLog2(Integer.MAX_VALUE), is(30)); } } From 0db8cd5fdcfca687f0aed8be7a1c0234e789da8b Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Fri, 24 Jan 2025 16:23:40 +0100 Subject: [PATCH 05/11] use correct name for test Signed-off-by: Ludovic Orban --- .../jetty/util/{MathUtilTest.java => MathUtilsTest.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/{MathUtilTest.java => MathUtilsTest.java} (98%) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilsTest.java similarity index 98% rename from jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java rename to jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilsTest.java index 0bb1e0f1a67..9239cb9351d 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/MathUtilsTest.java @@ -19,7 +19,7 @@ import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertThrows; -public class MathUtilTest +public class MathUtilsTest { @Test public void testCeilNextPowerOfTwo() From c4919366afb7be23cae7ba7903ca778184bc5bed Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 28 Jan 2025 10:34:51 +0100 Subject: [PATCH 06/11] use heuristic by default Signed-off-by: Ludovic Orban --- .../src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index be96dad9442..c398aaec43d 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -734,7 +734,7 @@ public Quadratic() public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize) { - this(minCapacity, maxCapacity, maxBucketSize, -1L, -1L); + this(minCapacity, maxCapacity, maxBucketSize, 0L, 0L); } public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize, long maxHeapMemory, long maxDirectMemory) From 2f7e4c5adea3a79ecfd629bac9aa7f9c943657b1 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 28 Jan 2025 10:36:53 +0100 Subject: [PATCH 07/11] add Predefined buffer pool Signed-off-by: Ludovic Orban --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 56 +++++++++++++++++++ .../jetty/io/ArrayByteBufferPoolTest.java | 56 +++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index c398aaec43d..87ac5e8f7c4 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -720,6 +720,62 @@ protected void acquire() } } + /** + * A variant of the {@link ArrayByteBufferPool} that + * uses a predefined set of buckets of buffers. + */ + public static class Predefined extends ArrayByteBufferPool + { + public Predefined(int... capacities) + { + this(0L, 0L, capacities); + } + + public Predefined(long maxHeapMemory, long maxDirectMemory, int... capacities) + { + super(sort(capacities)[0], 1, capacities[capacities.length - 1], Integer.MAX_VALUE, maxHeapMemory, maxDirectMemory, + c -> indexOfClosestBelow(c, capacities), i -> capacityOfIndex(i, capacities)); + } + + private static int[] sort(int... values) + { + if (values.length == 0) + throw new IllegalArgumentException("At least one capacity is needed"); + Arrays.sort(values); + return values; + } + + private static int capacityOfIndex(int idx, int... capacities) + { + if (idx >= capacities.length) + { + int largestCapacity = capacities[capacities.length - 1]; + return (idx - capacities.length + 2) * largestCapacity; + } + return capacities[idx]; + } + + private static int indexOfClosestBelow(int capacity, int... capacities) + { + int largestCapacity = capacities[capacities.length - 1]; + if (capacity > largestCapacity) + { + int remainder = capacity % largestCapacity != 0 ? 1 : 0; + return capacity / largestCapacity - 1 + remainder + capacities.length - 1; + } + + int previous = -1; + for (int i = 0; i < capacities.length; i++) + { + int cap = capacities[i]; + if (cap > capacity) + break; + previous = i; + } + return previous; + } + } + /** * A variant of the {@link ArrayByteBufferPool} that * uses buckets of buffers that increase in size by a power of diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index dcdc7bf53b7..cf3d71bb9cd 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -426,6 +426,62 @@ public void testQuadraticPoolBucketSizes() assertThat(dump3, containsString("{capacity=200,")); } + @Test + public void testPredefinedPoolBucketSizes() + { + { + ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(1024, 65536); + String dump = pool.dump(); + assertThat(dump, containsString("direct size=2\n")); + assertThat(dump, containsString("{capacity=1024,")); + assertThat(dump, containsString("{capacity=65536,")); + } + { + ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(30, 24); + String dump = pool.dump(); + assertThat(dump, containsString("direct size=2\n")); + assertThat(dump, containsString("{capacity=24,")); + assertThat(dump, containsString("{capacity=30,")); + } + { + ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(3, 7, 100); + String dump = pool.dump(); + assertThat(dump, containsString("direct size=3\n")); + assertThat(dump, containsString("{capacity=3,")); + assertThat(dump, containsString("{capacity=7,")); + assertThat(dump, containsString("{capacity=100,")); + } + } + + @Test + public void testPredefinedPoolNoBucketSizes() + { + { + ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(1, 100); + pool.setStatisticsEnabled(true); + pool.acquire(200, false).release(); + pool.acquire(300, false).release(); + pool.acquire(800, false).release(); + pool.acquire(150, false).release(); + String dump = pool.dump(); + assertThat(dump, containsString("200: 2\n")); + assertThat(dump, containsString("300: 1\n")); + assertThat(dump, containsString("800: 1\n")); + } + { + ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(1, 7, 50, 100); + pool.setStatisticsEnabled(true); + pool.acquire(200, false).release(); + pool.acquire(300, false).release(); + pool.acquire(800, false).release(); + pool.acquire(150, false).release(); + String dump = pool.dump(); + assertThat(dump, containsString("200: 2\n")); + assertThat(dump, containsString("300: 1\n")); + assertThat(dump, containsString("800: 1\n")); + } + } + @Test public void testEndiannessResetOnRelease() { From 9672c699bb9f29fec19bac615074dec4b79de748 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 28 Jan 2025 15:09:48 +0100 Subject: [PATCH 08/11] handle review comments Signed-off-by: Ludovic Orban --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 18 +++++++++++----- .../jetty/io/ArrayByteBufferPoolTest.java | 21 ++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 87ac5e8f7c4..b25b7073057 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -724,17 +724,17 @@ protected void acquire() * A variant of the {@link ArrayByteBufferPool} that * uses a predefined set of buckets of buffers. */ - public static class Predefined extends ArrayByteBufferPool + public static class WithBucketCapacities extends ArrayByteBufferPool { - public Predefined(int... capacities) + public WithBucketCapacities(int... capacities) { this(0L, 0L, capacities); } - public Predefined(long maxHeapMemory, long maxDirectMemory, int... capacities) + public WithBucketCapacities(long maxHeapMemory, long maxDirectMemory, int... capacities) { super(sort(capacities)[0], 1, capacities[capacities.length - 1], Integer.MAX_VALUE, maxHeapMemory, maxDirectMemory, - c -> indexOfClosestBelow(c, capacities), i -> capacityOfIndex(i, capacities)); + c -> floorBucketIndexFor(c, capacities), i -> capacityOfIndex(i, capacities)); } private static int[] sort(int... values) @@ -749,17 +749,25 @@ private static int capacityOfIndex(int idx, int... capacities) { if (idx >= capacities.length) { + // An index over the capacities array's length is considered + // to refer to a multiple of the largest configured capacity; + // this logic is only meant for recordNoBucketAcquire(). int largestCapacity = capacities[capacities.length - 1]; return (idx - capacities.length + 2) * largestCapacity; } return capacities[idx]; } - private static int indexOfClosestBelow(int capacity, int... capacities) + private static int floorBucketIndexFor(int capacity, int... capacities) { int largestCapacity = capacities[capacities.length - 1]; if (capacity > largestCapacity) { + // A capacity over the largest configured capacity returns an + // index that corresponds to where in the capacities array it + // would stand if the latter had more entries that would all + // be multiples of the largest configured capacity; + // this logic is only meant for recordNoBucketAcquire(). int remainder = capacity % largestCapacity != 0 ? 1 : 0; return capacity / largestCapacity - 1 + remainder + capacities.length - 1; } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index cf3d71bb9cd..229958f54e4 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -427,24 +427,24 @@ public void testQuadraticPoolBucketSizes() } @Test - public void testPredefinedPoolBucketSizes() + public void testWithBucketCapacitiesBucketSizes() { { - ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(1024, 65536); + ArrayByteBufferPool pool = new ArrayByteBufferPool.WithBucketCapacities(1024, 65536); String dump = pool.dump(); assertThat(dump, containsString("direct size=2\n")); assertThat(dump, containsString("{capacity=1024,")); assertThat(dump, containsString("{capacity=65536,")); } { - ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(30, 24); + ArrayByteBufferPool pool = new ArrayByteBufferPool.WithBucketCapacities(30, 24); String dump = pool.dump(); assertThat(dump, containsString("direct size=2\n")); assertThat(dump, containsString("{capacity=24,")); assertThat(dump, containsString("{capacity=30,")); } { - ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(3, 7, 100); + ArrayByteBufferPool pool = new ArrayByteBufferPool.WithBucketCapacities(3, 7, 100); String dump = pool.dump(); assertThat(dump, containsString("direct size=3\n")); assertThat(dump, containsString("{capacity=3,")); @@ -454,10 +454,10 @@ public void testPredefinedPoolBucketSizes() } @Test - public void testPredefinedPoolNoBucketSizes() + public void testWithBucketCapacitiesNoBucketSizes() { { - ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(1, 100); + ArrayByteBufferPool pool = new ArrayByteBufferPool.WithBucketCapacities(1, 100); pool.setStatisticsEnabled(true); pool.acquire(200, false).release(); pool.acquire(300, false).release(); @@ -469,7 +469,7 @@ public void testPredefinedPoolNoBucketSizes() assertThat(dump, containsString("800: 1\n")); } { - ArrayByteBufferPool pool = new ArrayByteBufferPool.Predefined(1, 7, 50, 100); + ArrayByteBufferPool pool = new ArrayByteBufferPool.WithBucketCapacities(1, 7, 50, 100); pool.setStatisticsEnabled(true); pool.acquire(200, false).release(); pool.acquire(300, false).release(); @@ -480,6 +480,13 @@ public void testPredefinedPoolNoBucketSizes() assertThat(dump, containsString("300: 1\n")); assertThat(dump, containsString("800: 1\n")); } + { + ArrayByteBufferPool pool = new ArrayByteBufferPool.WithBucketCapacities(128, 512, 2048); + pool.setStatisticsEnabled(true); + pool.acquire(8192, false).release(); + String dump = pool.dump(); + assertThat(dump, containsString("8192: 1\n")); + } } @Test From d96dfe8a9c940a1a598d80c70c14dde2c98408e2 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 28 Jan 2025 18:25:25 +0100 Subject: [PATCH 09/11] handle review comments Signed-off-by: Ludovic Orban --- .../java/org/eclipse/jetty/io/ArrayByteBufferPool.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index b25b7073057..21cbd2bd92a 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -734,7 +734,7 @@ public WithBucketCapacities(int... capacities) public WithBucketCapacities(long maxHeapMemory, long maxDirectMemory, int... capacities) { super(sort(capacities)[0], 1, capacities[capacities.length - 1], Integer.MAX_VALUE, maxHeapMemory, maxDirectMemory, - c -> floorBucketIndexFor(c, capacities), i -> capacityOfIndex(i, capacities)); + c -> floorBucketIndexFor(c, capacities), i -> bucketCapacityForIndex(i, capacities)); } private static int[] sort(int... values) @@ -745,7 +745,7 @@ private static int[] sort(int... values) return values; } - private static int capacityOfIndex(int idx, int... capacities) + private static int bucketCapacityForIndex(int idx, int... capacities) { if (idx >= capacities.length) { @@ -753,7 +753,8 @@ private static int capacityOfIndex(int idx, int... capacities) // to refer to a multiple of the largest configured capacity; // this logic is only meant for recordNoBucketAcquire(). int largestCapacity = capacities[capacities.length - 1]; - return (idx - capacities.length + 2) * largestCapacity; + int virtualIdx = idx - (capacities.length - 1); + return (virtualIdx + 1) * largestCapacity; } return capacities[idx]; } From ef683843dc98a7d0251fe09be29edf51dd53f938 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 28 Jan 2025 18:40:19 +0100 Subject: [PATCH 10/11] handle review comments Signed-off-by: Ludovic Orban --- .../main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 21cbd2bd92a..23a30cace9c 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -770,7 +770,8 @@ private static int floorBucketIndexFor(int capacity, int... capacities) // be multiples of the largest configured capacity; // this logic is only meant for recordNoBucketAcquire(). int remainder = capacity % largestCapacity != 0 ? 1 : 0; - return capacity / largestCapacity - 1 + remainder + capacities.length - 1; + int overLargestCapacityFactor = (capacity / largestCapacity) + remainder; + return overLargestCapacityFactor - 1 + capacities.length - 1; } int previous = -1; From dad09cec51a773bd166097baaf93ba7d6929610d Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 29 Jan 2025 10:16:08 +0100 Subject: [PATCH 11/11] add size stat Signed-off-by: Ludovic Orban --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 31 ++++++++++++------- .../jetty/io/ArrayByteBufferPoolTest.java | 24 ++++++++++++++ 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 23a30cace9c..8c1d28b259e 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -19,6 +19,7 @@ import java.nio.ByteBuffer; import java.time.Instant; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import java.util.Objects; import java.util.Set; @@ -225,7 +226,7 @@ public RetainableByteBuffer.Mutable acquire(int size, boolean direct) return RetainableByteBuffer.wrap(BufferUtil.allocate(size, direct)); } - bucket.recordAcquire(); + bucket.recordAcquire(size); // Try to acquire a pooled entry. Pool.Entry entry = bucket.getPool().acquire(); @@ -504,6 +505,7 @@ public String toString() private class RetainedBucket { private final LongAdder _acquires = new LongAdder(); + private final LongAdder _totalAcquired = new LongAdder(); private final LongAdder _pooled = new LongAdder(); private final LongAdder _nonPooled = new LongAdder(); private final LongAdder _evicts = new LongAdder(); @@ -524,10 +526,13 @@ private RetainedBucket(int capacity, int poolSize) _capacity = capacity; } - public void recordAcquire() + public void recordAcquire(int size) { if (isStatisticsEnabled()) + { _acquires.increment(); + _totalAcquired.add(size); + } } public void recordEvict() @@ -590,6 +595,7 @@ private int evict() public void clear() { _acquires.reset(); + _totalAcquired.reset(); _pooled.reset(); _nonPooled.reset(); _evicts.reset(); @@ -603,8 +609,10 @@ public String toString() { int entries = 0; int inUse = 0; - for (Pool.Entry entry : getPool().stream().toList()) + Iterator> iterator = getPool().stream().iterator(); + while (iterator.hasNext()) { + Pool.Entry entry = iterator.next(); entries++; if (entry.isInUse()) inUse++; @@ -612,8 +620,9 @@ public String toString() long pooled = _pooled.longValue(); long acquires = _acquires.longValue(); - float hitRatio = acquires == 0 ? Float.NaN : pooled * 100F / acquires; - return String.format("%s{capacity=%d,in-use=%d/%d,pooled/acquires=%d/%d(%.3f%%),non-pooled/evicts/removes/releases=%d/%d/%d/%d}", + float hitRatio = acquires == 0L ? Float.NaN : pooled * 100F / acquires; + long avgSize = acquires == 0L ? 0L : _totalAcquired.longValue() / acquires; + return String.format("%s{capacity=%d,in-use=%d/%d,pooled/acquires=%d/%d(%.3f%%),avgsize=%d,non-pooled/evicts/removes/releases=%d/%d/%d/%d}", super.toString(), getCapacity(), inUse, @@ -621,6 +630,7 @@ public String toString() pooled, acquires, hitRatio, + avgSize, _nonPooled.longValue(), _evicts.longValue(), _removes.longValue(), @@ -733,7 +743,7 @@ public WithBucketCapacities(int... capacities) public WithBucketCapacities(long maxHeapMemory, long maxDirectMemory, int... capacities) { - super(sort(capacities)[0], 1, capacities[capacities.length - 1], Integer.MAX_VALUE, maxHeapMemory, maxDirectMemory, + super(-1, 1, sort(capacities)[capacities.length - 1], Integer.MAX_VALUE, maxHeapMemory, maxDirectMemory, c -> floorBucketIndexFor(c, capacities), i -> bucketCapacityForIndex(i, capacities)); } @@ -774,15 +784,14 @@ private static int floorBucketIndexFor(int capacity, int... capacities) return overLargestCapacityFactor - 1 + capacities.length - 1; } - int previous = -1; + int idx = 0; for (int i = 0; i < capacities.length; i++) { - int cap = capacities[i]; - if (cap > capacity) + idx = i; + if (capacities[i] > capacity) break; - previous = i; } - return previous; + return idx; } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index 229958f54e4..c7b78446ad2 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -489,6 +489,30 @@ public void testWithBucketCapacitiesNoBucketSizes() } } + @Test + public void testWithBucketCapacitiesStats() + { + ArrayByteBufferPool pool = new ArrayByteBufferPool.WithBucketCapacities(1024, 65536); + pool.setStatisticsEnabled(true); + for (int i = 0; i < 5; i++) + { + pool.acquire(1023, false).release(); + } + pool.acquire(2048, false).release(); + pool.acquire(4096, false).release(); + pool.acquire(4096, false).release(); + pool.acquire(6144, false).release(); + pool.acquire(65536 + 1, false).release(); + pool.acquire(65536 + 2, false).release(); + pool.acquire(65536 * 2 + 1, false).release(); + pool.acquire(65536 * 3 - 1, false).release(); + String dump = pool.dump(); + assertThat(dump, containsString("{capacity=1024,in-use=0/1,pooled/acquires=4/5(80.000%),avgsize=1023,non-pooled/evicts/removes/releases=0/0/0/5}")); + assertThat(dump, containsString("{capacity=65536,in-use=0/1,pooled/acquires=3/4(75.000%),avgsize=4096,non-pooled/evicts/removes/releases=0/0/0/4}")); + assertThat(dump, containsString("131072: 2\n")); + assertThat(dump, containsString("196608: 2\n")); + } + @Test public void testEndiannessResetOnRelease() {