You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ojb-dev@db.apache.org by ar...@apache.org on 2004/01/04 00:20:49 UTC

cvs commit: db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess AnonymousPersistentFieldForInheritance.java AnonymousPersistentField.java

arminw      2004/01/03 15:20:48

  Modified:    src/java/org/apache/ojb/broker/metadata/fieldaccess
                        AnonymousPersistentFieldForInheritance.java
                        AnonymousPersistentField.java
  Log:
  workaround for problem posted by Andy Malakov. AnonymousPersistentField
  implementation use the pc object instance as key in a WeakHashMap. This
  can cause problems when user override equals() method in pc objects (details see
  comment in AnonymousPersistentField class)
  
  Revision  Changes    Path
  1.8       +4 -4      db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentFieldForInheritance.java
  
  Index: AnonymousPersistentFieldForInheritance.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentFieldForInheritance.java,v
  retrieving revision 1.7
  retrieving revision 1.8
  diff -u -r1.7 -r1.8
  --- AnonymousPersistentFieldForInheritance.java	9 Dec 2003 17:19:58 -0000	1.7
  +++ AnonymousPersistentFieldForInheritance.java	3 Jan 2004 23:20:48 -0000	1.8
  @@ -95,7 +95,7 @@
       public synchronized void set(Object obj, Object value) throws MetadataException
       {
           copyObjectToObject(value, obj);
  -        getFKCacheMap().put(obj, value);
  +        putToFieldCache(obj, value);
       }
   
       /**
  @@ -107,7 +107,7 @@
        */
       public synchronized Object get(Object obj) throws MetadataException
       {
  -        Object value = getFKCacheMap().get(obj);
  +        Object value = getFromFieldCache(obj);
           if (null == value)
           {
               try
  @@ -118,7 +118,7 @@
               {
                   throw new MetadataException(e);
               }
  -            fkCache.put(obj, value);
  +            putToFieldCache(obj, value);
           }
   
           copyObjectToObject(obj, value);
  
  
  
  1.7       +73 -9     db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentField.java
  
  Index: AnonymousPersistentField.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentField.java,v
  retrieving revision 1.6
  retrieving revision 1.7
  diff -u -r1.6 -r1.7
  --- AnonymousPersistentField.java	9 Dec 2003 17:19:58 -0000	1.6
  +++ AnonymousPersistentField.java	3 Jan 2004 23:20:48 -0000	1.7
  @@ -54,11 +54,11 @@
    * <http://www.apache.org/>.
    */
   
  -import org.apache.ojb.broker.metadata.MetadataException;
  -
   import java.util.Map;
   import java.util.WeakHashMap;
   
  +import org.apache.ojb.broker.metadata.MetadataException;
  +
   /**
    * This class handle an anonymous persistent fiels for 1-1 association,
    * and ojbConcreteClass
  @@ -67,8 +67,8 @@
    */
   public class AnonymousPersistentField implements PersistentField
   {
  -    protected transient Map fkCache;
  -    protected String fieldname;
  +    private transient Map fkCache;
  +    private String fieldname;
   
       public AnonymousPersistentField(String fieldname)
       {
  @@ -77,21 +77,26 @@
   
       public synchronized void set(Object obj, Object value) throws MetadataException
       {
  -        getFKCacheMap().put(obj, value);
  +        putToFieldCache(obj, value);
       }
   
       public synchronized Object get(Object anObject) throws MetadataException
       {
  -        return getFKCacheMap().get(anObject);
  +        return getFromFieldCache(anObject);
       }
   
  -    protected Map getFKCacheMap()
  +    protected void  putToFieldCache(Object key, Object value)
       {
           if(fkCache == null)
           {
               fkCache = new WeakHashMap();
           }
  -        return fkCache;
  +        fkCache.put(new IdentityKey(key), value);
  +    }
  +
  +    protected Object getFromFieldCache(Object key)
  +    {
  +        return fkCache != null ? fkCache.get(new IdentityKey(key)) : null;
       }
   
       /**
  @@ -128,4 +133,63 @@
       {
           return false;
       }
  +
  +    //******************************************************************
  +    // inner class
  +    //******************************************************************
  +    /*
  +    TODO: replace this workaround with a IdentityWeakHashMap
  +    arminw:
  +    Workaround till we can use a IdentityWeakHashMap implementation.
  +    WeakHashMap use obj.equals(...) method to compare the object key in the
  +    map. Better to use an identity comparison.
  +
  +    Here is an snip of the mail from Andy Malakov:
  +
  +I found that usage of database identity in Java produces quite interesting problem in OJB:
  +In my application all persistent Java objects use database identity instead of Java reference identity
  +(i.e. Persistable.equals() is redefined so that two persistent objects are the same if they have the same
  +primary key and top-level class).
  +
  +In OJB, for each field declared in repository there is dedicated instance of AnonymousPersistentField that stores
  +object-to-field-value mapping in WeakHashMap (in fkCache attribute). Despite usage of cache
  +(ObjectCachePerBrokerImpl in my case) it is possible that identical DB objects will end up as different
  +Java objects during retrieval of complex objects.
  +
  +Now imagine what happens when two identical instances are retrieved:
  +1)
  +When first instance is retrieved it stores its foreign keys in AnonymousPersistentField.fkCache under instance's
  +identity. (happens in RowReaderDefaultImpl.buildWithReflection())
  +2)
  +When second object is retrieved and stored in fkCache, first instance is probably still cached
  +[WeakHashMap entries are cleaned up only during GC]. Since keys are identical WeakHashMap only updates entry
  +value and DOES NOT update entry key.
  +3)
  +If Full GC happens after that moment it will dispose fcCache entry if the FIRST reference becomes
  +soft-referenced only.
  +    */
  +    static class IdentityKey
  +    {
  +        Object obj;
  +
  +        public IdentityKey(Object obj)
  +        {
  +            this.obj = obj;
  +        }
  +
  +        public boolean equals(Object obj)
  +        {
  +            if(obj instanceof IdentityKey)
  +            {
  +                return this.obj == ((IdentityKey)obj).obj;
  +            }
  +            else return false;
  +        }
  +
  +        public int hashCode()
  +		{
  +			return System.identityHashCode(obj);
  +		}
  +    }
   }
  +
  
  
  

---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: cvs commit: db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess AnonymousPersistentFieldForInheritance.java AnonymousPersistentField.java

Posted by Armin Waibel <ar...@code-au-lait.de>.
Hi Andy,

Andy Malakov wrote:
> Hello Armin,
> 
> Thank you very much for submission. I will verify my application with the latest version of OJB as soon as I can.
> 
> Regarding Apache commons-based implementation of WeakIdentityHashMap. I just gave it another look. Unfortunately we have to
> copy-paste and rewrite 50% of the ReferenceMap to make it working as desired. (most relevant method getEntry (Ojbect key) is
> declared as private, usage of .hashcode() and .equals() everywhere ....). Makes me think that would be a nasty solution.
> 
> What do you think?

I agree with you. If we need to rewrite 50% of the code instead of 
simply extend/override a few methods I think it's better to use the 
workaround with the wrapper class.
I feel confident that we will find a better solution till version 1.1 ;-)

regards,
Armin

> 
> 
> ----- Original Message ----- 
> From: <ar...@apache.org>
> To: <db...@apache.org>
> Sent: Saturday, January 03, 2004 6:20 PM
> Subject: cvs commit: db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess AnonymousPersistentFieldForInheritance.java
> AnonymousPersistentField.java
> 
> 
> 
>>arminw      2004/01/03 15:20:48
>>
>>  Modified:    src/java/org/apache/ojb/broker/metadata/fieldaccess
>>                        AnonymousPersistentFieldForInheritance.java
>>                        AnonymousPersistentField.java
>>  Log:
>>  workaround for problem posted by Andy Malakov. AnonymousPersistentField
>>  implementation use the pc object instance as key in a WeakHashMap. This
>>  can cause problems when user override equals() method in pc objects (details see
>>  comment in AnonymousPersistentField class)
>>
>>  Revision  Changes    Path
>>  1.8       +4 -4      db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentFieldForInheritance.java
>>
>>  Index: AnonymousPersistentFieldForInheritance.java
>>  ===================================================================
>>  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentFieldForInheritance.java,v
>>  retrieving revision 1.7
>>  retrieving revision 1.8
>>  diff -u -r1.7 -r1.8
>>  --- AnonymousPersistentFieldForInheritance.java 9 Dec 2003 17:19:58 -0000 1.7
>>  +++ AnonymousPersistentFieldForInheritance.java 3 Jan 2004 23:20:48 -0000 1.8
>>  @@ -95,7 +95,7 @@
>>       public synchronized void set(Object obj, Object value) throws MetadataException
>>       {
>>           copyObjectToObject(value, obj);
>>  -        getFKCacheMap().put(obj, value);
>>  +        putToFieldCache(obj, value);
>>       }
>>
>>       /**
>>  @@ -107,7 +107,7 @@
>>        */
>>       public synchronized Object get(Object obj) throws MetadataException
>>       {
>>  -        Object value = getFKCacheMap().get(obj);
>>  +        Object value = getFromFieldCache(obj);
>>           if (null == value)
>>           {
>>               try
>>  @@ -118,7 +118,7 @@
>>               {
>>                   throw new MetadataException(e);
>>               }
>>  -            fkCache.put(obj, value);
>>  +            putToFieldCache(obj, value);
>>           }
>>
>>           copyObjectToObject(obj, value);
>>
>>
>>
>>  1.7       +73 -9     db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentField.java
>>
>>  Index: AnonymousPersistentField.java
>>  ===================================================================
>>  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentField.java,v
>>  retrieving revision 1.6
>>  retrieving revision 1.7
>>  diff -u -r1.6 -r1.7
>>  --- AnonymousPersistentField.java 9 Dec 2003 17:19:58 -0000 1.6
>>  +++ AnonymousPersistentField.java 3 Jan 2004 23:20:48 -0000 1.7
>>  @@ -54,11 +54,11 @@
>>    * <http://www.apache.org/>.
>>    */
>>
>>  -import org.apache.ojb.broker.metadata.MetadataException;
>>  -
>>   import java.util.Map;
>>   import java.util.WeakHashMap;
>>
>>  +import org.apache.ojb.broker.metadata.MetadataException;
>>  +
>>   /**
>>    * This class handle an anonymous persistent fiels for 1-1 association,
>>    * and ojbConcreteClass
>>  @@ -67,8 +67,8 @@
>>    */
>>   public class AnonymousPersistentField implements PersistentField
>>   {
>>  -    protected transient Map fkCache;
>>  -    protected String fieldname;
>>  +    private transient Map fkCache;
>>  +    private String fieldname;
>>
>>       public AnonymousPersistentField(String fieldname)
>>       {
>>  @@ -77,21 +77,26 @@
>>
>>       public synchronized void set(Object obj, Object value) throws MetadataException
>>       {
>>  -        getFKCacheMap().put(obj, value);
>>  +        putToFieldCache(obj, value);
>>       }
>>
>>       public synchronized Object get(Object anObject) throws MetadataException
>>       {
>>  -        return getFKCacheMap().get(anObject);
>>  +        return getFromFieldCache(anObject);
>>       }
>>
>>  -    protected Map getFKCacheMap()
>>  +    protected void  putToFieldCache(Object key, Object value)
>>       {
>>           if(fkCache == null)
>>           {
>>               fkCache = new WeakHashMap();
>>           }
>>  -        return fkCache;
>>  +        fkCache.put(new IdentityKey(key), value);
>>  +    }
>>  +
>>  +    protected Object getFromFieldCache(Object key)
>>  +    {
>>  +        return fkCache != null ? fkCache.get(new IdentityKey(key)) : null;
>>       }
>>
>>       /**
>>  @@ -128,4 +133,63 @@
>>       {
>>           return false;
>>       }
>>  +
>>  +    //******************************************************************
>>  +    // inner class
>>  +    //******************************************************************
>>  +    /*
>>  +    TODO: replace this workaround with a IdentityWeakHashMap
>>  +    arminw:
>>  +    Workaround till we can use a IdentityWeakHashMap implementation.
>>  +    WeakHashMap use obj.equals(...) method to compare the object key in the
>>  +    map. Better to use an identity comparison.
>>  +
>>  +    Here is an snip of the mail from Andy Malakov:
>>  +
>>  +I found that usage of database identity in Java produces quite interesting problem in OJB:
>>  +In my application all persistent Java objects use database identity instead of Java reference identity
>>  +(i.e. Persistable.equals() is redefined so that two persistent objects are the same if they have the same
>>  +primary key and top-level class).
>>  +
>>  +In OJB, for each field declared in repository there is dedicated instance of AnonymousPersistentField that stores
>>  +object-to-field-value mapping in WeakHashMap (in fkCache attribute). Despite usage of cache
>>  +(ObjectCachePerBrokerImpl in my case) it is possible that identical DB objects will end up as different
>>  +Java objects during retrieval of complex objects.
>>  +
>>  +Now imagine what happens when two identical instances are retrieved:
>>  +1)
>>  +When first instance is retrieved it stores its foreign keys in AnonymousPersistentField.fkCache under instance's
>>  +identity. (happens in RowReaderDefaultImpl.buildWithReflection())
>>  +2)
>>  +When second object is retrieved and stored in fkCache, first instance is probably still cached
>>  +[WeakHashMap entries are cleaned up only during GC]. Since keys are identical WeakHashMap only updates entry
>>  +value and DOES NOT update entry key.
>>  +3)
>>  +If Full GC happens after that moment it will dispose fcCache entry if the FIRST reference becomes
>>  +soft-referenced only.
>>  +    */
>>  +    static class IdentityKey
>>  +    {
>>  +        Object obj;
>>  +
>>  +        public IdentityKey(Object obj)
>>  +        {
>>  +            this.obj = obj;
>>  +        }
>>  +
>>  +        public boolean equals(Object obj)
>>  +        {
>>  +            if(obj instanceof IdentityKey)
>>  +            {
>>  +                return this.obj == ((IdentityKey)obj).obj;
>>  +            }
>>  +            else return false;
>>  +        }
>>  +
>>  +        public int hashCode()
>>  + {
>>  + return System.identityHashCode(obj);
>>  + }
>>  +    }
>>   }
>>  +
>>
>>
>>
>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>>For additional commands, e-mail: ojb-dev-help@db.apache.org
>>
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
> 
> 
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: cvs commit: db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess AnonymousPersistentFieldForInheritance.java AnonymousPersistentField.java

Posted by Andy Malakov <an...@transdecisions.com>.
Hello Armin,

Thank you very much for submission. I will verify my application with the latest version of OJB as soon as I can.

Regarding Apache commons-based implementation of WeakIdentityHashMap. I just gave it another look. Unfortunately we have to
copy-paste and rewrite 50% of the ReferenceMap to make it working as desired. (most relevant method getEntry (Ojbect key) is
declared as private, usage of .hashcode() and .equals() everywhere ....). Makes me think that would be a nasty solution.

What do you think?


----- Original Message ----- 
From: <ar...@apache.org>
To: <db...@apache.org>
Sent: Saturday, January 03, 2004 6:20 PM
Subject: cvs commit: db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess AnonymousPersistentFieldForInheritance.java
AnonymousPersistentField.java


> arminw      2004/01/03 15:20:48
>
>   Modified:    src/java/org/apache/ojb/broker/metadata/fieldaccess
>                         AnonymousPersistentFieldForInheritance.java
>                         AnonymousPersistentField.java
>   Log:
>   workaround for problem posted by Andy Malakov. AnonymousPersistentField
>   implementation use the pc object instance as key in a WeakHashMap. This
>   can cause problems when user override equals() method in pc objects (details see
>   comment in AnonymousPersistentField class)
>
>   Revision  Changes    Path
>   1.8       +4 -4      db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentFieldForInheritance.java
>
>   Index: AnonymousPersistentFieldForInheritance.java
>   ===================================================================
>   RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentFieldForInheritance.java,v
>   retrieving revision 1.7
>   retrieving revision 1.8
>   diff -u -r1.7 -r1.8
>   --- AnonymousPersistentFieldForInheritance.java 9 Dec 2003 17:19:58 -0000 1.7
>   +++ AnonymousPersistentFieldForInheritance.java 3 Jan 2004 23:20:48 -0000 1.8
>   @@ -95,7 +95,7 @@
>        public synchronized void set(Object obj, Object value) throws MetadataException
>        {
>            copyObjectToObject(value, obj);
>   -        getFKCacheMap().put(obj, value);
>   +        putToFieldCache(obj, value);
>        }
>
>        /**
>   @@ -107,7 +107,7 @@
>         */
>        public synchronized Object get(Object obj) throws MetadataException
>        {
>   -        Object value = getFKCacheMap().get(obj);
>   +        Object value = getFromFieldCache(obj);
>            if (null == value)
>            {
>                try
>   @@ -118,7 +118,7 @@
>                {
>                    throw new MetadataException(e);
>                }
>   -            fkCache.put(obj, value);
>   +            putToFieldCache(obj, value);
>            }
>
>            copyObjectToObject(obj, value);
>
>
>
>   1.7       +73 -9     db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentField.java
>
>   Index: AnonymousPersistentField.java
>   ===================================================================
>   RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/metadata/fieldaccess/AnonymousPersistentField.java,v
>   retrieving revision 1.6
>   retrieving revision 1.7
>   diff -u -r1.6 -r1.7
>   --- AnonymousPersistentField.java 9 Dec 2003 17:19:58 -0000 1.6
>   +++ AnonymousPersistentField.java 3 Jan 2004 23:20:48 -0000 1.7
>   @@ -54,11 +54,11 @@
>     * <http://www.apache.org/>.
>     */
>
>   -import org.apache.ojb.broker.metadata.MetadataException;
>   -
>    import java.util.Map;
>    import java.util.WeakHashMap;
>
>   +import org.apache.ojb.broker.metadata.MetadataException;
>   +
>    /**
>     * This class handle an anonymous persistent fiels for 1-1 association,
>     * and ojbConcreteClass
>   @@ -67,8 +67,8 @@
>     */
>    public class AnonymousPersistentField implements PersistentField
>    {
>   -    protected transient Map fkCache;
>   -    protected String fieldname;
>   +    private transient Map fkCache;
>   +    private String fieldname;
>
>        public AnonymousPersistentField(String fieldname)
>        {
>   @@ -77,21 +77,26 @@
>
>        public synchronized void set(Object obj, Object value) throws MetadataException
>        {
>   -        getFKCacheMap().put(obj, value);
>   +        putToFieldCache(obj, value);
>        }
>
>        public synchronized Object get(Object anObject) throws MetadataException
>        {
>   -        return getFKCacheMap().get(anObject);
>   +        return getFromFieldCache(anObject);
>        }
>
>   -    protected Map getFKCacheMap()
>   +    protected void  putToFieldCache(Object key, Object value)
>        {
>            if(fkCache == null)
>            {
>                fkCache = new WeakHashMap();
>            }
>   -        return fkCache;
>   +        fkCache.put(new IdentityKey(key), value);
>   +    }
>   +
>   +    protected Object getFromFieldCache(Object key)
>   +    {
>   +        return fkCache != null ? fkCache.get(new IdentityKey(key)) : null;
>        }
>
>        /**
>   @@ -128,4 +133,63 @@
>        {
>            return false;
>        }
>   +
>   +    //******************************************************************
>   +    // inner class
>   +    //******************************************************************
>   +    /*
>   +    TODO: replace this workaround with a IdentityWeakHashMap
>   +    arminw:
>   +    Workaround till we can use a IdentityWeakHashMap implementation.
>   +    WeakHashMap use obj.equals(...) method to compare the object key in the
>   +    map. Better to use an identity comparison.
>   +
>   +    Here is an snip of the mail from Andy Malakov:
>   +
>   +I found that usage of database identity in Java produces quite interesting problem in OJB:
>   +In my application all persistent Java objects use database identity instead of Java reference identity
>   +(i.e. Persistable.equals() is redefined so that two persistent objects are the same if they have the same
>   +primary key and top-level class).
>   +
>   +In OJB, for each field declared in repository there is dedicated instance of AnonymousPersistentField that stores
>   +object-to-field-value mapping in WeakHashMap (in fkCache attribute). Despite usage of cache
>   +(ObjectCachePerBrokerImpl in my case) it is possible that identical DB objects will end up as different
>   +Java objects during retrieval of complex objects.
>   +
>   +Now imagine what happens when two identical instances are retrieved:
>   +1)
>   +When first instance is retrieved it stores its foreign keys in AnonymousPersistentField.fkCache under instance's
>   +identity. (happens in RowReaderDefaultImpl.buildWithReflection())
>   +2)
>   +When second object is retrieved and stored in fkCache, first instance is probably still cached
>   +[WeakHashMap entries are cleaned up only during GC]. Since keys are identical WeakHashMap only updates entry
>   +value and DOES NOT update entry key.
>   +3)
>   +If Full GC happens after that moment it will dispose fcCache entry if the FIRST reference becomes
>   +soft-referenced only.
>   +    */
>   +    static class IdentityKey
>   +    {
>   +        Object obj;
>   +
>   +        public IdentityKey(Object obj)
>   +        {
>   +            this.obj = obj;
>   +        }
>   +
>   +        public boolean equals(Object obj)
>   +        {
>   +            if(obj instanceof IdentityKey)
>   +            {
>   +                return this.obj == ((IdentityKey)obj).obj;
>   +            }
>   +            else return false;
>   +        }
>   +
>   +        public int hashCode()
>   + {
>   + return System.identityHashCode(obj);
>   + }
>   +    }
>    }
>   +
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org