diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java index fc3bfbbf5..e128d52a1 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java @@ -408,6 +408,7 @@ public String toString() { 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; @@ -835,6 +836,20 @@ public boolean getMessageStatistics() { return messageStatistics; } + /** + * 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 the minimum amount of time an object may sit idle in the pool * before it is eligible for eviction by the idle object evictor (if any - @@ -1270,6 +1285,7 @@ protected void setConfig(final BaseObjectPoolConfig config) { setEvictionPolicy(policy); } setEvictorShutdownTimeout(config.getEvictorShutdownTimeoutDuration()); + setCollectDetailedStatistics(config.getCollectDetailedStatistics()); } /** @@ -1455,6 +1471,21 @@ public void setMessagesStatistics(final boolean messagesDetails) { this.messageStatistics = messagesDetails; } + /** + * 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. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

+ * + * @param collectDetailedStatistics whether to collect detailed statistics. + * @since 2.13.0 + */ + public void setCollectDetailedStatistics(final boolean collectDetailedStatistics) { + this.collectDetailedStatistics = collectDetailedStatistics; + } + /** * Sets the minimum amount of time an object may sit idle in the pool * before it is eligible for eviction by the idle object evictor (if any - @@ -1738,20 +1769,19 @@ protected void toStringAppendFields(final StringBuilder builder) { */ final void updateStatsBorrow(final PooledObject 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 >= waitDuration) { -// break; -// } - 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)); + } } /** @@ -1762,7 +1792,10 @@ final void updateStatsBorrow(final PooledObject p, final Duration waitDuratio */ 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/pool3/impl/BaseObjectPoolConfig.java b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java index 5abbbece7..e47a04510 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java @@ -166,6 +166,19 @@ public abstract class BaseObjectPoolConfig 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. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

+ * + * @since 2.13.0 + */ + public static final boolean DEFAULT_COLLECT_DETAILED_STATISTICS = true; + private boolean lifo = DEFAULT_LIFO; private boolean fairness = DEFAULT_FAIRNESS; @@ -203,6 +216,8 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Clon private String jmxNameBase = DEFAULT_JMX_NAME_BASE; + private boolean collectDetailedStatistics = DEFAULT_COLLECT_DETAILED_STATISTICS; + /** * Constructs a new instance. */ @@ -224,6 +239,23 @@ public boolean getBlockWhenExhausted() { return blockWhenExhausted; } + /** + * Gets the value for the {@code collectDetailedStatistics} configuration attribute + * for pools created with this configuration instance. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

+ * + * @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. @@ -478,6 +510,26 @@ public void setBlockWhenExhausted(final boolean blockWhenExhausted) { 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. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

+ * + * @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 timeBetweenEvictionRuns} configuration * attribute for pools created with this configuration instance. @@ -755,5 +807,7 @@ protected void toStringAppendFields(final StringBuilder builder) { builder.append(jmxNamePrefix); builder.append(", jmxNameBase="); builder.append(jmxNameBase); + builder.append(", collectDetailedStatistics="); + builder.append(collectDetailedStatistics); } } diff --git a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java index d9fb5ec23..caeb2d851 100644 --- a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java @@ -18,10 +18,17 @@ package org.apache.commons.pool3.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; @@ -140,4 +147,167 @@ void testJMXRegistrationLatency() { 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 config = new GenericObjectPoolConfig<>(); + config.setCollectDetailedStatistics(false); + try (GenericObjectPool 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 pooledObject = (DefaultPooledObject) 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 pooledObject = (DefaultPooledObject) 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 pooledObject = (DefaultPooledObject) 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> 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 pooledObject = (DefaultPooledObject) 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 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 pooledObject = (DefaultPooledObject) 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 config = new GenericObjectPoolConfig<>(); + config.setCollectDetailedStatistics(false); + try (GenericObjectPool 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"); + } + } }