This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch POOL_2_X
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/POOL_2_X by this push:
new a1b1dbaf Made statistics collection optional in BaseGenericObjectPool
(#449)
a1b1dbaf is described below
commit a1b1dbaf8ad5860ff000e229a79f7e330ae05891
Author: Pratyay <[email protected]>
AuthorDate: Tue Dec 2 18:58:49 2025 +0530
Made statistics collection optional in BaseGenericObjectPool (#449)
---
.../commons/pool2/impl/BaseGenericObjectPool.java | 60 ++++++--
.../commons/pool2/impl/BaseObjectPoolConfig.java | 53 +++++++
.../pool2/impl/TestBaseGenericObjectPool.java | 170 +++++++++++++++++++++
3 files changed, 271 insertions(+), 12 deletions(-)
diff --git
a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
index a12cba72..e2e6f5b5 100644
--- a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
@@ -407,6 +407,7 @@ public abstract class BaseGenericObjectPool<T> extends
BaseObject implements Aut
private volatile SwallowedExceptionListener swallowedExceptionListener;
private volatile boolean messageStatistics;
+ private volatile boolean collectDetailedStatistics =
BaseObjectPoolConfig.DEFAULT_COLLECT_DETAILED_STATISTICS;
/** Additional configuration properties for abandoned object tracking. */
protected volatile AbandonedConfig abandonedConfig;
@@ -870,6 +871,20 @@ public abstract class BaseGenericObjectPool<T> extends
BaseObject implements Aut
return idleTimes.getMean();
}
+ /**
+ * Gets whether detailed timing statistics collection is enabled.
+ * When {@code false}, the pool will not collect detailed timing
statistics for
+ * mean active time, mean idle time, and mean borrow wait time,
+ * improving performance under high load.
+ *
+ * @return {@code true} if detailed statistics collection is enabled,
+ * {@code false} if disabled for improved performance.
+ * @since 2.13.0
+ */
+ public boolean getCollectDetailedStatistics() {
+ return collectDetailedStatistics;
+ }
+
/**
* Gets whether to include statistics in exception messages.
* <p>
@@ -1396,6 +1411,7 @@ public abstract class BaseGenericObjectPool<T> extends
BaseObject implements Aut
setEvictionPolicy(policy);
}
setEvictorShutdownTimeout(config.getEvictorShutdownTimeoutDuration());
+ setCollectDetailedStatistics(config.getCollectDetailedStatistics());
}
/**
@@ -1601,6 +1617,21 @@ public abstract class BaseGenericObjectPool<T> extends
BaseObject implements Aut
setMaxWait(Duration.ofMillis(maxWaitMillis));
}
+ /**
+ * Sets whether detailed timing statistics collection is enabled.
+ * When {@code false}, the pool will not collect detailed timing
statistics,
+ * improving performance under high load at the cost of reduced monitoring
capabilities.
+ * <p>
+ * This setting affects data collection for mean active time, mean idle
time, and mean borrow wait time.
+ * </p>
+ *
+ * @param collectDetailedStatistics whether to collect detailed statistics.
+ * @since 2.13.0
+ */
+ public void setCollectDetailedStatistics(final boolean
collectDetailedStatistics) {
+ this.collectDetailedStatistics = collectDetailedStatistics;
+ }
+
/**
* Sets whether to include statistics in exception messages.
* <p>
@@ -2068,17 +2099,19 @@ public abstract class BaseGenericObjectPool<T> extends
BaseObject implements Aut
*/
final void updateStatsBorrow(final PooledObject<T> p, final Duration
waitDuration) {
borrowedCount.incrementAndGet();
- idleTimes.add(p.getIdleDuration());
- waitTimes.add(waitDuration);
-
- // lock-free optimistic-locking maximum
- Duration currentMaxDuration;
- do {
- currentMaxDuration = maxBorrowWaitDuration.get();
- if (currentMaxDuration.compareTo(waitDuration) >= 0) {
- break;
- }
- } while (!maxBorrowWaitDuration.compareAndSet(currentMaxDuration,
waitDuration));
+ // Only collect detailed statistics if enabled
+ if (collectDetailedStatistics) {
+ idleTimes.add(p.getIdleDuration());
+ waitTimes.add(waitDuration);
+ // lock-free optimistic-locking maximum
+ Duration currentMaxDuration;
+ do {
+ currentMaxDuration = maxBorrowWaitDuration.get();
+ if (currentMaxDuration.compareTo(waitDuration) >= 0) {
+ break;
+ }
+ } while (!maxBorrowWaitDuration.compareAndSet(currentMaxDuration,
waitDuration));
+ }
}
/**
@@ -2089,7 +2122,10 @@ public abstract class BaseGenericObjectPool<T> extends
BaseObject implements Aut
*/
final void updateStatsReturn(final Duration activeTime) {
returnedCount.incrementAndGet();
- activeTimes.add(activeTime);
+ // Only collect detailed statistics if enabled
+ if (collectDetailedStatistics) {
+ activeTimes.add(activeTime);
+ }
}
/**
diff --git
a/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
b/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
index 042785a4..76247227 100644
--- a/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
+++ b/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
@@ -254,6 +254,19 @@ public abstract class BaseObjectPoolConfig<T> extends
BaseObject implements Clon
*/
public static final String DEFAULT_EVICTION_POLICY_CLASS_NAME =
DefaultEvictionPolicy.class.getName();
+ /**
+ * The default value for the {@code collectDetailedStatistics}
configuration
+ * attribute. When {@code true}, the pool will collect detailed timing
statistics
+ * for monitoring purposes. When {@code false}, detailed statistics
collection
+ * is disabled, improving performance under high load.
+ * <p>
+ * This setting affects data collection for mean active time, mean idle
time, and mean borrow wait time.
+ * </p>
+ *
+ * @since 2.13.0
+ */
+ public static final boolean DEFAULT_COLLECT_DETAILED_STATISTICS = true;
+
private boolean lifo = DEFAULT_LIFO;
private boolean fairness = DEFAULT_FAIRNESS;
@@ -291,6 +304,8 @@ public abstract class BaseObjectPoolConfig<T> extends
BaseObject implements Clon
private String jmxNameBase = DEFAULT_JMX_NAME_BASE;
+ private boolean collectDetailedStatistics =
DEFAULT_COLLECT_DETAILED_STATISTICS;
+
/**
* Constructs a new instance.
*/
@@ -309,6 +324,23 @@ public abstract class BaseObjectPoolConfig<T> extends
BaseObject implements Clon
return blockWhenExhausted;
}
+ /**
+ * Gets the value for the {@code collectDetailedStatistics} configuration
attribute
+ * for pools created with this configuration instance.
+ * <p>
+ * This setting affects data collection for mean active time, mean idle
time, and mean borrow wait time.
+ * </p>
+ *
+ * @return {@code true} if detailed statistics collection is enabled,
+ * {@code false} if disabled for improved performance.
+ * @see GenericObjectPool#getCollectDetailedStatistics()
+ * @see GenericKeyedObjectPool#getCollectDetailedStatistics()
+ * @since 2.13.0
+ */
+ public boolean getCollectDetailedStatistics() {
+ return collectDetailedStatistics;
+ }
+
/**
* Gets the value for the {@code timeBetweenEvictionRuns} configuration
attribute for pools created with this configuration instance.
*
@@ -630,6 +662,25 @@ public abstract class BaseObjectPoolConfig<T> extends
BaseObject implements Clon
this.blockWhenExhausted = blockWhenExhausted;
}
+ /**
+ * Sets the value for the {@code collectDetailedStatistics} configuration
attribute
+ * for pools created with this configuration instance. When {@code false},
the pool
+ * will not collect detailed timing statistics, improving performance
under high load
+ * at the cost of reduced monitoring capabilities.
+ * <p>
+ * This setting affects data collection for mean active time, mean idle
time, and mean borrow wait time.
+ * </p>
+ *
+ * @param collectDetailedStatistics The new setting of {@code
collectDetailedStatistics}
+ * for this configuration instance.
+ * @see GenericObjectPool#getCollectDetailedStatistics()
+ * @see GenericKeyedObjectPool#getCollectDetailedStatistics()
+ * @since 2.13.0
+ */
+ public void setCollectDetailedStatistics(final boolean
collectDetailedStatistics) {
+ this.collectDetailedStatistics = collectDetailedStatistics;
+ }
+
/**
* Sets the value for the {@code evictionPolicyClass} configuration
attribute for pools created with this configuration instance.
*
@@ -960,5 +1011,7 @@ public abstract class BaseObjectPoolConfig<T> extends
BaseObject implements Clon
builder.append(jmxNamePrefix);
builder.append(", jmxNameBase=");
builder.append(jmxNameBase);
+ builder.append(", collectDetailedStatistics=");
+ builder.append(collectDetailedStatistics);
}
}
diff --git
a/src/test/java/org/apache/commons/pool2/impl/TestBaseGenericObjectPool.java
b/src/test/java/org/apache/commons/pool2/impl/TestBaseGenericObjectPool.java
index ed2dd77c..451653d4 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestBaseGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestBaseGenericObjectPool.java
@@ -18,10 +18,17 @@
package org.apache.commons.pool2.impl;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.lang.management.ManagementFactory;
import java.time.Duration;
import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
@@ -139,4 +146,167 @@ class TestBaseGenericObjectPool {
pools.forEach(GenericObjectPool::close);
}
}
+
+ @Test
+ void testCollectDetailedStatisticsDefault() {
+ // Test that collectDetailedStatistics defaults to true for backward
compatibility
+ assertTrue(pool.getCollectDetailedStatistics());
+ }
+
+ @Test
+ void testCollectDetailedStatisticsConfiguration() {
+ // Test configuration through config object
+ final GenericObjectPoolConfig<String> config = new
GenericObjectPoolConfig<>();
+ config.setCollectDetailedStatistics(false);
+ try (GenericObjectPool<String> testPool = new
GenericObjectPool<>(factory, config)) {
+ assertFalse(testPool.getCollectDetailedStatistics());
+ }
+ // Test runtime configuration
+ pool.setCollectDetailedStatistics(false);
+ assertFalse(pool.getCollectDetailedStatistics());
+ pool.setCollectDetailedStatistics(true);
+ assertTrue(pool.getCollectDetailedStatistics());
+ }
+
+ @Test
+ void testCollectDetailedStatisticsDisabled() throws Exception {
+ // Configure pool to disable detailed statistics
+ pool.setCollectDetailedStatistics(false);
+ final DefaultPooledObject<String> pooledObject =
(DefaultPooledObject<String>) factory.makeObject();
+ // Record initial values
+ final long initialActiveTime = pool.getMeanActiveTimeMillis();
+ final long initialIdleTime = pool.getMeanIdleDuration().toMillis();
+ final long initialWaitTime = pool.getMeanBorrowWaitTimeMillis();
+ final long initialMaxWaitTime = pool.getMaxBorrowWaitTimeMillis();
+ // Update statistics - should be ignored for detailed stats
+ pool.updateStatsBorrow(pooledObject, Duration.ofMillis(100));
+ pool.updateStatsReturn(Duration.ofMillis(200));
+ // Basic counters should still work
+ assertEquals(1, pool.getBorrowedCount());
+ assertEquals(1, pool.getReturnedCount());
+ // Detailed statistics should remain unchanged
+ assertEquals(initialActiveTime, pool.getMeanActiveTimeMillis());
+ assertEquals(initialIdleTime, pool.getMeanIdleDuration().toMillis());
+ assertEquals(initialWaitTime, pool.getMeanBorrowWaitTimeMillis());
+ assertEquals(initialMaxWaitTime, pool.getMaxBorrowWaitTimeMillis());
+ }
+
+ @Test
+ void testCollectDetailedStatisticsEnabled() throws Exception {
+ // Ensure detailed statistics are enabled (default)
+ pool.setCollectDetailedStatistics(true);
+ final DefaultPooledObject<String> pooledObject =
(DefaultPooledObject<String>) factory.makeObject();
+ // Update statistics
+ pool.updateStatsBorrow(pooledObject, Duration.ofMillis(100));
+ pool.updateStatsReturn(Duration.ofMillis(200));
+ // All counters should work
+ assertEquals(1, pool.getBorrowedCount());
+ assertEquals(1, pool.getReturnedCount());
+ // Detailed statistics should be updated
+ assertEquals(200, pool.getMeanActiveTimeMillis());
+ assertEquals(100, pool.getMeanBorrowWaitTimeMillis());
+ assertEquals(100, pool.getMaxBorrowWaitTimeMillis());
+ }
+
+ @Test
+ void testCollectDetailedStatisticsToggling() throws Exception {
+ final DefaultPooledObject<String> pooledObject =
(DefaultPooledObject<String>) factory.makeObject();
+ // Start with detailed stats enabled
+ pool.setCollectDetailedStatistics(true);
+ pool.updateStatsBorrow(pooledObject, Duration.ofMillis(50));
+ pool.updateStatsReturn(Duration.ofMillis(100));
+ assertEquals(50, pool.getMeanBorrowWaitTimeMillis());
+ assertEquals(100, pool.getMeanActiveTimeMillis());
+ // Disable detailed stats
+ pool.setCollectDetailedStatistics(false);
+ pool.updateStatsBorrow(pooledObject, Duration.ofMillis(200));
+ pool.updateStatsReturn(Duration.ofMillis(300));
+ // Detailed stats should remain at previous values
+ assertEquals(50, pool.getMeanBorrowWaitTimeMillis());
+ assertEquals(100, pool.getMeanActiveTimeMillis());
+ // Basic counters should continue to increment
+ assertEquals(2, pool.getBorrowedCount());
+ assertEquals(2, pool.getReturnedCount());
+ }
+
+ @Test
+ void testStatsStoreConcurrentAccess() throws Exception {
+ // Test the lock-free StatsStore implementation under concurrent load
+ final int numThreads = 10;
+ final int operationsPerThread = 1000;
+ final ExecutorService executor =
Executors.newFixedThreadPool(numThreads);
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final CountDownLatch completeLatch = new CountDownLatch(numThreads);
+ final List<Future<Void>> futures = new ArrayList<>();
+ // Create threads that will concurrently update statistics
+ for (int i = 0; i < numThreads; i++) {
+ final int threadId = i;
+ futures.add(executor.submit(() -> {
+ try {
+ final DefaultPooledObject<String> pooledObject =
(DefaultPooledObject<String>) factory.makeObject();
+ // Wait for all threads to be ready
+ startLatch.await();
+ // Perform concurrent operations
+ for (int j = 0; j < operationsPerThread; j++) {
+ pool.updateStatsBorrow(pooledObject,
Duration.ofMillis(threadId * 10 + j));
+ pool.updateStatsReturn(Duration.ofMillis(threadId * 20
+ j));
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ } finally {
+ completeLatch.countDown();
+ }
+ return null;
+ }));
+ }
+ // Start all threads simultaneously
+ startLatch.countDown();
+ // Wait for completion
+ assertTrue(completeLatch.await(30, TimeUnit.SECONDS), "Concurrent test
should complete within 30 seconds");
+ // Verify no exceptions occurred
+ for (Future<Void> future : futures) {
+ future.get(); // Will throw if there was an exception
+ }
+ // Verify that statistics were collected (exact values may vary due to
race conditions)
+ assertEquals(numThreads * operationsPerThread,
pool.getBorrowedCount());
+ assertEquals(numThreads * operationsPerThread,
pool.getReturnedCount());
+ // Mean values should be reasonable (not zero or wildly incorrect)
+ assertTrue(pool.getMeanActiveTimeMillis() >= 0);
+ assertTrue(pool.getMeanBorrowWaitTimeMillis() >= 0);
+ assertTrue(pool.getMaxBorrowWaitTimeMillis() >= 0);
+ executor.shutdown();
+ assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS));
+ }
+
+ @Test
+ void testStatsStoreCircularBuffer() throws Exception {
+ // Test that StatsStore properly handles circular buffer behavior
+ final DefaultPooledObject<String> pooledObject =
(DefaultPooledObject<String>) factory.makeObject();
+ // Fill beyond the cache size (100) to test circular behavior
+ final int cacheSize = 100; //
BaseGenericObjectPool.MEAN_TIMING_STATS_CACHE_SIZE
+ for (int i = 0; i < cacheSize + 50; i++) {
+ pool.updateStatsBorrow(pooledObject, Duration.ofMillis(i));
+ pool.updateStatsReturn(Duration.ofMillis(i * 2));
+ }
+ // Statistics should still be meaningful after circular buffer wrapping
+ assertTrue(pool.getMeanActiveTimeMillis() > 0);
+ assertTrue(pool.getMeanBorrowWaitTimeMillis() > 0);
+ assertTrue(pool.getMaxBorrowWaitTimeMillis() > 0);
+ // The mean should reflect recent values, not all historical values
+ // (exact assertion depends on circular buffer implementation)
+ assertTrue(pool.getMeanBorrowWaitTimeMillis() >= 50); // Should be
influenced by recent higher values
+ }
+
+ @Test
+ void testDetailedStatisticsConfigIntegration() {
+ // Test that config property is properly applied during pool
construction
+ final GenericObjectPoolConfig<String> config = new
GenericObjectPoolConfig<>();
+ config.setCollectDetailedStatistics(false);
+ try (GenericObjectPool<String> testPool = new
GenericObjectPool<>(factory, config)) {
+ assertFalse(testPool.getCollectDetailedStatistics(), "Pool should
respect collectDetailedStatistics setting from config");
+ // Test that toString includes the new property
+ final String configString = config.toString();
+ assertTrue(configString.contains("collectDetailedStatistics"),
"Config toString should include collectDetailedStatistics property");
+ }
+ }
}
