You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2009/11/17 09:40:11 UTC

svn commit: r881195 - in /ofbiz/branches/release09.04: ./ framework/base/src/org/ofbiz/base/util/cache/UtilCache.java

Author: jleroux
Date: Tue Nov 17 08:40:11 2009
New Revision: 881195

URL: http://svn.apache.org/viewvc?rev=881195&view=rev
Log:
"Applied fix from trunk for revision: 881194" 
------------------------------------------------------------------------
r881194 | jleroux | 2009-11-17 09:36:35 +0100 (mar. 17 nov. 2009) | 10 lignes

A patch from Philippe Mouawad "Ofbiz freeze" (https://issues.apache.org/jira/browse/OFBIZ-2124) - OFBIZ-2124

In a certain scenario the application freezes, no more AJP connector threads are available.
After analysis the lock seems to come from very slow response time in GenericDelegator.clearAllCaches and particularily in CacheLineTable.keySet.
Furthermore, looking at org.ofbiz.base.util.cache.UtilCache, the class seems to have some synchronisation problems:
1) utilCacheTable.put calls are not synchronized
2) findCache synchronizes access to utilCacheTable while only getting an element which has big performance impact since utilCacheTable is static
3) clearExpiredFromAllCaches and clearAllCaches does not synchronize while clearCachesThatStartWith does, why ?

The patch synchronizes correctly the access to utilCacheTable and optimises findCache 
------------------------------------------------------------------------


Modified:
    ofbiz/branches/release09.04/   (props changed)
    ofbiz/branches/release09.04/framework/base/src/org/ofbiz/base/util/cache/UtilCache.java

Propchange: ofbiz/branches/release09.04/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Nov 17 08:40:11 2009
@@ -1 +1 @@
-/ofbiz/trunk:765933,766011,766015,766293,766307,766316,766325,766462,766522,766800,767060,767072,767093,767098-767099,767102,767123,767125,767127,767279,767287,767671,767688,767694,767822,767845,768358,768490,768550,768675,768686,768705,768811,768815,768960,769030,769500,770272,770308,770997,771073,772401,772464-772465,773076,773557,773628,773659,773697,774014,774632,774661,774995,775292,775667,776227,776594,776620,776922,777004,777020,777768,777792,777893,777947,778078,778094,778107,778273,778278,778280,778364,778374,778402,778576,778594,778628,779020,779477,779496,779639,779834,779856,779866,779873,780111,780138,780180,780199,780203,780906,780945,781201,781534,781549,781669,781680,781694,782663,783257,783266,783833,783913,783917,785123,785764,785967,786778,787126,787435-787436,787442,787520,788965,788983,788987,789329,789337,789506,789548,796769,799185,800461,800846,801023,802346,804364,805307,806127,806377,808786-808787,808792,809141,810370,810438,810465,810807,810809,810
 814,810832,810836,810878,810917,811020,811280,811297,811419,811528,811708,811714,811716,811793,811838,811860,811865,811870,812159,812182,812192,812456,812540,813126,813131,813283,813672,813702,814168,814205,814251,814349,814531,814576,814681,814731,815158,815165,815350,815687,815977,816255,816863,818030,818049,818150,818494,818500,818716,818976,819275-819276,819282,819337,821263,821270,822659,823877-823878,823883,823888,823892,824511,825181-825182,826253,827730,828971,829085,829376,829412,829416,829527,830091,830112,830366,830528,830677,830874,830880,831238,831801,832361,832698,832776,832908,833324,833686,833703,834825,835161,835357,835585,836015
+/ofbiz/trunk:765933,766011,766015,766293,766307,766316,766325,766462,766522,766800,767060,767072,767093,767098-767099,767102,767123,767125,767127,767279,767287,767671,767688,767694,767822,767845,768358,768490,768550,768675,768686,768705,768811,768815,768960,769030,769500,770272,770308,770997,771073,772401,772464-772465,773076,773557,773628,773659,773697,774014,774632,774661,774995,775292,775667,776227,776594,776620,776922,777004,777020,777768,777792,777893,777947,778078,778094,778107,778273,778278,778280,778364,778374,778402,778576,778594,778628,779020,779477,779496,779639,779834,779856,779866,779873,780111,780138,780180,780199,780203,780906,780945,781201,781534,781549,781669,781680,781694,782663,783257,783266,783833,783913,783917,785123,785764,785967,786778,787126,787435-787436,787442,787520,788965,788983,788987,789329,789337,789506,789548,796769,799185,800461,800846,801023,802346,804364,805307,806127,806377,808786-808787,808792,809141,810370,810438,810465,810807,810809,810
 814,810832,810836,810878,810917,811020,811280,811297,811419,811528,811708,811714,811716,811793,811838,811860,811865,811870,812159,812182,812192,812456,812540,813126,813131,813283,813672,813702,814168,814205,814251,814349,814531,814576,814681,814731,815158,815165,815350,815687,815977,816255,816863,818030,818049,818150,818494,818500,818716,818976,819275-819276,819282,819337,821263,821270,822659,823877-823878,823883,823888,823892,824511,825181-825182,826253,827730,828971,829085,829376,829412,829416,829527,830091,830112,830366,830528,830677,830874,830880,831238,831801,832361,832698,832776,832908,833324,833686,833703,834825,835161,835357,835585,836015,881194

