Author: markt
Date: Mon Mar 28 11:35:06 2011
New Revision: 1086194
URL: http://svn.apache.org/viewvc?rev=1086194&view=rev
Log:
Move allocate() calls outside sync blocks to address performance issues
identified in Phil's testing
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1086194&r1=1086193&r2=1086194&view=diff
==============================================================================
---
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
(original)
+++
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Mon Mar 28 11:35:06 2011
@@ -274,8 +274,10 @@ public class GenericKeyedObjectPool<K,V>
* Use a negative value for no limit.
* @see #getMaxTotal
*/
- public synchronized void setMaxTotalPerKey(int maxTotalPerKey) {
- this.maxTotalPerKey = maxTotalPerKey;
+ public void setMaxTotalPerKey(int maxTotalPerKey) {
+ synchronized(this) {
+ this.maxTotalPerKey = maxTotalPerKey;
+ }
allocate();
}
@@ -301,8 +303,10 @@ public class GenericKeyedObjectPool<K,V>
* Use a negative value for no limit.
* @see #getMaxTotal
*/
- public synchronized void setMaxTotal(int maxTotal) {
- this.maxTotal = maxTotal;
+ public void setMaxTotal(int maxTotal) {
+ synchronized(this) {
+ this.maxTotal = maxTotal;
+ }
allocate();
}
@@ -327,8 +331,10 @@ public class GenericKeyedObjectPool<K,V>
* @param whenExhaustedAction the action code
* @see #getWhenExhaustedAction
*/
- public synchronized void setWhenExhaustedAction(WhenExhaustedAction
whenExhaustedAction) {
- this.whenExhaustedAction = whenExhaustedAction;
+ public void setWhenExhaustedAction(WhenExhaustedAction
whenExhaustedAction) {
+ synchronized(this) {
+ this.whenExhaustedAction = whenExhaustedAction;
+ }
allocate();
}
@@ -367,8 +373,10 @@ public class GenericKeyedObjectPool<K,V>
* @see #setWhenExhaustedAction
* @see WhenExhaustedAction#BLOCK
*/
- public synchronized void setMaxWait(long maxWait) {
- this.maxWait = maxWait;
+ public void setMaxWait(long maxWait) {
+ synchronized(this) {
+ this.maxWait = maxWait;
+ }
allocate();
}
@@ -400,8 +408,10 @@ public class GenericKeyedObjectPool<K,V>
*
* @since 2.0
*/
- public synchronized void setMaxIdlePerKey(int maxIdlePerKey) {
- this.maxIdlePerKey = maxIdlePerKey;
+ public void setMaxIdlePerKey(int maxIdlePerKey) {
+ synchronized(this) {
+ this.maxIdlePerKey = maxIdlePerKey;
+ }
allocate();
}
@@ -685,11 +695,10 @@ public class GenericKeyedObjectPool<K,V>
// Add this request to the queue
_allocationQueue.add(latch);
-
- // Work the allocation queue, allocating idle instances and
- // instance creation permits in request arrival order
- allocate();
}
+ // Work the allocation queue, allocating idle instances and
+ // instance creation permits in request arrival order
+ allocate();
for(;;) {
synchronized (this) {
@@ -747,6 +756,7 @@ public class GenericKeyedObjectPool<K,V>
}
}
} catch(InterruptedException e) {
+ boolean doAllocate = false;
synchronized (this) {
// Need to handle the all three
possibilities
if (latch.getPair() == null &&
!latch.mayCreate()) {
@@ -757,7 +767,7 @@ public class GenericKeyedObjectPool<K,V>
// Case 2: latch has been given
permission to create
// a new object
latch.getPool().decrementInternalProcessingCount();
- allocate();
+ doAllocate = true;
} else {
// Case 3: An object has been allocated
latch.getPool().decrementInternalProcessingCount();
@@ -765,6 +775,9 @@ public class GenericKeyedObjectPool<K,V>
returnObject(latch.getkey(),
latch.getPair().getValue());
}
}
+ if (doAllocate) {
+ allocate();
+ }
Thread.currentThread().interrupt();
throw e;
}
@@ -801,8 +814,8 @@ public class GenericKeyedObjectPool<K,V>
synchronized (this) {
latch.getPool().decrementInternalProcessingCount();
// No need to reset latch - about to throw
exception
- allocate();
}
+ allocate();
}
}
}
@@ -833,8 +846,8 @@ public class GenericKeyedObjectPool<K,V>
latch.reset();
_allocationQueue.add(0, latch);
}
- allocate();
}
+ allocate();
if (newlyCreated) {
throw new NoSuchElementException(
"Could not create a validated object, cause: " +
@@ -850,7 +863,8 @@ public class GenericKeyedObjectPool<K,V>
/**
* Allocate available instances to latches in the allocation queue. Then
* set _mayCreate to true for as many additional latches remaining in queue
- * as _maxActive allows for each key.
+ * as _maxActive allows for each key. This method <b>MUST NOT</b> be called
+ * from inside a sync block.
*/
private void allocate() {
boolean clearOldest = false;
@@ -1078,8 +1092,8 @@ public class GenericKeyedObjectPool<K,V>
_poolList.remove(key);
}
}
- allocate();
}
+ allocate();
}
}
}
@@ -1171,8 +1185,8 @@ public class GenericKeyedObjectPool<K,V>
_poolMap.remove(key);
_poolList.remove(key);
}
- allocate();
}
+ allocate();
}
}
}
@@ -1207,6 +1221,7 @@ public class GenericKeyedObjectPool<K,V>
// Add instance to pool if there is room and it has passed validation
// (if testOnreturn is set)
+ boolean doAllocate = false;
synchronized (this) {
// grab the pool (list) of objects associated with the given key
pool = (_poolMap.get(key));
@@ -1235,11 +1250,14 @@ public class GenericKeyedObjectPool<K,V>
if (decrementNumActive) {
pool.decrementActiveCount();
}
- allocate();
+ doAllocate = true;
}
}
}
-
+ if (doAllocate) {
+ allocate();
+ }
+
// Destroy the instance if necessary
if (shouldDestroy) {
try {
@@ -1257,8 +1275,8 @@ public class GenericKeyedObjectPool<K,V>
_poolMap.remove(key);
_poolList.remove(key);
}
- allocate();
}
+ allocate();
}
}
}
@@ -1285,8 +1303,8 @@ public class GenericKeyedObjectPool<K,V>
_poolList.add(key);
}
pool.decrementActiveCount();
- allocate(); // _totalActive has changed
}
+ allocate(); // _totalActive has changed
}
}
@@ -1632,8 +1650,8 @@ public class GenericKeyedObjectPool<K,V>
} finally {
synchronized (this) {
pool.decrementInternalProcessingCount();
- allocate();
}
+ allocate();
}
}
}
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1086194&r1=1086193&r2=1086194&view=diff
==============================================================================
---
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
(original)
+++
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
Mon Mar 28 11:35:06 2011
@@ -256,8 +256,10 @@ public class GenericObjectPool<T> extend
* by the pool.
* @see #getMaxTotal
*/
- public synchronized void setMaxTotal(int maxTotal) {
- this.maxTotal = maxTotal;
+ public void setMaxTotal(int maxTotal) {
+ synchronized(this) {
+ this.maxTotal = maxTotal;
+ }
allocate();
}
@@ -283,8 +285,10 @@ public class GenericObjectPool<T> extend
* or {@link WhenExhaustedAction#GROW}
* @see #getWhenExhaustedAction
*/
- public synchronized void setWhenExhaustedAction(WhenExhaustedAction
whenExhaustedAction) {
- this.whenExhaustedAction = whenExhaustedAction;
+ public void setWhenExhaustedAction(WhenExhaustedAction
whenExhaustedAction) {
+ synchronized(this) {
+ this.whenExhaustedAction = whenExhaustedAction;
+ }
allocate();
}
@@ -323,8 +327,10 @@ public class GenericObjectPool<T> extend
* @see #setWhenExhaustedAction
* @see WhenExhaustedAction#BLOCK
*/
- public synchronized void setMaxWait(long maxWait) {
- this.maxWait = maxWait;
+ public void setMaxWait(long maxWait) {
+ synchronized(this) {
+ this.maxWait = maxWait;
+ }
allocate();
}
@@ -350,8 +356,10 @@ public class GenericObjectPool<T> extend
* Use a negative value to indicate an unlimited number of idle instances.
* @see #getMaxIdle
*/
- public synchronized void setMaxIdle(int maxIdle) {
- this.maxIdle = maxIdle;
+ public void setMaxIdle(int maxIdle) {
+ synchronized(this) {
+ this.maxIdle = maxIdle;
+ }
allocate();
}
@@ -367,8 +375,10 @@ public class GenericObjectPool<T> extend
* @see #getMinIdle
* @see #getTimeBetweenEvictionRunsMillis()
*/
- public synchronized void setMinIdle(int minIdle) {
- this.minIdle = minIdle;
+ public void setMinIdle(int minIdle) {
+ synchronized(this) {
+ this.minIdle = minIdle;
+ }
allocate();
}
@@ -656,11 +666,10 @@ public class GenericObjectPool<T> extend
// Add this request to the queue
_allocationQueue.add(latch);
-
- // Work the allocation queue, allocating idle instances and
- // instance creation permits in request arrival order
- allocate();
}
+ // Work the allocation queue, allocating idle instances and
+ // instance creation permits in request arrival order
+ allocate();
for(;;) {
synchronized (this) {
@@ -719,6 +728,7 @@ public class GenericObjectPool<T> extend
}
}
} catch(InterruptedException e) {
+ boolean doAllocate = false;
synchronized(this) {
// Need to handle the all three
possibilities
if (latch.getPair() == null &&
!latch.mayCreate()) {
@@ -729,7 +739,7 @@ public class GenericObjectPool<T> extend
// Case 2: latch has been given
permission to create
// a new object
_numInternalProcessing--;
- allocate();
+ doAllocate = true;
} else {
// Case 3: An object has been allocated
_numInternalProcessing--;
@@ -737,6 +747,9 @@ public class GenericObjectPool<T> extend
returnObject(latch.getPair().getValue());
}
}
+ if (doAllocate) {
+ allocate();
+ }
Thread.currentThread().interrupt();
throw e;
}
@@ -774,8 +787,8 @@ public class GenericObjectPool<T> extend
synchronized (this) {
_numInternalProcessing--;
// No need to reset latch - about to throw
exception
- allocate();
}
+ allocate();
}
}
}
@@ -807,8 +820,8 @@ public class GenericObjectPool<T> extend
latch.reset();
_allocationQueue.add(0, latch);
}
- allocate();
}
+ allocate();
if(newlyCreated) {
throw new NoSuchElementException("Could not create a
validated object, cause: " + e.getMessage());
}
@@ -822,7 +835,8 @@ public class GenericObjectPool<T> extend
/**
* Allocate available instances to latches in the allocation queue. Then
* set _mayCreate to true for as many additional latches remaining in queue
- * as _maxActive allows.
+ * as _maxActive allows. While it is safe for GOP, for consistency with
GKOP
+ * this method should not be called from inside a sync block.
*/
private synchronized void allocate() {
if (isClosed()) return;
@@ -869,8 +883,8 @@ public class GenericObjectPool<T> extend
} finally {
synchronized (this) {
_numActive--;
- allocate();
}
+ allocate();
}
}
@@ -919,8 +933,8 @@ public class GenericObjectPool<T> extend
} finally {
synchronized (this) {
_numInternalProcessing--;
- allocate();
}
+ allocate();
}
}
}
@@ -979,8 +993,8 @@ public class GenericObjectPool<T> extend
// "behavior flag", decrementNumActive, from addObjectToPool.
synchronized(this) {
_numActive--;
- allocate();
}
+ allocate();
}
}
@@ -1009,6 +1023,7 @@ public class GenericObjectPool<T> extend
// Add instance to pool if there is room and it has passed validation
// (if testOnreturn is set)
+ boolean doAllocate = false;
synchronized (this) {
if (isClosed()) {
shouldDestroy = true;
@@ -1026,10 +1041,13 @@ public class GenericObjectPool<T> extend
if (decrementNumActive) {
_numActive--;
}
- allocate();
+ doAllocate = true;
}
}
}
+ if (doAllocate) {
+ allocate();
+ }
// Destroy the instance if necessary
if(shouldDestroy) {
@@ -1042,8 +1060,8 @@ public class GenericObjectPool<T> extend
if (decrementNumActive) {
synchronized(this) {
_numActive--;
- allocate();
}
+ allocate();
}
}
@@ -1178,8 +1196,8 @@ public class GenericObjectPool<T> extend
} finally {
synchronized (this) {
_numInternalProcessing--;
- allocate();
}
+ allocate();
}
}
}
