openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pinaki Poddar" <ppod...@bea.com>
Subject Discussion on proposed change of dynamic modification of Values
Date Wed, 29 Aug 2007 13:24:53 GMT

Some out-of-band discussion on proposed change of dynamic modification
of Values.


> -----Original Message-----
> From: Pinaki Poddar 
> Sent: Wednesday, August 29, 2007 7:41 AM
> To: Patrick Linskey
> Subject: RE: EJB/Kodo team activity
> 
> Hi,
> I thought of prserving the oldValue and improve the 
> computaion of hashCode based on our earlier discssion (I 
> agree that a bug is lurking in purely supressing all dynamic 
> values to compute Configuration.hashCode).
> 
> But, currently all Value implementations update its internal 
> state *before* notifying the listener on via valueChanged(). 
> I did not know how to achieve that effect without changing 
> all the Value implementaions.
> Essentially internal state is overwritten -- I can (and I 
> think we should) make the change across all Value 
> implementations to first call valueChanged() (or rather 
> valueCahnaging()) so that copy-on-post-freeze-write can be in 
> a single place at Value implementaion.   
> Also I propose to make Value.valueChanged() final if we are 
> adopting this current strategy in discussion.
> 
> So to summarize following are the proposed changes:
> 1. Rename ValueListener.valueChanged() to valueChanging() to 
> reflect the vetoing power of ValueListener 
> 2. Reorder set() methods in all Value implementations to allow
ValueListener 
> to veto and also to enable copy-on-post-freeze-write 
> 3. Improve Configuration.hashCode computation for strong equality 
> 4. Remove superfluous assertReadOnly 
> 5. Make Value.valueChanged() final 
> 6. reorder condition check for Value.isDynamic() to preempt a cast 
> 
> 
> Pinaki Poddar
> 972.834.2865
> 
> -----Original Message-----
> From: Patrick Linskey 
> Sent: Wednesday, August 29, 2007 1:11 AM
> To: Pinaki Poddar
> Subject: RE: EJB/Kodo team activity
> 
> Hi,
> 
> It looks great.
> 
> There is one more thing that I think that we can do pretty 
> easily with this architecture: if a Value preserves what it 
> was set to pre-freeze (probably through a 
> copy-on-post-freeze-write algorithm), then we can have a 
> Value.getOriginal() call that the hashCode() and equals() 
> methods in Configuration can use. By doing this, we can 
> preserve strong equality whereby two configurations whose 
> values were the same when they were frozen are equal, rather 
> than disregarding dynamic properties altogether.
> 
> Also, with the veto-change logic in place, I think that we 
> can safely simplify the *ConfigurationImpl classes by 
> removing all the assertNotReadOnly() calls, which should now 
> be superfluous.
> 
> Finally, a minor detail: if you move the 'this.isDynamic' 
> check to the beginning of that 'if' statement, the VM will be 
> able to avoid a cast and a method call. Theoretically, I 
> guess the JIT should take care of that, but it's always nice 
> to line things up appropriately when possible.
> 
> -Patrick
> 
> --
> Patrick Linskey
> BEA Systems, Inc.
> ______________________________________________________________
> 
> > -----Original Message-----
> > From: Pinaki Poddar
> > Sent: Tuesday, August 28, 2007 7:54 PM
> > To: Patrick Linskey
> > Subject: RE: EJB/Kodo team activity
> > 
> > Please comment if this is somewhat inline with your thinking...
> > I have not removed the implementation of the APIs that I 
> added but I 
> > will...
> > 
> > Index: 
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Configur
> > ation.java
> > ===================================================================
> > ---
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Configur
> > ation.java	(revision 568257)
> > +++ 
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Configur
> > ation.java	(working copy)
> > @@ -230,21 +230,21 @@
> >       *
> >       * @since 1.0.0
> >       */
> > -    public void modifyDynamic(String property, Object newValue);
> > -    
> > -    /**
> > -     * Affirms if the given property can be modified 
> > <em>dynamically</em> i.e.
> > -     * even after the receiver is {@link 
> > #setReadOnly(boolean) frozen}. 
> > -     *
> > -     * @since 1.0.0
> > -     */
> > -    public boolean isDynamic(String property);
> > -    
> > -    /**
> > -     * Gets the values that can be modified 
> <em>dynamically</em> i.e.
> > -     * even after the receiver is {@link 
> > #setReadOnly(boolean) frozen}. 
> > -     *
> > -     * @since 1.0.0
> > -     */
> > -    public Value[] getDynamicValues();
> > +//    public void modifyDynamic(String property, Object newValue);
> > +//    
> > +//    /**
> > +//     * Affirms if the given property can be modified 
> > <em>dynamically</em> i.e.
> > +//     * even after the receiver is {@link 
> > #setReadOnly(boolean) frozen}. 
> > +//     *
> > +//     * @since 1.0.0
> > +//     */
> > +//    public boolean isDynamic(String property);
> > +//    
> > +//    /**
> > +//     * Gets the values that can be modified 
> > <em>dynamically</em> i.e.
> > +//     * even after the receiver is {@link 
> > #setReadOnly(boolean) frozen}. 
> > +//     *
> > +//     * @since 1.0.0
> > +//     */
> > +//    public Value[] getDynamicValues();
> >  }
> > Index: 
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Value.java
> > ===================================================================
> > ---
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Val
> > ue.java	(revision 568257)
> > +++ 
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Val
> > ue.java	(working copy)
> > @@ -43,6 +43,7 @@
> >      private ValueListener listen = null;
> >      private boolean aliasListComprehensive = false;
> >      private Class scope = null;
> > +    private boolean isDynamic = false;
> >  
> >      /**
> >       * Default constructor.
> > @@ -345,9 +346,23 @@
> >       * Subclasses should call this method when their inernal value 
> > changes.
> >       */
> >      public void valueChanged() {
> > -        if (listen != null)
> > -            listen.valueChanged(this);
> > +        if (listen != null) {
> > +        	if (listen instanceof Configuration && 
> > +        		(((Configuration)listen).isReadOnly())
> > && (!this.isDynamic)) {
> > +        		throw new
> > RuntimeException(s_loc.get("veto-change",
> > +        			this.getString()).toString());
> > +       		}
> > +        	listen.valueChanged(this);
> > +        }
> >      }
> > +    
> > +    public void setDynamic(boolean flag) {
> > +    	isDynamic = flag;
> > +    }
> > +    
> > +    public boolean isDynamic() {
> > +    	return isDynamic; 
> > +    }
> >  
> >      public int hashCode() {
> >          String str = getString();
> > Index: 
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Configur
> > ationImpl.java
> > ===================================================================
> > ---
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Configur
> > ationImpl.java	(revision 568257)
> > +++ 
> > openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Configur
> > ationImpl.java	(working copy)
> > @@ -1013,9 +1013,10 @@
> >      	if (map == null)
> >      		return null;
> >      	Map copy = new HashMap(map);
> > -    	Value[] dynamicValues = getDynamicValues();
> > -    	for (int i=0; i<dynamicValues.length; i++) {
> > -    		
> > Configurations.removeProperty(dynamicValues[i].getProperty(), copy);
> > +    	Value[] values = getValues();
> > +    	for (int i=0; i<values.length; i++) {
> > +    		if (values[i].isDynamic())
> > +    			
> > Configurations.removeProperty(values[i].getProperty(), copy);
> >      	}
> >      	return copy;
> >      }
> > Index: 
> > openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPACo
> > nfigurationImpl.java
> > ===================================================================
> > ---
> > openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPACo
> > nfigurationImpl.java	(revision 568257)
> > +++ 
> > openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPACo
> > nfigurationImpl.java	(working copy)
> > @@ -209,6 +209,7 @@
> >          dataCacheTimeout = addInt("DataCacheTimeout");
> >          dataCacheTimeout.setDefault("-1");
> >          dataCacheTimeout.set(-1);
> > +        dataCacheTimeout.setDynamic(true);
> >  
> >          queryCachePlugin = addPlugin("QueryCache", true);
> >          aliases = new String[] {
> > @@ -387,6 +388,7 @@
> >          fetchBatchSize = addInt("FetchBatchSize");
> >          fetchBatchSize.setDefault("-1");
> >          fetchBatchSize.set(-1);
> > +        fetchBatchSize.setDynamic(true);
> >  
> >          maxFetchDepth = addInt("MaxFetchDepth");
> >          maxFetchDepth.setDefault("-1"); @@ -410,7 +412,8 @@
> >          lockTimeout = addInt("LockTimeout");
> >          lockTimeout.setDefault("-1");
> >          lockTimeout.set(-1);
> > -
> > +        lockTimeout.setDynamic(true);
> > +        
> >          readLockLevel = addInt("ReadLockLevel");
> >          aliases =
> >              new String[] {
> >  
> > 
> > 
> > Pinaki Poddar
> > 972.834.2865
> > 

Notice:  This email message, together with any attachments, may contain information  of  BEA
Systems,  Inc.,  its subsidiaries  and  affiliated entities,  that may be confidential,  proprietary,
 copyrighted  and/or legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient, and have received
this message in error, please immediately return this by email and then delete it.

Mime
View raw message