From f86a719bce89844337e4f2bde68e8e147095ed80 Mon Sep 17 00:00:00 2001
From: Lachlan
Date: Thu, 25 Nov 2021 10:09:10 +1100
Subject: [PATCH] Issue #6974 - improvements & fixes to ByteBufferPool
implementations (#7017)
- WebSocket should user server ByteBufferPool if possible
- fix various bugs ByteBufferPool implementations
- add heuristic for maxHeapMemory and maxDirectMemory
- Add dump for ByteBufferPools
- add LogArrayByteBufferPool that does exponential scaling of bucket size.
- ByteBufferPools should default to use maxMemory heuristic
- Add module jetty-bytebufferpool-logarithmic
Signed-off-by: Lachlan Roberts
Co-authored-by: Simone Bordet
---
.../jetty/io/AbstractByteBufferPool.java | 34 ++-
.../eclipse/jetty/io/ArrayByteBufferPool.java | 131 ++++++---
.../org/eclipse/jetty/io/ByteBufferPool.java | 70 +++--
.../io/LogarithmicArrayByteBufferPool.java | 112 ++++++++
.../jetty/io/MappedByteBufferPool.java | 96 +++++--
.../jetty/io/ArrayByteBufferPoolTest.java | 21 +-
.../jetty/io/MappedByteBufferPoolTest.java | 7 +-
.../etc/jetty-bytebufferpool-logarithmic.xml | 10 +
.../main/config/etc/jetty-bytebufferpool.xml | 4 +-
.../modules/bytebufferpool-logarithmic.mod | 30 +++
.../main/config/modules/bytebufferpool.mod | 11 +-
.../tests/WebSocketBufferPoolTest.java | 248 ++++++++++++++++++
.../server/WebSocketServerFactory.java | 22 +-
13 files changed, 671 insertions(+), 125 deletions(-)
create mode 100644 jetty-io/src/main/java/org/eclipse/jetty/io/LogarithmicArrayByteBufferPool.java
create mode 100644 jetty-server/src/main/config/etc/jetty-bytebufferpool-logarithmic.xml
create mode 100644 jetty-server/src/main/config/modules/bytebufferpool-logarithmic.mod
create mode 100644 jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketBufferPoolTest.java
diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractByteBufferPool.java
index c46d134dd8b..81b9a0bbe24 100644
--- a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractByteBufferPool.java
+++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractByteBufferPool.java
@@ -21,6 +21,7 @@ package org.eclipse.jetty.io;
import java.nio.ByteBuffer;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Consumer;
+import java.util.function.IntConsumer;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
@@ -32,16 +33,24 @@ abstract class AbstractByteBufferPool implements ByteBufferPool
private final int _factor;
private final int _maxQueueLength;
private final long _maxHeapMemory;
- private final AtomicLong _heapMemory = new AtomicLong();
private final long _maxDirectMemory;
+ private final AtomicLong _heapMemory = new AtomicLong();
private final AtomicLong _directMemory = new AtomicLong();
+ /**
+ * Creates a new ByteBufferPool with the given configuration.
+ *
+ * @param factor the capacity factor
+ * @param maxQueueLength the maximum ByteBuffer queue length
+ * @param maxHeapMemory the max heap memory in bytes, -1 for unlimited memory or 0 to use default heuristic.
+ * @param maxDirectMemory the max direct memory in bytes, -1 for unlimited memory or 0 to use default heuristic.
+ */
protected AbstractByteBufferPool(int factor, int maxQueueLength, long maxHeapMemory, long maxDirectMemory)
{
_factor = factor <= 0 ? 1024 : factor;
_maxQueueLength = maxQueueLength;
- _maxHeapMemory = maxHeapMemory;
- _maxDirectMemory = maxDirectMemory;
+ _maxHeapMemory = (maxHeapMemory != 0) ? maxHeapMemory : Runtime.getRuntime().maxMemory() / 4;
+ _maxDirectMemory = (maxDirectMemory != 0) ? maxDirectMemory : Runtime.getRuntime().maxMemory() / 4;
}
protected int getCapacityFactor()
@@ -54,11 +63,13 @@ abstract class AbstractByteBufferPool implements ByteBufferPool
return _maxQueueLength;
}
+ @Deprecated
protected void decrementMemory(ByteBuffer buffer)
{
updateMemory(buffer, false);
}
+ @Deprecated
protected void incrementMemory(ByteBuffer buffer)
{
updateMemory(buffer, true);
@@ -95,12 +106,29 @@ abstract class AbstractByteBufferPool implements ByteBufferPool
return getMemory(false);
}
+ @ManagedAttribute("The max num of bytes that can be retained from direct ByteBuffers")
+ public long getMaxDirectMemory()
+ {
+ return _maxDirectMemory;
+ }
+
+ @ManagedAttribute("The max num of bytes that can be retained from heap ByteBuffers")
+ public long getMaxHeapMemory()
+ {
+ return _maxHeapMemory;
+ }
+
public long getMemory(boolean direct)
{
AtomicLong memory = direct ? _directMemory : _heapMemory;
return memory.get();
}
+ IntConsumer updateMemory(boolean direct)
+ {
+ return (direct) ? _directMemory::addAndGet : _heapMemory::addAndGet;
+ }
+
@ManagedOperation(value = "Clears this ByteBufferPool", impact = "ACTION")
public void clear()
{
diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java
index 66d036e7e21..5d708d2f9e7 100644
--- a/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java
+++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java
@@ -18,14 +18,19 @@
package org.eclipse.jetty.io;
+import java.io.IOException;
import java.nio.ByteBuffer;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.List;
import java.util.Objects;
-import java.util.function.IntFunction;
+import java.util.stream.Collectors;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
+import org.eclipse.jetty.util.component.Dumpable;
+import org.eclipse.jetty.util.component.DumpableCollection;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
@@ -36,13 +41,15 @@ import org.eclipse.jetty.util.log.Logger;
* 2048, and so on.
*/
@ManagedObject
-public class ArrayByteBufferPool extends AbstractByteBufferPool
+public class ArrayByteBufferPool extends AbstractByteBufferPool implements Dumpable
{
private static final Logger LOG = Log.getLogger(MappedByteBufferPool.class);
+ private final int _maxCapacity;
private final int _minCapacity;
private final ByteBufferPool.Bucket[] _direct;
private final ByteBufferPool.Bucket[] _indirect;
+ private boolean _detailedDump = false;
/**
* Creates a new ArrayByteBufferPool with a default configuration.
@@ -61,7 +68,7 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
*/
public ArrayByteBufferPool(int minCapacity, int factor, int maxCapacity)
{
- this(minCapacity, factor, maxCapacity, -1, -1, -1);
+ this(minCapacity, factor, maxCapacity, -1, 0, 0);
}
/**
@@ -74,7 +81,7 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
*/
public ArrayByteBufferPool(int minCapacity, int factor, int maxCapacity, int maxQueueLength)
{
- this(minCapacity, factor, maxCapacity, maxQueueLength, -1, -1);
+ this(minCapacity, factor, maxCapacity, maxQueueLength, 0, 0);
}
/**
@@ -84,8 +91,8 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
* @param factor the capacity factor
* @param maxCapacity the maximum ByteBuffer capacity
* @param maxQueueLength the maximum ByteBuffer queue length
- * @param maxHeapMemory the max heap memory in bytes
- * @param maxDirectMemory the max direct memory in bytes
+ * @param maxHeapMemory the max heap memory in bytes, -1 for unlimited memory or 0 to use default heuristic.
+ * @param maxDirectMemory the max direct memory in bytes, -1 for unlimited memory or 0 to use default heuristic.
*/
public ArrayByteBufferPool(int minCapacity, int factor, int maxCapacity, int maxQueueLength, long maxHeapMemory, long maxDirectMemory)
{
@@ -98,24 +105,30 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
maxCapacity = 64 * 1024;
if ((maxCapacity % factor) != 0 || factor >= maxCapacity)
throw new IllegalArgumentException("The capacity factor must be a divisor of maxCapacity");
+ _maxCapacity = maxCapacity;
_minCapacity = minCapacity;
- int length = maxCapacity / factor;
+ // Initialize all buckets in constructor and never modify the array again.
+ int length = bucketFor(maxCapacity);
_direct = new ByteBufferPool.Bucket[length];
_indirect = new ByteBufferPool.Bucket[length];
+ for (int i = 0; i < length; i++)
+ {
+ _direct[i] = newBucket(i + 1, true);
+ _indirect[i] = newBucket(i + 1, false);
+ }
}
@Override
public ByteBuffer acquire(int size, boolean direct)
{
- int capacity = size < _minCapacity ? size : (bucketFor(size) + 1) * getCapacityFactor();
- ByteBufferPool.Bucket bucket = bucketFor(size, direct, null);
+ int capacity = size < _minCapacity ? size : capacityFor(bucketFor(size));
+ ByteBufferPool.Bucket bucket = bucketFor(size, direct);
if (bucket == null)
return newByteBuffer(capacity, direct);
ByteBuffer buffer = bucket.acquire();
if (buffer == null)
return newByteBuffer(capacity, direct);
- decrementMemory(buffer);
return buffer;
}
@@ -127,26 +140,29 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
int capacity = buffer.capacity();
// Validate that this buffer is from this pool.
- if ((capacity % getCapacityFactor()) != 0)
+ if (capacity != capacityFor(bucketFor(capacity)))
{
if (LOG.isDebugEnabled())
LOG.debug("ByteBuffer {} does not belong to this pool, discarding it", BufferUtil.toDetailString(buffer));
return;
}
+ // Don't release into the pool if greater than the maximum ByteBuffer capacity.
+ if (capacity > _maxCapacity)
+ return;
+
boolean direct = buffer.isDirect();
- ByteBufferPool.Bucket bucket = bucketFor(capacity, direct, this::newBucket);
+ ByteBufferPool.Bucket bucket = bucketFor(capacity, direct);
if (bucket != null)
{
bucket.release(buffer);
- incrementMemory(buffer);
- releaseExcessMemory(direct, this::clearOldestBucket);
+ releaseExcessMemory(direct, this::releaseMemory);
}
}
- private Bucket newBucket(int key)
+ private Bucket newBucket(int key, boolean direct)
{
- return new Bucket(this, key * getCapacityFactor(), getMaxQueueLength());
+ return new Bucket(this, capacityFor(key), getMaxQueueLength(), updateMemory(direct));
}
@Override
@@ -155,18 +171,12 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
super.clear();
for (int i = 0; i < _direct.length; ++i)
{
- Bucket bucket = _direct[i];
- if (bucket != null)
- bucket.clear();
- _direct[i] = null;
- bucket = _indirect[i];
- if (bucket != null)
- bucket.clear();
- _indirect[i] = null;
+ _direct[i].clear();
+ _indirect[i].clear();
}
}
- private void clearOldestBucket(boolean direct)
+ protected void releaseMemory(boolean direct)
{
long oldest = Long.MAX_VALUE;
int index = -1;
@@ -174,7 +184,7 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
for (int i = 0; i < buckets.length; ++i)
{
Bucket bucket = buckets[i];
- if (bucket == null)
+ if (bucket.isEmpty())
continue;
long lastUpdate = bucket.getLastUpdate();
if (lastUpdate < oldest)
@@ -186,31 +196,29 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
if (index >= 0)
{
Bucket bucket = buckets[index];
- buckets[index] = null;
- // The same bucket may be concurrently
- // removed, so we need this null guard.
- if (bucket != null)
- bucket.clear(this::decrementMemory);
+ bucket.clear();
}
}
- private int bucketFor(int capacity)
+ protected int bucketFor(int capacity)
{
- return (capacity - 1) / getCapacityFactor();
+ return (int)Math.ceil((double)capacity / getCapacityFactor());
}
- private ByteBufferPool.Bucket bucketFor(int capacity, boolean direct, IntFunction newBucket)
+ protected int capacityFor(int bucket)
+ {
+ return bucket * getCapacityFactor();
+ }
+
+ private ByteBufferPool.Bucket bucketFor(int capacity, boolean direct)
{
if (capacity < _minCapacity)
return null;
- int b = bucketFor(capacity);
- if (b >= _direct.length)
+ int index = bucketFor(capacity) - 1;
+ if (index >= _direct.length)
return null;
Bucket[] buckets = bucketsFor(direct);
- Bucket bucket = buckets[b];
- if (bucket == null && newBucket != null)
- buckets[b] = bucket = newBucket.apply(b + 1);
- return bucket;
+ return buckets[index];
}
@ManagedAttribute("The number of pooled direct ByteBuffers")
@@ -238,4 +246,47 @@ public class ArrayByteBufferPool extends AbstractByteBufferPool
{
return direct ? _direct : _indirect;
}
+
+ public boolean isDetailedDump()
+ {
+ return _detailedDump;
+ }
+
+ public void setDetailedDump(boolean detailedDump)
+ {
+ _detailedDump = detailedDump;
+ }
+
+ @Override
+ public void dump(Appendable out, String indent) throws IOException
+ {
+ List