You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2011/09/02 01:40:42 UTC
Re: svn commit: r1164051 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
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
Re: svn commit: r1164051 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Posted by Phil Steitz <ph...@gmail.com>.
On 9/1/11 6:31 PM, sebb wrote:
> On 2 September 2011 01:23, Phil Steitz <ph...@gmail.com> wrote:
>> On 9/1/11 4:59 PM, Mark Thomas wrote:
>>> On 02/09/2011 00:40, Phil Steitz wrote:
>>>> 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.
>>> For me, thread safe means that a local copy is taken for any value used
>>> more than once that needs to be consistent between uses. I'm not sure we
>>> can do much more without tying ourselves in synchronisation knots.
>>>
>>>> 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.
>>> My analysis was that if you reduce the limits (maxTotal, maxIdle etc.)
>>> while this method is executing then there is a risk that it could exceed
>>> those limits. However, there is a fair chance that the pool will already
>>> be exceeding the limits when the limits are changed. Therefore, there is
>>> actually very little difference between these two scenarios. In both
>>> cases the pool will conform to the limits as soon as circumstances allow
>>> and once operating within the limits, will not exceed them.
>> I have never worried much about races against setters for maxTotal,
>> MaxIdle, though it is good to fix this as you did. What I have
>> always worried about is that once we removed the sync on this and
>> the pool statistics getters, what it computes becomes suspect. For
>> example, if things enter / exit the idle object pool between the
>> time objectDeficit and growLimit are computed, the return value will
>> be incorrect. As I said, IIRC previous traces through the code, the
>> use of the return value has guards to prevent too much badness from
>> this; but since the correctness of the computation done inside this
>> method depends on the idle instance pool keeping the same size over
>> the duration of its execution, it is neither really threadsafe nor
>> correct.
>>>> 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.
>>> I'm not against that but I don't have the time to do it right now and am
>>> unlikely to any time soon.
>> I will do some tracing and more analysis of the code to verify that
>> the problems that I think may exist in the correctness (or maybe
>> meaningfulness) of the return value are harmless.
> There's another threading issue - the JVM can cache shared variables
> locally in a thread.
> i.e. a reader thread may not see the latest copies of of all variables
> written by another thread.
> (in theory it might even see a half-written long variable)
>
> This is easily fixed by making variables volatile, but that can have a
> performance impact.
The problem that I am referring to has nothing to do with member
fields. I think Mark's change does all that is necessary to ensure
that the member fields are read consistently. The problem is that
objectDeque.getIdleObjects().size() is read twice in the method and
can change during execution or after execution before the value is
used. As I said, there are guards in place (I think) to ensure that
incorrectness of the value returned is harmless; but we dig into the
code and convince ourselves that that is still true.
Phil
>
>> Phil
>>> Mark
>>>
>>>
>>>> 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
>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1164051 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Posted by sebb <se...@gmail.com>.
On 2 September 2011 01:23, Phil Steitz <ph...@gmail.com> wrote:
> On 9/1/11 4:59 PM, Mark Thomas wrote:
>> On 02/09/2011 00:40, Phil Steitz wrote:
>>> 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.
>> For me, thread safe means that a local copy is taken for any value used
>> more than once that needs to be consistent between uses. I'm not sure we
>> can do much more without tying ourselves in synchronisation knots.
>>
>>> 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.
>> My analysis was that if you reduce the limits (maxTotal, maxIdle etc.)
>> while this method is executing then there is a risk that it could exceed
>> those limits. However, there is a fair chance that the pool will already
>> be exceeding the limits when the limits are changed. Therefore, there is
>> actually very little difference between these two scenarios. In both
>> cases the pool will conform to the limits as soon as circumstances allow
>> and once operating within the limits, will not exceed them.
>
> I have never worried much about races against setters for maxTotal,
> MaxIdle, though it is good to fix this as you did. What I have
> always worried about is that once we removed the sync on this and
> the pool statistics getters, what it computes becomes suspect. For
> example, if things enter / exit the idle object pool between the
> time objectDeficit and growLimit are computed, the return value will
> be incorrect. As I said, IIRC previous traces through the code, the
> use of the return value has guards to prevent too much badness from
> this; but since the correctness of the computation done inside this
> method depends on the idle instance pool keeping the same size over
> the duration of its execution, it is neither really threadsafe nor
> correct.
>>
>>> 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.
>> I'm not against that but I don't have the time to do it right now and am
>> unlikely to any time soon.
>
> I will do some tracing and more analysis of the code to verify that
> the problems that I think may exist in the correctness (or maybe
> meaningfulness) of the return value are harmless.
There's another threading issue - the JVM can cache shared variables
locally in a thread.
i.e. a reader thread may not see the latest copies of of all variables
written by another thread.
(in theory it might even see a half-written long variable)
This is easily fixed by making variables volatile, but that can have a
performance impact.
> Phil
>>
>> Mark
>>
>>
>>> 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
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1164051 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Posted by Phil Steitz <ph...@gmail.com>.
On 9/1/11 4:59 PM, Mark Thomas wrote:
> On 02/09/2011 00:40, Phil Steitz wrote:
>> 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.
> For me, thread safe means that a local copy is taken for any value used
> more than once that needs to be consistent between uses. I'm not sure we
> can do much more without tying ourselves in synchronisation knots.
>
>> 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.
> My analysis was that if you reduce the limits (maxTotal, maxIdle etc.)
> while this method is executing then there is a risk that it could exceed
> those limits. However, there is a fair chance that the pool will already
> be exceeding the limits when the limits are changed. Therefore, there is
> actually very little difference between these two scenarios. In both
> cases the pool will conform to the limits as soon as circumstances allow
> and once operating within the limits, will not exceed them.
I have never worried much about races against setters for maxTotal,
MaxIdle, though it is good to fix this as you did. What I have
always worried about is that once we removed the sync on this and
the pool statistics getters, what it computes becomes suspect. For
example, if things enter / exit the idle object pool between the
time objectDeficit and growLimit are computed, the return value will
be incorrect. As I said, IIRC previous traces through the code, the
use of the return value has guards to prevent too much badness from
this; but since the correctness of the computation done inside this
method depends on the idle instance pool keeping the same size over
the duration of its execution, it is neither really threadsafe nor
correct.
>
>> 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.
> I'm not against that but I don't have the time to do it right now and am
> unlikely to any time soon.
I will do some tracing and more analysis of the code to verify that
the problems that I think may exist in the correctness (or maybe
meaningfulness) of the return value are harmless.
Phil
>
> Mark
>
>
>> 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
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1164051 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Posted by Mark Thomas <ma...@apache.org>.
On 02/09/2011 00:40, Phil Steitz wrote:
> 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.
For me, thread safe means that a local copy is taken for any value used
more than once that needs to be consistent between uses. I'm not sure we
can do much more without tying ourselves in synchronisation knots.
> 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.
My analysis was that if you reduce the limits (maxTotal, maxIdle etc.)
while this method is executing then there is a risk that it could exceed
those limits. However, there is a fair chance that the pool will already
be exceeding the limits when the limits are changed. Therefore, there is
actually very little difference between these two scenarios. In both
cases the pool will conform to the limits as soon as circumstances allow
and once operating within the limits, will not exceed them.
> 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.
I'm not against that but I don't have the time to do it right now and am
unlikely to any time soon.
Mark
>
> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org