commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <>
Subject Re: [pool2] again on get rid of synchronized block
Date Tue, 13 Dec 2011 05:58:07 GMT
On 12/12/11 5:30 PM, sebb wrote:
> On 13 December 2011 00:02, sebb <> wrote:
>> On 12 December 2011 21:07, Phil Steitz <> wrote:
>>> On 12/12/11 1:15 PM, sebb wrote:
>>>> On 12 December 2011 19:42, Phil Steitz <> wrote:
>>>>> On 12/12/11 12:29 PM, Simone Tripodi wrote:
>>>>>> Hi Phil,
>>>>>> thanks for the feedbacks! I would put some energies on [pool2] since
>>>>>> am currently in the situation I need it - of course in the meantime
>>>>>> can work with snapshots, but having a release would be definitively
>>>>>> better :)
>>>>>> So I start filling an issue and update synchronized pools in
>>>>>> PoolUtils, then IMHO we should back speaking about implementing
>>>>>> Read/Write policy in Pools implementation. Thoughts?
>>>>> Personally, I think the 2.0 implementations in GKOP and GOP are good
>>>>> and would prefer to focus on making the final preparations for
>>>>> release rather than reworking the - very tricky - locking and
>>>>> synchronization semantics in those implementations.
>>>> +1
>>>>> That said, if
>>>>> you or anyone else has better ideas, we can certainly look at them.
>>>>> Performance and intensive concurrency testing of the code in trunk
>>>>> would also be very helpful.
>>>> Probably also useful to work through documenting which classes are
>>>> intended to be thread-safe, immutable, etc, and then checking that
>>>> they are ...
>>>> For example, it looks as though GenericKeyedObjectPool is intended to
>>>> be thread-safe (but this is not yet documented).
>>>> However, the boolean field _lifo is not volatile, and the
>>>> setter/getter are not synchronized.
>>>> This means changes to the field are not guaranteed to be safely published.
>>>> Of course that's not a problem if GKOP is not intended to be thread-safe.
>>> GKOP should be threadsafe.
>> OK.
>>> Probably _lifo should be made final.
>> OK, the setter could be removed then, as lifo is part of the ctor init.
>> However that was just an example picked at random - there are lots of
>> other similar setters, including a setConfig() that sets them all ...
>> I think GKOP should be largely immutable - does it make sense to
>> change the behaviour once it has been instantiated?
>> IIRC, the idea of the Config class was to hold all the various
>> settings, and then the ctor would save them in final variables.
>> However at present, the settings are saved in mutable variables, and
>> are also settable individually.
>> This does not make sense to me.
>> Given that the setters are currently part of the public API, I think
>> this needs to be addressed before 2.0 can be released.
>> Exactly the same issues apply to GenericObjectPool.
> Actually, not quite - the class uses setters/getters, but the mutable
> fields are volatile, so at least they will be published correctly...
> However, does it really make sense to be able to re-configure the pool
> after creation?
> If so, what settings have a genuine use-case?

I like the idea of being conservative on this to start.  Now that we
have the config object, why not just make none of the config
parameters mutable once the pool is constructed?  The only ones that
people are likely to miss are maxActive and maxWait and possibly min
or max idle.  I wonder how many people are actually using this in
[pool] 1.x after initialization.  As long as the actual pool fields
are private, if we get talked into making the associated config
properties mutable post-construction later, we can do that without
breaking compatibility.

This will cause some pain for some DI frameworks and existing JNDI
factories, so we have to think through how to enable and explain
using the new config object to accomplish initialization in these
contexts.  We also need to make sure that whatever we do works for

>> Also, what about GenericKeyedObjectPoolFactory ?
>> That also has setters for the fields that are already set via the ctor.
>> Very flexible, but harder to test thoroughly and not thread-safe as it stands.
>> Given that Pool2 is a new API and does not need to maintain
>> compatibilty, I think it would make sense to start with the smallest
>> possible public API and only expand as necessary. So I would favour
>> making almost all fields final. Any fields that need to be mutable
>> must be private. Final fields should also be private unless they are
>> constants.
>>> Phil
>>>>> Phil
>>>>>> -Simo
>>>>>> On Mon, Dec 12, 2011 at 7:21 PM, Phil Steitz <>
>>>>>>> On 12/12/11 10:56 AM, Phil Steitz wrote:
>>>>>>>> On 12/12/11 10:26 AM, Simone Tripodi wrote:
>>>>>>>>> Hi all guys,
>>>>>>>>> time ago we spoke about replacing the synchronized blocks
>>>>>>>>> pools, maybe using different strategies like Java5 Read/Write
lock (I
>>>>>>>>> remind you Pool2 requires Java5) and I just started playing
>>>>>>>>> PoolUtils with the SynchronizedObjectPool[1] inner class...
>>>>>>>>> Can we discuss about introducing such modifications inside
pool implementations?
>>>>>>>> I would say go for it in PoolUtils.  Assuming we keep these
>>>>>>>> we should update them as we have GOP, GKOP.  Regarding the
pool you
>>>>>>>> mention specifically, it is a little funny in that it is
designed to
>>>>>>>> be fully synchronized.  Make sure that whatever implementation
>>>>>>>> changes you propose maintain the contract.
>>>>>>> Looking at the github code, I think there may be a problem. 
>>>>>>> is as much a "write" operation as return - both modify the state
>>>>>>> the pool.
>>>>>>> Phil
>>>>>>>> Phil
>>>>>>>>> Many thanks in advance, all the best!
>>>>>>>>> -Simo
>>>>>>>>> [1]
>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>> To unsubscribe, e-mail:
>>>>>>>>> For additional commands, e-mail:
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail:
>>>>>>> For additional commands, e-mail:
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail:
>>>>>> For additional commands, e-mail:
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail:
>>>>> For additional commands, e-mail:
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail:
>>>> For additional commands, e-mail:
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail:
>>> For additional commands, e-mail:
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message