commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simone Tripodi <simone.trip...@gmail.com>
Subject Re: [pool] Refactoring Config classes (Was: [pool] Pool config vs. factory hierarchies.)
Date Tue, 21 Dec 2010 06:51:59 GMT
Thanks for the feedbacks Gary,
I'm committing the latest modifications right now, every kind of
feedback/suggestion is appreciated, every contribution is more than
welcome.
Many thanks in advance, have a nice day,
Simo

http://people.apache.org/~simonetripodi/
http://www.99soft.org/



On Tue, Dec 21, 2010 at 12:30 AM, Gary Gregory
<GGregory@seagullsoftware.com> wrote:
> Please go ahead as you see fit.
>
> I am in the middle of moving now and can only process emails.
>
> I am also not sure about the merit of the refactoring anymore. I recall a message casting
doubt on this refactoring adventure :)
>
> Gary
>
> On Dec 20, 2010, at 13:56, "Simone Tripodi" <simone.tripodi@gmail.com> wrote:
>
>> Hi Gary,
>> IMHO the optimizations you're proposing are very important and since I
>> like cooperating with you, I thought was better making aware about
>> what I'd intend to do :P
>> If you agree I could start committing the latest modifications, so you
>> can apply your refactoring later. WDYT?
>> Have a nice day, all the best,
>> Simo
>>
>> http://people.apache.org/~simonetripodi/
>> http://www.99soft.org/
>>
>>
>>
>> On Mon, Dec 20, 2010 at 10:41 PM, Gary Gregory
>> <GGregory@seagullsoftware.com> wrote:
>>> Don't worry about my stuff. I am in the middle of a move.
>>>
>>> Gary
>>>
>>> On Dec 20, 2010, at 12:20, "Simone Tripodi" <simone.tripodi@gmail.com>
wrote:
>>>
>>>> Hi all guys,
>>>> I spent the afternoon refactoring the config classes according to
>>>> informations collected on[1], have a ready commit.
>>>> Do you want to read the patch before I commit or I I can commit so
>>>> everybody can review and contribute to the modifications?
>>>> This could block the Gary's work on optimization, please let me know!
>>>> Thanks in advance,
>>>> Simo
>>>>
>>>> [1] http://wiki.apache.org/commons/PoolRoadMap
>>>>
>>>> http://people.apache.org/~simonetripodi/
>>>> http://www.99soft.org/
>>>>
>>>>
>>>>
>>>> On Sun, Dec 5, 2010 at 2:27 PM, Simone Tripodi <simone.tripodi@gmail.com>
wrote:
>>>>> Hi Gary/all :)
>>>>> I collected informations on the wiki on the RoadMap page[1], based on
>>>>> what we discussed in this thread.
>>>>> If you agree on what is written, we can start back coding.
>>>>> Have a nice weekend,
>>>>> Simo
>>>>>
>>>>> [1] http://wiki.apache.org/commons/PoolRoadMap
>>>>>
>>>>> http://people.apache.org/~simonetripodi/
>>>>> http://www.99soft.org/
>>>>>
>>>>>
>>>>>
>>>>> On Wed, Dec 1, 2010 at 3:21 PM, Gary Gregory
>>>>> <GGregory@seagullsoftware.com> wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Simone Tripodi [mailto:simone.tripodi@gmail.com]
>>>>>>> Sent: Wednesday, December 01, 2010 08:51
>>>>>>> To: Commons Developers List
>>>>>>> Subject: Re: [pool] Pool config vs. factory hierarchies.
>>>>>>>
>>>>>>> Hi Gary,
>>>>>>> yes, more people involved on defining these details is better,
I
>>>>>>> agree. I'm thinking about creating a wiki page to resume all
the
>>>>>>> requirements, what do you think?
>>>>>>
>>>>>> Good idea!
>>>>>>
>>>>>> Gary
>>>>>>
>>>>>>> Simo
>>>>>>>
>>>>>>> http://people.apache.org/~simonetripodi/
>>>>>>> http://www.99soft.org/
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Dec 1, 2010 at 2:39 PM, Gary Gregory
>>>>>>> <GGregory@seagullsoftware.com> wrote:
>>>>>>>> On Dec 1, 2010, at 2:01, "Simone Tripodi" <simone.tripodi@gmail.com>
wrote:
>>>>>>>>
>>>>>>>>> Hi Gary :)
>>>>>>>>> thanks for the feedback, IMHO once the Configuration
for
>>>>>>>>> Generic(Keyed)ObjectPool(Factory) will be fixed, we could
start
>>>>>>>>> thinking about a new release of the new pool. This evening/tonight
(in
>>>>>>>>> my local time) I'll start re-arranging stuff, of course
suggestions
>>>>>>>>> will be more than appreciated.
>>>>>>>>>
>>>>>>>>> About duplication: I agree with you, but after re-reading
all the
>>>>>>>>> mails we wrote about it, I recently become convinced
that
>>>>>>>>> Configuration for GKOB/GOB have different semantic even
if some
>>>>>>>>> configuration property have same name/type, what's your
opinion about
>>>>>>>>> it? Many thanks in advance!
>>>>>>>>>
>>>>>>>> Yes, semantics are different iirc and I'd confusing is that
some props have
>>>>>>> the same name but mean different things for each pool type.
>>>>>>>>
>>>>>>>> I am not sure if it worth changing these method names (new
method and
>>>>>>> deprecated old method) or just writing better javadocs. I would
go with an
>>>>>>> experts opinion there.
>>>>>>>>
>>>>>>>> Gary
>>>>>>>>> Have a nice day,
>>>>>>>>> Simo
>>>>>>>>>
>>>>>>>>> http://people.apache.org/~simonetripodi/
>>>>>>>>> http://www.99soft.org/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Nov 30, 2010 at 11:02 PM, Gary Gregory
>>>>>>>>> <GGregory@seagullsoftware.com> wrote:
>>>>>>>>>> Yes, I thought we were on a roll there! Lots of good
discussions and
>>>>>>> then... quiet. That's OK though. We all get busy. Time to come
back and
>>>>>>> reflect.
>>>>>>>>>>
>>>>>>>>>> I am still looking for these goals:
>>>>>>>>>> - Generics released ASAP. I would be OK for a earlier
release just to get
>>>>>>> this out.
>>>>>>>>>> - Better names for properties and methods
>>>>>>>>>> - Refactor to remove duplication
>>>>>>>>>>
>>>>>>>>>> Gary
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: Simone Tripodi [mailto:simone.tripodi@gmail.com]
>>>>>>>>>>> Sent: Tuesday, November 30, 2010 09:33
>>>>>>>>>>> To: Commons Developers List
>>>>>>>>>>> Subject: Re: [pool] Pool config vs. factory hierarchies.
>>>>>>>>>>>
>>>>>>>>>>> Hi all guys,
>>>>>>>>>>> sorry for resurrecting a zombie message, but
I've been busy at work
>>>>>>>>>>> and haven't had the chance to contribute at all.
>>>>>>>>>>> I could re-start committing code according to
the requirements
>>>>>>>>>>> described by Phil, If it works for you, so other
tasks like
>>>>>>>>>>> JMX/autoconfigure can be unlocked, please let
me know.
>>>>>>>>>>> Have a nice day,
>>>>>>>>>>> Simo
>>>>>>>>>>>
>>>>>>>>>>> http://people.apache.org/~simonetripodi/
>>>>>>>>>>> http://www.99soft.org/
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Nov 3, 2010 at 11:01 PM, Phil Steitz
<phil.steitz@gmail.com>
>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Nov 3, 2010, at 5:03 PM, Steven Siebert
<smsiebe@gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> You restore the pool fields that
used to hold the configuration
>>>>>>> setting
>>>>>>>>>>>>>> properties and leave the getters
and setters (for the mutable ones) in
>>>>>>>>>>>>>> place.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Phil
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>> so something like this?
>>>>>>>>>>>>>
>>>>>>>>>>>>> public class GOP extends .... {
>>>>>>>>>>>>>
>>>>>>>>>>>>>   /**
>>>>>>>>>>>>>    * ref to immutable config reference,
immutable config values are
>>>>>>> either
>>>>>>>>>>>>> referred directly here
>>>>>>>>>>>>>    * or are copied to a final instance
field
>>>>>>>>>>>>>   */
>>>>>>>>>>>>>   private GOPConfig config
>>>>>>>>>>>>
>>>>>>>>>>>> No.  There is no config member.  It is
used only to encapsulate the
>>>>>>>>>>> parameters of the ctors.  The GOP class stores
the config data in
>>>>>>> individual
>>>>>>>>>>> fields, accessed by getters and setters.  The
setters at least are
>>>>>>>>>>> synchronized using the pool monitor. Look at
the old code.  What I am
>>>>>>>>>>> proposing is that we limit the use and lifetime
of the config objects to
>>>>>>> the
>>>>>>>>>>> ctors and/or factories.
>>>>>>>>>>>>
>>>>>>>>>>>> Phil
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>   //mutable configuration values are
mutated/accessed from pool
>>>>>>> instance
>>>>>>>>>>>>>   private volatile int mut1;  //probably
better to use read/write locks
>>>>>>>>>>>>>   private volatile int mut2;
>>>>>>>>>>>>>
>>>>>>>>>>>>>   public GOP (GOPConfig config) {
>>>>>>>>>>>>>      this.config = config;
>>>>>>>>>>>>>      reconfigure(config);
>>>>>>>>>>>>>   }
>>>>>>>>>>>>>
>>>>>>>>>>>>>   /**
>>>>>>>>>>>>>    * using this model, this method
isn't really required (at least not
>>>>>>>>>>>>> public)
>>>>>>>>>>>>>    * but would be a convenience for
"batch"/atomic changes to
>>>>>>> configuration
>>>>>>>>>>>>> values -
>>>>>>>>>>>>>    * this is possible if we switch
from volatile to a r/w locking
>>>>>>> mechanism
>>>>>>>>>>>>>   */
>>>>>>>>>>>>>   public void reconfigure (GOPConfig
config) {
>>>>>>>>>>>>>        mut1 = config.getMut1;
>>>>>>>>>>>>>        mut2  = config.getMut2;
>>>>>>>>>>>>>   }
>>>>>>>>>>>>>
>>>>>>>>>>>>>   public void setMut1 (int m) {
>>>>>>>>>>>>>      mut1 = m;
>>>>>>>>>>>>>   }
>>>>>>>>>>>>>
>>>>>>>>>>>>>   public int getMut1 () {
>>>>>>>>>>>>>       return mut1;
>>>>>>>>>>>>>   }
>>>>>>>>>>>>>
>>>>>>>>>>>>>   ....
>>>>>>>>>>>>> }
>>>>>>>>>>>>>
>>>>>>>>>>>>> I wonder, with this model....what is
the reason for having an immutable
>>>>>>>>>>>>> configuration instance if we're going
to copy the values locally for
>>>>>>> (at
>>>>>>>>>>>>> least) mutability purposes?  I believe
the attraction of the immutable
>>>>>>>>>>>>> configuration instance was for concurrency
issues...but with this
>>>>>>> model, we
>>>>>>>>>>>>> would need to use pool-local syncronization
(locking) anyway.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I wrote a quick mock-up implementation
like this, using a
>>>>>>>>>>>>> ReentrantReadWriteLock, and the amount
of concurrency work in each
>>>>>>>>>>>>> pool/factory started to pile up.  We
already identified that
>>>>>>> inheritance of
>>>>>>>>>>>>> the Pool/Factory classes might not be
the best approach (I agree with
>>>>>>> this
>>>>>>>>>>>>> as well...which would cause POOL-177
to no longer be implemented)...so
>>>>>>> this
>>>>>>>>>>>>> means duplication of synchronization
code as well.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think I'm falling back to my initial
thought on this in that the
>>>>>>> config
>>>>>>>>>>>>> classes should, IMO, either be mutable
(where appropriate) and made
>>>>>>> thread
>>>>>>>>>>>>> safe (internally synchronized) to reduce
the amount of concurrency work
>>>>>>>>>>>>> needed in each class that aggregates
the instance...or immutable and
>>>>>>> any
>>>>>>>>>>>>> changes to the config instance needs
to be done by going back to the
>>>>>>>>>>> Builder
>>>>>>>>>>>>> (something like new Builder(configInstance).change().create());)
and
>>>>>>> then
>>>>>>>>>>>>> the config reference in each pool/factory
could be made volatile.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I know this is confusing in email....I
would be glad to create a quick
>>>>>>>>>>> patch
>>>>>>>>>>>>> or UML for this to clear things up if
this would help.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thoughts?
>>>>>>>>>>>>>
>>>>>>>>>>>>> S
>>>>>>>>>>>>
>>>>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>>>>> 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
>>>>>>
>>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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


Mime
View raw message