svn commit: r1327732 - in /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache: AbstractEntityConditionCache.java EntityListCache.java

[doogie-3]

Author: doogie
Date: Wed Apr 18 23:48:20 2012
New Revision: 1327732

URL: http://svn.apache.org/viewvc?rev=1327732&view=rev
Log:
FEATURE: Remove synchronization on entity caching; this requires the
non-blocking features I added to UtilCache; without them, you'll get
internal map corruption.

Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/EntityListCache.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java?rev=1327732&r1=1327731&r2=1327732&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java Wed Apr 18 23:48:20 2012
@@ -21,6 +21,8 @@ package org.ofbiz.entity.cache;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 import javolution.util.FastMap;
 
@@ -34,7 +36,7 @@ import org.ofbiz.entity.GenericValue;
 import org.ofbiz.entity.condition.EntityCondition;
 import org.ofbiz.entity.model.ModelEntity;
 
-public abstract class AbstractEntityConditionCache<K, V> extends AbstractCache<EntityCondition, Map<K, V>> {
+public abstract class AbstractEntityConditionCache<K, V> extends AbstractCache<EntityCondition, ConcurrentMap<K, V>> {
 
     public static final String module = AbstractEntityConditionCache.class.getName();
 
@@ -43,9 +45,8 @@ public abstract class AbstractEntityCond
     }
 
     protected V get(String entityName, EntityCondition condition, K key) {
-        Map<K, V> conditionCache = getConditionCache(entityName, condition);
+        ConcurrentMap<K, V> conditionCache = getConditionCache(entityName, condition);
         if (conditionCache == null) return null;
-        // the following line was synchronized, but for pretty good safety and better performance, only syncrhnizing the put; synchronized (conditionCache) {
         return conditionCache.get(key);
     }
 
@@ -57,23 +58,19 @@ public abstract class AbstractEntityCond
         }
 
         Map<K, V> conditionCache = getOrCreateConditionCache(entityName, condition);
-        synchronized (conditionCache) {
-            return conditionCache.put(key, value);
-        }
+        return conditionCache.put(key, value);
     }
 
     public void remove(String entityName, EntityCondition condition) {
-        UtilCache<EntityCondition, Map<K, V>> cache = getCache(entityName);
+        UtilCache<EntityCondition, ConcurrentMap<K, V>> cache = getCache(entityName);
         if (cache == null) return;
         cache.remove(condition);
     }
 
     protected V remove(String entityName, EntityCondition condition, K key) {
-        Map<K, V> conditionCache = getConditionCache(entityName, condition);
+        ConcurrentMap<K, V> conditionCache = getConditionCache(entityName, condition);
         if (conditionCache == null) return null;
-        synchronized (conditionCache) {
-            return conditionCache.remove(key);
-         }
+        return conditionCache.remove(key);
     }
 
     public static final EntityCondition getConditionKey(EntityCondition condition) {
@@ -92,18 +89,18 @@ public abstract class AbstractEntityCond
         return frozenCondition;
     }
 
-    protected Map<K, V> getConditionCache(String entityName, EntityCondition condition) {
-        UtilCache<EntityCondition, Map<K, V>> cache = getCache(entityName);
+    protected ConcurrentMap<K, V> getConditionCache(String entityName, EntityCondition condition) {
+        UtilCache<EntityCondition, ConcurrentMap<K, V>> cache = getCache(entityName);
         if (cache == null) return null;
         return cache.get(getConditionKey(condition));
     }
 
     protected Map<K, V> getOrCreateConditionCache(String entityName, EntityCondition condition) {
-        UtilCache<EntityCondition, Map<K, V>> utilCache = getOrCreateCache(entityName);
+        UtilCache<EntityCondition, ConcurrentMap<K, V>> utilCache = getOrCreateCache(entityName);
         EntityCondition conditionKey = getConditionKey(condition);
-        Map<K, V> conditionCache = utilCache.get(conditionKey);
+        ConcurrentMap<K, V> conditionCache = utilCache.get(conditionKey);
         if (conditionCache == null) {
-            conditionCache = FastMap.newInstance();
+            conditionCache = new ConcurrentHashMap<K, V>();
             utilCache.put(conditionKey, conditionCache);
         }
         return conditionCache;
@@ -156,7 +153,7 @@ public abstract class AbstractEntityCond
         }
     }
 
-    public synchronized void storeHook(boolean isPK, GenericEntity oldEntity, GenericEntity newEntity) {
+    public void storeHook(boolean isPK, GenericEntity oldEntity, GenericEntity newEntity) {
         ModelEntity model = getModelCheckValid(oldEntity, newEntity);
         String entityName = model.getEntityName();
         // for info about cache clearing

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/EntityListCache.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/EntityListCache.java?rev=1327732&r1=1327731&r2=1327732&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/EntityListCache.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/EntityListCache.java Wed Apr 18 23:48:20 2012
@@ -21,6 +21,7 @@ package org.ofbiz.entity.cache;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
 
 import org.ofbiz.entity.GenericValue;
 import org.ofbiz.entity.condition.EntityCondition;
@@ -39,7 +40,7 @@ public class EntityListCache extends Abs
     }
 
     public List<GenericValue> get(String entityName, EntityCondition condition, List<String> orderBy) {
-        Map<Object, List<GenericValue>> conditionCache = getConditionCache(entityName, condition);
+        ConcurrentMap<Object, List<GenericValue>> conditionCache = getConditionCache(entityName, condition);
         if (conditionCache == null) return null;
         Object orderByKey = getOrderByKey(orderBy);
         List<GenericValue> valueList = conditionCache.get(orderByKey);
@@ -48,11 +49,12 @@ public class EntityListCache extends Abs
             Iterator<List<GenericValue>> it = conditionCache.values().iterator();
             if (it.hasNext()) valueList = it.next();
 
-            synchronized (conditionCache) {
-                if (valueList != null) {
-                    valueList = EntityUtil.orderBy(valueList, orderBy);
-                    conditionCache.put(orderByKey, valueList);
-                }
+            if (valueList != null) {
+                // Does not need to be synchronized; if 2 threads do the same ordering,
+                // the result will be exactly the same, and won't actually cause any
+                // incorrect results.
+                valueList = EntityUtil.orderBy(valueList, orderBy);
+                conditionCache.put(orderByKey, valueList);
             }
         }
         return valueList;