svn commit: r921990 - in /ofbiz/trunk/framework/entity/src/org/ofbiz/entity: DelegatorFactory.java DelegatorFactoryImpl.java GenericDelegator.java

[doogie-3]

Author: doogie
Date: Thu Mar 11 18:59:34 2010
New Revision: 921990

URL: http://svn.apache.org/viewvc?rev=921990&view=rev
Log:
Fix bad caching.  Previously, any call to getDelegator() would *always*
walk the entire classpath, trying to find all implementations of
DelegatorFactory.  This was fixed by moving the cache higher up.

In detail, the entity caching system only stored a String delegator
name.  It would then try to find the appropriate delegator to match that
name.  However, during a custom importer, the caching system was being
exercised very heavily, so DelegatorFactory.getDelegator() was being
called in a very tight loop.  Since the delegator instances were not
being cached at the high level, this caused a *big* slowdown; basically,
a 15 minute importer was taking over 2 hours, and was no near finishing.
This was because UtilObject.getObjectFromFactory would end up walking
the entire classpath, finding every jar, looking at every zip entry, to
try and find the appropriate file in META-INF/services.

Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactory.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactoryImpl.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactory.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactory.java?rev=921990&r1=921989&r2=921990&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactory.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactory.java Thu Mar 11 18:59:34 2010
@@ -18,6 +18,8 @@
  */
 package org.ofbiz.entity;
 
+import java.util.concurrent.ConcurrentHashMap;
+
 import org.ofbiz.base.util.Debug;
 import org.ofbiz.base.util.Factory;
 import org.ofbiz.base.util.UtilObject;
@@ -25,13 +27,29 @@ import org.ofbiz.base.util.UtilObject;
 /** <code>Delegator</code> factory abstract class. */
 public abstract class DelegatorFactory implements Factory<Delegator, String> {
     public static final String module = DelegatorFactoryImpl.class.getName();
+    private static final ConcurrentHashMap<String, Delegator> delegatorCache = new ConcurrentHashMap<String, Delegator>();
 
     public static Delegator getDelegator(String delegatorName) {
-        try {
-            return UtilObject.getObjectFromFactory(DelegatorFactory.class, delegatorName);
-        } catch (ClassNotFoundException e) {
-            Debug.logError(e, module);
+        if (delegatorName == null) {
+            delegatorName = "default";
+            //Debug.logWarning(new Exception("Location where getting delegator with null name"), "Got a getGenericDelegator call with a null delegatorName, assuming default for the name.", module);
         }
-        return null;
+        do {
+            Delegator delegator = delegatorCache.get(delegatorName);
+
+            if (delegator != null) {
+                // setup the Entity ECA Handler
+                delegator.initEntityEcaHandler();
+                //Debug.logInfo("got delegator(" + delegatorName + ") from cache", module);
+                return delegator;
+            }
+            try {
+                delegator = UtilObject.getObjectFromFactory(DelegatorFactory.class, delegatorName);
+            } catch (ClassNotFoundException e) {
+                Debug.logError(e, module);
+            }
+            //Debug.logInfo("putting delegator(" + delegatorName + ") into cache", module);
+            delegatorCache.putIfAbsent(delegatorName, delegator);
+        } while (true);
     }
 }

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactoryImpl.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactoryImpl.java?rev=921990&r1=921989&r2=921990&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactoryImpl.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactoryImpl.java Thu Mar 11 18:59:34 2010
@@ -27,32 +27,13 @@ public class DelegatorFactoryImpl extend
     public static final String module = DelegatorFactoryImpl.class.getName();
 
     public Delegator getInstance(String delegatorName) {
-        if (delegatorName == null) {
-            delegatorName = "default";
-            Debug.logWarning(new Exception("Location where getting delegator with null name"), "Got a getGenericDelegator call with a null delegatorName, assuming default for the name.", module);
+        if (Debug.infoOn()) Debug.logInfo("Creating new delegator [" + delegatorName + "] (" + Thread.currentThread().getName() + ")", module);
+        //Debug.logInfo(new Exception(), "Showing stack where new delegator is being created...", module);
+        try {
+            return new GenericDelegator(delegatorName);
+        } catch (GenericEntityException e) {
+            Debug.logError(e, "Error creating delegator", module);
+            return null;
         }
-        GenericDelegator delegator = GenericDelegator.delegatorCache.get(delegatorName);
-        if (delegator == null) {
-            synchronized (GenericDelegator.delegatorCache) {
-                // must check if null again as one of the blocked threads can still enter
-                delegator = GenericDelegator.delegatorCache.get(delegatorName);
-                if (delegator == null) {
-                    if (Debug.infoOn()) Debug.logInfo("Creating new delegator [" + delegatorName + "] (" + Thread.currentThread().getName() + ")", module);
-                    //Debug.logInfo(new Exception(), "Showing stack where new delegator is being created...", module);
-                    try {
-                        delegator = new GenericDelegator(delegatorName);
-                    } catch (GenericEntityException e) {
-                        Debug.logError(e, "Error creating delegator", module);
-                    }
-                    if (delegator != null) {
-                        GenericDelegator.delegatorCache.put(delegatorName, delegator);
-                    } else {
-                        Debug.logError("Could not create delegator with name " + delegatorName + ", constructor failed (got null value) not sure why/how.", module);
-                    }
-                }
-            }
-        }
-        return delegator;
     }
-
 }

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=921990&r1=921989&r2=921990&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Thu Mar 11 18:59:34 2010
@@ -95,9 +95,6 @@ public class GenericDelegator implements
     /** This flag is only here for lower level technical testing, it shouldn't be user configurable (or at least I don't think so yet); when true all operations without a transaction will be wrapped in one; seems to be necessary for some (all?) XA aware connection pools, and should improve overall stability and consistency */
     public static final boolean alwaysUseTransaction = true;
 
-    /** the delegatorCache will now be a HashMap, allowing reload of definitions,
-     * but the delegator will always be the same object for the given name */
-    public static Map<String, GenericDelegator> delegatorCache = FastMap.newInstance();
     protected String delegatorName = null;
     protected DelegatorInfo delegatorInfo = null;
 
@@ -274,8 +271,6 @@ public class GenericDelegator implements
         }
 
         // NOTE: doing some things before the ECAs and such to make sure it is in place just in case it is used in a service engine startup thing or something
-        // put the delegator in the master Map by its name
-        GenericDelegator.delegatorCache.put(delegatorName, this);
 
         // setup the crypto class
         this.crypto = new EntityCrypto(this);
@@ -306,15 +301,15 @@ public class GenericDelegator implements
         } else {
             Debug.logInfo("Distributed Cache Clear System disabled for delegator [" + delegatorName + "]", module);
         }
-
-        // setup the Entity ECA Handler
-        initEntityEcaHandler();
     }
 
     /* (non-Javadoc)
      * @see org.ofbiz.entity.Delegator#initEntityEcaHandler()
      */
-    public void initEntityEcaHandler() {
+    public synchronized void initEntityEcaHandler() {
+        if (!getDelegatorInfo().useEntityEca || this.entityEcaHandler != null) {
+            return;
+        }
         if (getDelegatorInfo().useEntityEca) {
             ClassLoader loader = Thread.currentThread().getContextClassLoader();
             // initialize the entity eca handler