On Nov 3, 2010, at 5:03 PM, Steven Siebert <smsi...@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-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to