commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <>
Subject Re: [pool] Pool config vs. factory hierarchies.
Date Sun, 31 Oct 2010 01:34:57 GMT
On 10/30/10 2:30 PM, Simone Tripodi wrote:
> Hi Phil,
> the benefits of eliminating the member variables in favor of storing
> pool config reference are IMHO in therms of code maintainability and
> keep it as much simple as possible.

Maybe I am being dense here, but I don't quite get that.  The pool 
has properties such as numActive that it needs to maintain as well 
as the config-related properties.  Keeping the Config instance 
around as a new pool property adds complexity and doesn't really 
improve maintainability, as far as I can see; but it is quite 
possible I am missing something.

> You can see the difference between the current
> Stack(Keyed)ObjectPool(Factory) - which are implemented according your
> vision - and Generic(Keyed)ObjectPool(Factory) implementations - that
> are still implemented with my first refactory.

No, these have the same new complexity (vis a vis the original code) 
and I am not seeing what the benefit is.  Keeping the pool 
properties as member fields, making the Config objects immutable and 
just having the ctors copy properties from the immutable Config 
instances (which are then thrown away) seems simpler and no harder 
to maintain to me.  Again, I must be missing something.

I guess at the end of the day, I don't see the "Config" abstraction 
as adding anything post-construction - i.e,, this abstraction is 
only useful at pool construction time.  From the user perspective, 
the config properties are no different from any other pool 
properties, nor are they different internally.  Why should we need 
to write Config.maxActive internally, but just use numActive and 
other properties directly?  This looks to me like added complexity 
that does not add any value, at least for the pools.  It makes more 
sense to me for the factories to hold a reference to a Config instance.


> On Sat, Oct 30, 2010 at 6:56 PM, Phil Steitz<>  wrote:
>> On 10/29/10 2:41 PM, James Carman wrote:
>>> On Fri, Oct 29, 2010 at 2:24 PM, sebb<>    wrote:
>>>> I had overlooked that aspect ...
>>>> If some changes are more expensive to perform, then the method might
>>>> want to determine which items have changed, rather than just
>>>> reconfiguring everything.
>>>> There may be some changes that don't require a pool update.
>>> Right now, it appears that they just call that "allocate" method which
>>> seems like they kind of "nuke and pave", so I don't think it's that
>>> big of a deal.
>>>> Factory reconfig probably just needs to update the stored config
>>>> variable, which can be volatile.
>>> I'm not familiar with this factory config stuff.  I'll have to dig in
>>> further.
>> Sorry to be late on this.  Here are the requirements:
>> 1. Some subset of the config properties (need to decide this - should be
>> topic of a different thread) need to be *individually* mutable at runtime -
>> e.g., setMaxActive(newMaxActive) needs to remain.  We have agreed at this
>> point that at least maxActive and maxWait need to be runtime mutable.
>> 2. Correct functioning of the pool with the current implementation requires
>> that no thread can change maxActive while another thread holds the lock on
>> the pool's monitor.  Just making the properties volatile or protecting them
>> with another lock will cause problems.
>> I am OK keeping the mutable Config instances around, but I don't see any
>> real advantage to eliminating the member variables storing pool config
>> properties - i.e., my preference would be to make the Config instances
>> immutable and only used as structs for ctors.
>> I am +0 on adding a (pool-synchronized) reconfigure(Config) to enable
>> multiple properties to be changed atomically.
>> Phil
>>> ---------------------------------------------------------------------
>>> 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