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
commit a7552be617b01574e5c7ce625c7a0af9885da17e Author: Gary Gregory <[email protected]> AuthorDate: Tue Dec 2 08:31:11 2025 -0500 Sort members --- .../commons/pool2/impl/BaseGenericObjectPool.java | 58 +++---- .../pool2/impl/TestBaseGenericObjectPool.java | 180 ++++++++++----------- 2 files changed, 119 insertions(+), 119 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 e2e6f5b5..4dfb0878 100644 --- a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java @@ -547,6 +547,20 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject implements Aut return borrowedCount.get(); } + /** + * 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 total number of objects created for this pool over the lifetime of * the pool. @@ -871,20 +885,6 @@ 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> @@ -1385,6 +1385,21 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject implements Aut this.blockWhenExhausted = blockWhenExhausted; } + /** + * 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 the receiver with the given configuration. * @@ -1617,21 +1632,6 @@ 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> 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 451653d4..66166a5e 100644 --- a/src/test/java/org/apache/commons/pool2/impl/TestBaseGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool2/impl/TestBaseGenericObjectPool.java @@ -95,64 +95,6 @@ class TestBaseGenericObjectPool { assertEquals(20, pool.getMaxBorrowWaitTimeMillis(), Double.MIN_VALUE); } - @Test - void testEvictionTimerMultiplePools() throws InterruptedException { - final AtomicIntegerFactory factory = new AtomicIntegerFactory(); - factory.setValidateLatency(50); - try (GenericObjectPool<AtomicInteger> evictingPool = new GenericObjectPool<>(factory)) { - evictingPool.setTimeBetweenEvictionRuns(Duration.ofMillis(100)); - evictingPool.setNumTestsPerEvictionRun(5); - evictingPool.setTestWhileIdle(true); - evictingPool.setMinEvictableIdleTime(Duration.ofMillis(50)); - for (int i = 0; i < 10; i++) { - try { - evictingPool.addObject(); - } catch (final Exception e) { - e.printStackTrace(); - } - } - - for (int i = 0; i < 1000; i++) { - try (GenericObjectPool<AtomicInteger> nonEvictingPool = new GenericObjectPool<>(factory)) { - // empty - } - } - - Thread.sleep(1000); - assertEquals(0, evictingPool.getNumIdle()); - } - } - - /** - * POOL-393 - * Tests JMX registration does not add too much latency to pool creation. - */ - @SuppressWarnings("resource") // pools closed in finally block - @Test - @Timeout(value = 10_000, unit = TimeUnit.MILLISECONDS) - void testJMXRegistrationLatency() { - final int numPools = 1000; - final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); - final ArrayList<GenericObjectPool<Waiter>> pools = new ArrayList<>(); - try { - // final long startTime = System.currentTimeMillis(); - for (int i = 0; i < numPools; i++) { - pools.add(new GenericObjectPool<>(new WaiterFactory<>(0, 0, 0, 0, 0, 0), new GenericObjectPoolConfig<>())); - } - // System.out.println("Duration: " + (System.currentTimeMillis() - startTime)); - final ObjectName oname = pools.get(numPools - 1).getJmxName(); - assertEquals(1, mbs.queryNames(oname, null).size()); - } finally { - 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 @@ -168,6 +110,12 @@ class TestBaseGenericObjectPool { assertTrue(pool.getCollectDetailedStatistics()); } + @Test + void testCollectDetailedStatisticsDefault() { + // Test that collectDetailedStatistics defaults to true for backward compatibility + assertTrue(pool.getCollectDetailedStatistics()); + } + @Test void testCollectDetailedStatisticsDisabled() throws Exception { // Configure pool to disable detailed statistics @@ -229,6 +177,90 @@ class TestBaseGenericObjectPool { assertEquals(2, pool.getReturnedCount()); } + @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"); + } + } + + @Test + void testEvictionTimerMultiplePools() throws InterruptedException { + final AtomicIntegerFactory factory = new AtomicIntegerFactory(); + factory.setValidateLatency(50); + try (GenericObjectPool<AtomicInteger> evictingPool = new GenericObjectPool<>(factory)) { + evictingPool.setTimeBetweenEvictionRuns(Duration.ofMillis(100)); + evictingPool.setNumTestsPerEvictionRun(5); + evictingPool.setTestWhileIdle(true); + evictingPool.setMinEvictableIdleTime(Duration.ofMillis(50)); + for (int i = 0; i < 10; i++) { + try { + evictingPool.addObject(); + } catch (final Exception e) { + e.printStackTrace(); + } + } + + for (int i = 0; i < 1000; i++) { + try (GenericObjectPool<AtomicInteger> nonEvictingPool = new GenericObjectPool<>(factory)) { + // empty + } + } + + Thread.sleep(1000); + assertEquals(0, evictingPool.getNumIdle()); + } + } + + /** + * POOL-393 + * Tests JMX registration does not add too much latency to pool creation. + */ + @SuppressWarnings("resource") // pools closed in finally block + @Test + @Timeout(value = 10_000, unit = TimeUnit.MILLISECONDS) + void testJMXRegistrationLatency() { + final int numPools = 1000; + final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); + final ArrayList<GenericObjectPool<Waiter>> pools = new ArrayList<>(); + try { + // final long startTime = System.currentTimeMillis(); + for (int i = 0; i < numPools; i++) { + pools.add(new GenericObjectPool<>(new WaiterFactory<>(0, 0, 0, 0, 0, 0), new GenericObjectPoolConfig<>())); + } + // System.out.println("Duration: " + (System.currentTimeMillis() - startTime)); + final ObjectName oname = pools.get(numPools - 1).getJmxName(); + assertEquals(1, mbs.queryNames(oname, null).size()); + } finally { + pools.forEach(GenericObjectPool::close); + } + } + + @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 testStatsStoreConcurrentAccess() throws Exception { // Test the lock-free StatsStore implementation under concurrent load @@ -277,36 +309,4 @@ class TestBaseGenericObjectPool { 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"); - } - } }
