commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r1164051 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Date Thu, 01 Sep 2011 23:40:42 GMT
On 9/1/11 5:41 AM, markt@apache.org wrote:
> Author: markt
> Date: Thu Sep  1 12:41:54 2011
> New Revision: 1164051
>
> URL: http://svn.apache.org/viewvc?rev=1164051&view=rev
> Log:
> Remove sync that wasn't helping.
> Make thread safe

I agree that the sync wasn't helping; but I am not sure exactly what
"thread safe" means for this method.  I have always worried about
this method.  I have usually been able to convince myself that it's
incorrectness is harmless (i.e., there are guards to prevent pool
invariants being violated due to incorrect / inconsistent return
value), but I am not sure that is right.  It might be good to
temporarily make this method protected and run some concurrency
tests directly against it, so we can understand and document what
happens when instances come in and out of the pool while it is
executing.

Phil
>
> Modified:
>     commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.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=1164051&r1=1164050&r2=1164051&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
Thu Sep  1 12:41:54 2011
> @@ -1577,25 +1577,30 @@ public class GenericKeyedObjectPool<K,T>
>       * @param pool the ObjectPool to calculate the deficit for
>       * @return The number of objects to be created
>       */
> -    private synchronized int calculateDeficit(ObjectDeque<T> objectDeque) {
> +    private int calculateDeficit(ObjectDeque<T> objectDeque) {
>          
>          if (objectDeque == null) {
>              return getMinIdlePerKey();
>          }
> +
> +        // Used more than once so keep a local copy so the value is consistent
> +        int maxTotal = getMaxTotal();
> +        int maxTotalPerKey = getMaxTotalPerKey();
> +
>          int objectDefecit = 0;
> -        
> +
>          // Calculate no of objects needed to be created, in order to have
>          // the number of pooled objects < maxTotalPerKey();
>          objectDefecit = getMinIdlePerKey() - objectDeque.getIdleObjects().size();
> -        if (getMaxTotalPerKey() > 0) {
> +        if (maxTotalPerKey > 0) {
>              int growLimit = Math.max(0,
> -                    getMaxTotalPerKey() - objectDeque.getIdleObjects().size());
> +                    maxTotalPerKey - objectDeque.getIdleObjects().size());
>              objectDefecit = Math.min(objectDefecit, growLimit);
>          }
>  
>          // Take the maxTotal limit into account
> -        if (getMaxTotal() > 0) {
> -            int growLimit = Math.max(0, getMaxTotal() - getNumActive() - getNumIdle());
> +        if (maxTotal > 0) {
> +            int growLimit = Math.max(0, maxTotal - getNumActive() - getNumIdle());
>              objectDefecit = Math.min(objectDefecit, growLimit);
>          }
>  
>
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message