Modified: ofbiz/branches/release09.04/framework/base/src/org/ofbiz/base/util/cache/UtilCache.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release09.04/framework/base/src/org/ofbiz/base/util/cache/UtilCache.java?rev=881195&r1=881194&r2=881195&view=diff
==============================================================================
--- ofbiz/branches/release09.04/framework/base/src/org/ofbiz/base/util/cache/UtilCache.java (original)
+++ ofbiz/branches/release09.04/framework/base/src/org/ofbiz/base/util/cache/UtilCache.java Tue Nov 17 08:40:11 2009
@@ -19,6 +19,7 @@
 package org.ofbiz.base.util.cache;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -115,7 +116,9 @@
 
         setPropertiesParams(cacheName);
 
-        utilCacheTable.put(name, this);
+        synchronized (utilCacheTable) {
+            utilCacheTable.put(name, this);
+        }
     }
 
     public UtilCache(String cacheName, int maxSize, long expireTime, boolean useSoftReference) {
@@ -143,8 +146,9 @@
         String name = "specified" + this.getNextDefaultIndex("specified");
 
         setPropertiesParams(name);
-
-        utilCacheTable.put(name, this);
+        synchronized (utilCacheTable) {
+            utilCacheTable.put(name, this);
+        }
     }
 
     /** This constructor takes a name for the cache, puts itself in the utilCacheTable.
@@ -157,8 +161,9 @@
 
         setPropertiesParams("default");
         setPropertiesParams(cacheName);
-
-        utilCacheTable.put(name, this);
+        synchronized (utilCacheTable) {
+            utilCacheTable.put(name, this);
+        }
     }
 
     /** This constructor takes a name for the cache, puts itself in the utilCacheTable.
@@ -170,8 +175,9 @@
 
         setPropertiesParams("default");
         setPropertiesParams(cacheName);
-
-        utilCacheTable.put(name, this);
+        synchronized (utilCacheTable) {
+            utilCacheTable.put(name, this);
+        }
     }
 
     /** Default constructor, all members stay at default values as defined in cache.properties, or the defaults in this file if cache.properties is not found, or there are no 'default' entries in it. */
@@ -179,7 +185,9 @@
         setPropertiesParams("default");
 
         name = "default" + this.getNextDefaultIndex("default");
-        utilCacheTable.put(name, this);
+        synchronized (utilCacheTable) {
+            utilCacheTable.put(name, this);
+        }
     }
 
     protected String getNextDefaultIndex(String cacheName) {
@@ -398,10 +406,24 @@
 
     /** Removes all elements from this cache */
     public static void clearAllCaches() {
-        for (Map.Entry<String, UtilCache<?, ?>> entry: utilCacheTable.entrySet()) {
-            UtilCache<?, ?> utilCache = entry.getValue();
-            utilCache.clear();
+        // We make a copy since clear may take time
+        List<UtilCache<?,?>> list = getUtilCacheTableValuesImage();
+        for (UtilCache<?,?> cache : list) {
+            cache.clear();
+        }
+        list.clear();
+    }
+
+    /**
+     * Return an image of the values at a time
+     * @return {@link List}
+     */
+    private static List getUtilCacheTableValuesImage() {
+        List list = new ArrayList(utilCacheTable.size());
+        synchronized (utilCacheTable) {
+            list.addAll(utilCacheTable.values());
         }
+        return list;
     }
 
     /** Getter for the name of the UtilCache instance.
@@ -632,10 +654,12 @@
 
     /** Clears all expired cache entries from all caches */
     public static void clearExpiredFromAllCaches() {
-        for (Map.Entry<String, UtilCache<?, ?>> entry: utilCacheTable.entrySet()) {
-            UtilCache<?, ?> utilCache = entry.getValue();
+        // We make a copy since clear may take time
+        List<UtilCache<?,?>> list = getUtilCacheTableValuesImage();
+        for (UtilCache<?,?> utilCache : list) {
             utilCache.clearExpired();
         }
+        list.clear();
     }
 
     /** Checks for a non-expired key in a specific cache */
@@ -650,13 +674,20 @@
 
     public static void clearCachesThatStartWith(String startsWith) {
         synchronized (utilCacheTable) {
-            for (Map.Entry<String, UtilCache<?, ?>> entry: utilCacheTable.entrySet()) {
-                String name = entry.getKey();
-                if (name.startsWith(startsWith)) {
-                    UtilCache<?, ?> cache = entry.getValue();
-                    cache.clear();
+            List<UtilCache<?, ?>> cachesToClear = FastList.newInstance();
+            synchronized (utilCacheTable) {
+                for (Map.Entry<String, UtilCache<?, ?>> entry: utilCacheTable.entrySet()) {
+                    String name = entry.getKey();
+                    if (name.startsWith(startsWith)) {
+                        UtilCache<?, ?> cache = entry.getValue();
+                        cachesToClear.add(cache);
+                    }
                 }
             }
+            for (UtilCache<?,?> cache : cachesToClear) {
+                cache.clear();
+            }
+            cachesToClear.clear();
         }
     }
 
@@ -668,8 +699,6 @@
 
     @SuppressWarnings("unchecked")
     public static <K, V> UtilCache<K, V> findCache(String cacheName) {
-        synchronized (UtilCache.utilCacheTable) {
-            return (UtilCache<K, V>) UtilCache.utilCacheTable.get(cacheName);
-        }
+        return (UtilCache<K, V>) UtilCache.utilCacheTable.get(cacheName);
     }
 }