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.
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. Analyzing the Stack(Keyed)ObjectPool(Factory) case: - Config properties are immutable; - A builder is added to simplify the Config building; - The builder contains exactly the same Config properties, initialized with default values; - Pool/Factory contains exactly the same Config properties, initialized copying the values from the config. OTOH, in the Generic(Keyed)ObjectPool(Factory) - Config properties are mutable; - No need of having a builder, config properties are already initialized with a default value; - Pool/Factory just reuse the config instance So I'm still +1 on adopting the second "pattern". -1 on adding the reconfigure(Config) method, we discussed about adding it in another thread, so I added to see if there are benefits but I don't see any advantage. Have a nice day, Simo http://people.apache.org/~simonetripodi/ http://www.99soft.org/ On Sat, Oct 30, 2010 at 6:56 PM, Phil Steitz <phil.ste...@gmail.com> wrote: > On 10/29/10 2:41 PM, James Carman wrote: >> >> On Fri, Oct 29, 2010 at 2:24 PM, sebb<seb...@gmail.com> 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: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
