Author: rmannibucau Date: Wed May 28 17:06:12 2014 New Revision: 1598071 URL: http://svn.apache.org/r1598071 Log: using reentrant locks instead of old synchronized
Added: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/logger/ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/logger/LogHelper.java Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/LRUMapJCS.java commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractDoubleLinkedListMemoryCache.java commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractMemoryCache.java commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/LRUMap.java commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SingleLinkedList.java Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/LRUMapJCS.java URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/LRUMapJCS.java?rev=1598071&r1=1598070&r2=1598071&view=diff ============================================================================== --- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/LRUMapJCS.java (original) +++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/LRUMapJCS.java Wed May 28 17:06:12 2014 @@ -19,6 +19,7 @@ package org.apache.commons.jcs.auxiliary * under the License. */ +import org.apache.commons.jcs.utils.logger.LogHelper; import org.apache.commons.jcs.utils.struct.LRUMap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -35,6 +36,7 @@ public class LRUMapJCS<K, V> /** The logger */ private static final Log log = LogFactory.getLog( LRUMapJCS.class ); + private static final LogHelper LOG_HELPER = new LogHelper(log); /** * This creates an unbounded version. @@ -69,7 +71,7 @@ public class LRUMapJCS<K, V> @Override protected void processRemovedLRU(K key, V value) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Removing key [" + key + "] from key store, value [" + value + "]" ); log.debug( "Key store size [" + this.size() + "]" ); Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java?rev=1598071&r1=1598070&r2=1598071&view=diff ============================================================================== --- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java (original) +++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java Wed May 28 17:06:12 2014 @@ -46,6 +46,7 @@ import org.apache.commons.jcs.engine.sta import org.apache.commons.jcs.engine.stats.behavior.ICacheStats; import org.apache.commons.jcs.engine.stats.behavior.IStatElement; import org.apache.commons.jcs.engine.stats.behavior.IStats; +import org.apache.commons.jcs.utils.logger.LogHelper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -70,6 +71,7 @@ public class CompositeCache<K, V> { /** log instance */ private static final Log log = LogFactory.getLog( CompositeCache.class ); + private static final LogHelper LOG_HELPER = new LogHelper(log); /** * EventQueue for handling element events. Lazy initialized. One for each region. To be more efficient, the manager @@ -228,7 +230,7 @@ public class CompositeCache<K, V> throw new IllegalArgumentException( "key cannot be a GroupId " + " for a put operation" ); } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Updating memory cache " + cacheElement.getKey() ); } @@ -271,7 +273,7 @@ public class CompositeCache<K, V> // You could run a database cache as either a remote or a local disk. // The types would describe the purpose. - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { if ( auxCaches.length > 0 ) { @@ -287,7 +289,7 @@ public class CompositeCache<K, V> { ICache<K, V> aux = auxCaches[i]; - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Auxilliary cache type: " + aux.getCacheType() ); } @@ -300,7 +302,7 @@ public class CompositeCache<K, V> // SEND TO REMOTE STORE if ( aux.getCacheType() == CacheType.REMOTE_CACHE ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "ce.getElementAttributes().getIsRemote() = " + cacheElement.getElementAttributes().getIsRemote() ); @@ -313,7 +315,7 @@ public class CompositeCache<K, V> // need to make sure the group cache understands that // the key is a group attribute on update aux.update( cacheElement ); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Updated remote store for " + cacheElement.getKey() + cacheElement ); } @@ -329,7 +331,7 @@ public class CompositeCache<K, V> { // lateral can't do the checking since it is dependent on the // cache region restrictions - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "lateralcache in aux list: cattr " + cacheAttr.isUseLateral() ); } @@ -339,7 +341,7 @@ public class CompositeCache<K, V> // Currently always multicast even if the value is // unchanged, to cause the cache item to move to the front. aux.update( cacheElement ); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "updated lateral cache for " + cacheElement.getKey() ); } @@ -348,7 +350,7 @@ public class CompositeCache<K, V> // update disk if the usage pattern permits else if ( aux.getCacheType() == CacheType.DISK_CACHE ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "diskcache in aux list: cattr " + cacheAttr.isUseDisk() ); } @@ -357,7 +359,7 @@ public class CompositeCache<K, V> && cacheElement.getElementAttributes().getIsSpool() ) { aux.update( cacheElement ); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "updated disk cache for " + cacheElement.getKey() ); } @@ -414,14 +416,14 @@ public class CompositeCache<K, V> { // swallow } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "spoolToDisk done for: " + ce.getKey() + " on disk cache[" + i + "]" ); } } else { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "DiskCache avaialbe, but JCS is not configured to use the DiskCache as a swap." ); } @@ -483,7 +485,7 @@ public class CompositeCache<K, V> boolean found = false; - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "get: key = " + key + ", localOnly = " + localOnly ); } @@ -500,7 +502,7 @@ public class CompositeCache<K, V> // Found in memory cache if ( isExpired( element ) ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Memory cache hit, but element expired" ); } @@ -513,7 +515,7 @@ public class CompositeCache<K, V> } else { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Memory cache hit" ); } @@ -539,7 +541,7 @@ public class CompositeCache<K, V> if ( !localOnly || cacheType == CacheType.DISK_CACHE ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Attempting to get from aux [" + aux.getCacheName() + "] which is of type: " + cacheType ); @@ -555,7 +557,7 @@ public class CompositeCache<K, V> } } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Got CacheElement: " + element ); } @@ -565,7 +567,7 @@ public class CompositeCache<K, V> { if ( isExpired( element ) ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Aux cache[" + i + "] hit, but element expired." ); } @@ -582,7 +584,7 @@ public class CompositeCache<K, V> } else { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Aux cache[" + i + "] hit" ); } @@ -610,7 +612,7 @@ public class CompositeCache<K, V> { missCountNotFound++; - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Miss" ); } @@ -666,7 +668,7 @@ public class CompositeCache<K, V> { Map<K, ICacheElement<K, V>> elements = new HashMap<K, ICacheElement<K, V>>(); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "get: key = " + keys + ", localOnly = " + localOnly ); } @@ -693,7 +695,7 @@ public class CompositeCache<K, V> { missCountNotFound += keys.size() - elements.size(); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - " + ( keys.size() - elements.size() ) + " Misses" ); } @@ -725,7 +727,7 @@ public class CompositeCache<K, V> // Found in memory cache if ( isExpired( element ) ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Memory cache hit, but element expired" ); } @@ -737,7 +739,7 @@ public class CompositeCache<K, V> } else { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Memory cache hit" ); } @@ -777,7 +779,7 @@ public class CompositeCache<K, V> if ( !localOnly || cacheType == CacheType.DISK_CACHE ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Attempting to get from aux [" + aux.getCacheName() + "] which is of type: " + cacheType ); @@ -793,7 +795,7 @@ public class CompositeCache<K, V> } } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Got CacheElements: " + elementsFromAuxiliary ); } @@ -859,7 +861,7 @@ public class CompositeCache<K, V> { Map<K, ICacheElement<K, V>> elements = new HashMap<K, ICacheElement<K, V>>(); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "get: pattern [" + pattern + "], localOnly = " + localOnly ); } @@ -932,7 +934,7 @@ public class CompositeCache<K, V> if ( !localOnly || cacheType == CacheType.DISK_CACHE ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Attempting to get from aux [" + aux.getCacheName() + "] which is of type: " + cacheType ); @@ -947,7 +949,7 @@ public class CompositeCache<K, V> log.error( "Error getting from aux", e ); } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Got CacheElements: " + elementsFromAuxiliary ); } @@ -983,7 +985,7 @@ public class CompositeCache<K, V> { if ( isExpired( element ) ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Aux cache[" + i + "] hit, but element expired." ); } @@ -999,7 +1001,7 @@ public class CompositeCache<K, V> } else { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheAttr.getCacheName() + " - Aux cache[" + i + "] hit" ); } @@ -1028,7 +1030,7 @@ public class CompositeCache<K, V> } else { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Skipping memory update since no items are allowed in memory" ); } @@ -1175,7 +1177,7 @@ public class CompositeCache<K, V> } try { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Removing " + key + " from cacheType" + cacheType ); } @@ -1234,7 +1236,7 @@ public class CompositeCache<K, V> { memCache.removeAll(); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Removed All keys from the memory cache." ); } @@ -1253,7 +1255,7 @@ public class CompositeCache<K, V> { try { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Removing All keys from cacheType" + aux.getCacheType() ); } @@ -1416,7 +1418,7 @@ public class CompositeCache<K, V> } } } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Called save for [" + cacheAttr.getCacheName() + "]" ); } @@ -1621,7 +1623,7 @@ public class CompositeCache<K, V> if ( maxLifeSeconds != -1 && ( timestamp - createTime ) > ( maxLifeSeconds * timeFactorForMilliseconds) ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Exceeded maxLife: " + element.getKey() ); } @@ -1640,7 +1642,7 @@ public class CompositeCache<K, V> if ( ( idleTime != -1 ) && ( timestamp - lastAccessTime ) > idleTime * timeFactorForMilliseconds ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.info( "Exceeded maxIdle: " + element.getKey() ); } @@ -1675,7 +1677,7 @@ public class CompositeCache<K, V> ArrayList<IElementEventHandler> eventHandlers = element.getElementAttributes().getElementEventHandlers(); if ( eventHandlers != null ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Element Handlers are registered. Create event type " + eventType ); } Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractDoubleLinkedListMemoryCache.java URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractDoubleLinkedListMemoryCache.java?rev=1598071&r1=1598070&r2=1598071&view=diff ============================================================================== --- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractDoubleLinkedListMemoryCache.java (original) +++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractDoubleLinkedListMemoryCache.java Wed May 28 17:06:12 2014 @@ -28,6 +28,7 @@ import org.apache.commons.jcs.engine.sta import org.apache.commons.jcs.engine.stats.Stats; import org.apache.commons.jcs.engine.stats.behavior.IStatElement; import org.apache.commons.jcs.engine.stats.behavior.IStats; +import org.apache.commons.jcs.utils.logger.LogHelper; import org.apache.commons.jcs.utils.struct.DoubleLinkedList; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -41,6 +42,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; /** * This class contains methods that are common to memory caches using the double linked list, such @@ -58,18 +61,19 @@ public abstract class AbstractDoubleLink /** The logger. */ private static final Log log = LogFactory.getLog( AbstractDoubleLinkedListMemoryCache.class ); + private static final LogHelper LOG_HELPER = new LogHelper(log); /** thread-safe double linked list for lru */ protected DoubleLinkedList<MemoryElementDescriptor<K, V>> list; // TODO privatise /** number of hits */ - private int hitCnt = 0; + private volatile int hitCnt = 0; /** number of misses */ - private int missCnt = 0; + private volatile int missCnt = 0; /** number of puts */ - private int putCnt = 0; + private volatile int putCnt = 0; /** * For post reflection creation initialization. @@ -77,11 +81,19 @@ public abstract class AbstractDoubleLink * @param hub */ @Override - public synchronized void initialize( CompositeCache<K, V> hub ) + public void initialize( CompositeCache<K, V> hub ) { - super.initialize( hub ); - list = new DoubleLinkedList<MemoryElementDescriptor<K, V>>(); - log.info( "initialized MemoryCache for " + cacheName ); + lock.lock(); + try + { + super.initialize(hub); + list = new DoubleLinkedList<MemoryElementDescriptor<K, V>>(); + log.info("initialized MemoryCache for " + cacheName); + } + finally + { + lock.unlock(); + } } /** @@ -110,17 +122,24 @@ public abstract class AbstractDoubleLink public final void update( ICacheElement<K, V> ce ) throws IOException { - putCnt++; + lock.lock(); + try + { + putCnt++; - MemoryElementDescriptor<K, V> newNode = adjustListForUpdate( ce ); + MemoryElementDescriptor<K, V> newNode = adjustListForUpdate(ce); - // this should be synchronized if we were not using a ConcurrentHashMap - MemoryElementDescriptor<K, V> oldNode = map.put( newNode.ce.getKey(), newNode ); + // this should be synchronized if we were not using a ConcurrentHashMap + MemoryElementDescriptor<K, V> oldNode = map.put(newNode.ce.getKey(), newNode); - // If the node was the same as an existing node, remove it. - if ( oldNode != null && newNode.ce.getKey().equals( oldNode.ce.getKey() ) ) + // If the node was the same as an existing node, remove it. + if (oldNode != null && newNode.ce.getKey().equals(oldNode.ce.getKey())) { + list.remove(oldNode); + } + } + finally { - list.remove( oldNode ); + lock.unlock(); } // If we are over the max spool some @@ -153,7 +172,7 @@ public abstract class AbstractDoubleLink return; } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "In memory limit reached, spooling" ); } @@ -161,7 +180,7 @@ public abstract class AbstractDoubleLink // Write the last 'chunkSize' items to disk. int chunkSizeCorrected = Math.min( size, chunkSize ); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "About to spool to disk cache, map size: " + size + ", max objects: " + this.cacheAttributes.getMaxObjects() + ", items to spool: " + chunkSizeCorrected ); @@ -175,7 +194,7 @@ public abstract class AbstractDoubleLink spoolLastElement(); } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "update: After spool map size: " + map.size() + " linked list size = " + dumpCacheSize() ); } @@ -194,7 +213,7 @@ public abstract class AbstractDoubleLink { ICacheElement<K, V> ce = null; - final boolean debugEnabled = log.isDebugEnabled(); + final boolean debugEnabled = LOG_HELPER.isDebugEnabled();; if (debugEnabled) { @@ -205,19 +224,37 @@ public abstract class AbstractDoubleLink if ( me != null ) { - ce = me.ce; - hitCnt++; + lock.lock(); + try + { + ce = me.ce; + hitCnt++; + + // ABSTRACT + adjustListForGet( me ); + } + finally + { + lock.unlock(); + } + if (debugEnabled) { log.debug( cacheName + ": LRUMemoryCache hit for " + ce.getKey() ); } - - // ABSTRACT - adjustListForGet( me ); } else { - missCnt++; + lock.lock(); + try + { + missCnt++; + } + finally + { + lock.unlock(); + } + if (debugEnabled) { log.debug( cacheName + ": LRUMemoryCache miss for " + key ); @@ -274,22 +311,28 @@ public abstract class AbstractDoubleLink final MemoryElementDescriptor<K, V> last = list.getLast(); if ( last != null ) { - toSpool = last.ce; - if ( toSpool != null ) + lock.lock(); + try { - cache.spoolToDisk( last.ce ); - if ( map.remove( last.ce.getKey() ) == null ) + toSpool = last.ce; + if (toSpool != null) { + cache.spoolToDisk(last.ce); + if (map.remove(last.ce.getKey()) == null) { + log.warn("update: remove failed for key: " + + last.ce.getKey()); + verifyCache(); + } + } + else { - log.warn( "update: remove failed for key: " - + last.ce.getKey() ); - verifyCache(); + throw new Error("update: last.ce is null!"); } + list.remove(last); } - else + finally { - throw new Error( "update: last.ce is null!" ); + lock.unlock(); } - list.remove(last); } else { @@ -320,7 +363,7 @@ public abstract class AbstractDoubleLink public boolean remove( K key ) throws IOException { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "removing item for key: " + key ); } @@ -336,11 +379,19 @@ public abstract class AbstractDoubleLink Map.Entry<K, MemoryElementDescriptor<K, V>> entry = itr.next(); K k = entry.getKey(); - if ( k instanceof String && ( (String) k ).startsWith( key.toString() ) ) + if (k instanceof String && ((String) k).startsWith(key.toString())) { - list.remove( entry.getValue() ); - itr.remove(); - removed = true; + lock.lock(); + try + { + list.remove(entry.getValue()); + itr.remove(); + removed = true; + } + finally + { + lock.unlock(); + } } } } @@ -355,21 +406,36 @@ public abstract class AbstractDoubleLink if ( k instanceof GroupAttrName && ((GroupAttrName<?>)k).groupId.equals(((GroupAttrName<?>)key).groupId)) { - list.remove( entry.getValue() ); - itr.remove(); - removed = true; + lock.lock(); + try + { + list.remove(entry.getValue()); + itr.remove(); + removed = true; + } + finally + { + lock.unlock(); + } } } } else { // remove single item. - MemoryElementDescriptor<K, V> me = map.remove( key ); - - if ( me != null ) + lock.lock(); + try { - list.remove( me ); - removed = true; + MemoryElementDescriptor<K, V> me = map.remove(key); + if (me != null) + { + list.remove(me); + removed = true; + } + } + finally + { + lock.unlock(); } } @@ -386,8 +452,16 @@ public abstract class AbstractDoubleLink public void removeAll() throws IOException { - list.removeAll(); - map.clear(); + lock.lock(); + try + { + list.removeAll(); + map.clear(); + } + finally + { + lock.unlock(); + } } // --------------------------- internal methods (linked list implementation) @@ -399,10 +473,18 @@ public abstract class AbstractDoubleLink */ protected MemoryElementDescriptor<K, V> addFirst( ICacheElement<K, V> ce ) { - MemoryElementDescriptor<K, V> me = new MemoryElementDescriptor<K, V>( ce ); - list.addFirst( me ); - verifyCache( ce.getKey() ); - return me; + lock.lock(); + try + { + MemoryElementDescriptor<K, V> me = new MemoryElementDescriptor<K, V>(ce); + list.addFirst(me); + verifyCache(ce.getKey()); + return me; + } + finally + { + lock.unlock(); + } } /** @@ -413,10 +495,18 @@ public abstract class AbstractDoubleLink */ protected MemoryElementDescriptor<K, V> addLast( ICacheElement<K, V> ce ) { - MemoryElementDescriptor<K, V> me = new MemoryElementDescriptor<K, V>( ce ); - list.addLast( me ); - verifyCache( ce.getKey() ); - return me; + lock.lock(); + try + { + MemoryElementDescriptor<K, V> me = new MemoryElementDescriptor<K, V>(ce); + list.addLast(me); + verifyCache(ce.getKey()); + return me; + } + finally + { + lock.unlock(); + } } // ---------------------------------------------------------- debug methods @@ -451,7 +541,7 @@ public abstract class AbstractDoubleLink @SuppressWarnings("unchecked") // No generics for public fields protected void verifyCache() { - if ( !log.isDebugEnabled() ) + if ( !LOG_HELPER.isDebugEnabled() ) { return; } @@ -534,7 +624,7 @@ public abstract class AbstractDoubleLink @SuppressWarnings("unchecked") // No generics for public fields private void verifyCache( K key ) { - if ( !log.isDebugEnabled() ) + if ( !LOG_HELPER.isDebugEnabled() ) { return; } @@ -684,12 +774,7 @@ public abstract class AbstractDoubleLink @Override public Set<K> getKeySet() { - // need a better locking strategy here. - synchronized ( this ) - { - // may need to lock to map here? - return new LinkedHashSet<K>(map.keySet()); - } + return new LinkedHashSet<K>(map.keySet()); } /** @@ -699,18 +784,26 @@ public abstract class AbstractDoubleLink * @see org.apache.commons.jcs.engine.memory.behavior.IMemoryCache#getStatistics() */ @Override - public synchronized IStats getStatistics() + public IStats getStatistics() { IStats stats = new Stats(); stats.setTypeName( /*add algorithm name*/"Memory Cache" ); ArrayList<IStatElement<?>> elems = new ArrayList<IStatElement<?>>(); - elems.add(new StatElement<Integer>( "List Size", Integer.valueOf(list.size()) ) ); - elems.add(new StatElement<Integer>( "Map Size", Integer.valueOf(map.size()) ) ); - elems.add(new StatElement<Integer>( "Put Count", Integer.valueOf(putCnt) ) ); - elems.add(new StatElement<Integer>( "Hit Count", Integer.valueOf(hitCnt) ) ); - elems.add(new StatElement<Integer>( "Miss Count", Integer.valueOf(missCnt) ) ); + lock.lock(); // not sure that's really relevant here but not that important + try + { + elems.add(new StatElement<Integer>("List Size", Integer.valueOf(list.size()))); + elems.add(new StatElement<Integer>("Map Size", Integer.valueOf(map.size()))); + elems.add(new StatElement<Integer>("Put Count", Integer.valueOf(putCnt))); + elems.add(new StatElement<Integer>("Hit Count", Integer.valueOf(hitCnt))); + elems.add(new StatElement<Integer>("Miss Count", Integer.valueOf(missCnt))); + } + finally + { + lock.unlock(); + } stats.setStatElements( elems ); Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractMemoryCache.java URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractMemoryCache.java?rev=1598071&r1=1598070&r2=1598071&view=diff ============================================================================== --- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractMemoryCache.java (original) +++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/memory/AbstractMemoryCache.java Wed May 28 17:06:12 2014 @@ -28,6 +28,7 @@ import org.apache.commons.jcs.engine.mem import org.apache.commons.jcs.engine.memory.util.MemoryElementDescriptor; import org.apache.commons.jcs.engine.stats.Stats; import org.apache.commons.jcs.engine.stats.behavior.IStats; +import org.apache.commons.jcs.utils.logger.LogHelper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -35,6 +36,8 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; /** * This base includes some common code for memory caches. @@ -47,6 +50,7 @@ public abstract class AbstractMemoryCach { /** Log instance */ private static final Log log = LogFactory.getLog( AbstractMemoryCache.class ); + private static final LogHelper LOG_HELPER = new LogHelper(log); /** The region name. This defines a namespace of sorts. */ protected String cacheName; // TODO privatise (mainly seems to be used externally for debugging) @@ -69,21 +73,31 @@ public abstract class AbstractMemoryCach /** How many to spool at a time. */ protected int chunkSize; + protected final Lock lock = new ReentrantLock(); + /** * For post reflection creation initialization * <p> * @param hub */ @Override - public synchronized void initialize( CompositeCache<K, V> hub ) + public void initialize( CompositeCache<K, V> hub ) { - this.cacheName = hub.getCacheName(); - this.cacheAttributes = hub.getCacheAttributes(); - this.cache = hub; - map = createMap(); + lock.lock(); + try + { + this.cacheName = hub.getCacheName(); + this.cacheAttributes = hub.getCacheAttributes(); + this.cache = hub; + map = createMap(); - chunkSize = cacheAttributes.getSpoolChunkSize(); - status = CacheStatus.ALIVE; + chunkSize = cacheAttributes.getSpoolChunkSize(); + status = CacheStatus.ALIVE; + } + finally + { + lock.unlock(); + } } /** @@ -163,14 +177,14 @@ public abstract class AbstractMemoryCach MemoryElementDescriptor<K, V> me = map.get( key ); if ( me != null ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheName + ": MemoryCache quiet hit for " + key ); } ce = me.ce; } - else if ( log.isDebugEnabled() ) + else if ( LOG_HELPER.isDebugEnabled() ) { log.debug( cacheName + ": MemoryCache quiet miss for " + key ); } @@ -261,7 +275,7 @@ public abstract class AbstractMemoryCach { String attributeCacheName = this.cacheAttributes.getCacheName(); if(attributeCacheName != null) - return attributeCacheName; + return attributeCacheName; return cacheName; } Added: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/logger/LogHelper.java URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/logger/LogHelper.java?rev=1598071&view=auto ============================================================================== --- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/logger/LogHelper.java (added) +++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/logger/LogHelper.java Wed May 28 17:06:12 2014 @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.jcs.utils.logger; + +import org.apache.commons.logging.Log; + +// when cached isDebugEnabled will save a lot of time +public class LogHelper +{ + protected static final boolean cacheDebug = Boolean.getBoolean("jcs.logger.cacheDebug"); // TODO: move it over cache config if really used (=default not nice enough) + + private boolean debug; + private Log log; + + public LogHelper(final Log log) + { + this.debug = log.isDebugEnabled(); + this.log = log; + } + + // faster than several log calls by default + public boolean isDebugEnabled() + { + return cacheDebug ? debug : log.isDebugEnabled(); + } +} Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/LRUMap.java URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/LRUMap.java?rev=1598071&r1=1598070&r2=1598071&view=diff ============================================================================== --- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/LRUMap.java (original) +++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/LRUMap.java Wed May 28 17:06:12 2014 @@ -24,6 +24,7 @@ import org.apache.commons.jcs.engine.sta import org.apache.commons.jcs.engine.stats.Stats; import org.apache.commons.jcs.engine.stats.behavior.IStatElement; import org.apache.commons.jcs.engine.stats.behavior.IStats; +import org.apache.commons.jcs.utils.logger.LogHelper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -37,6 +38,8 @@ import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; /** * This is a simple LRUMap. It implements most of the map methods. It is not recommended that you @@ -57,6 +60,7 @@ public class LRUMap<K, V> { /** The logger */ private static final Log log = LogFactory.getLog( LRUMap.class ); + private static final LogHelper LOG_HELPER = new LogHelper(log); /** double linked list for lru */ private final DoubleLinkedList<LRUElementDescriptor<K, V>> list; @@ -79,6 +83,8 @@ public class LRUMap<K, V> /** make configurable */ private int chunkSize = 1; + private final Lock lock = new ReentrantLock(); + /** * This creates an unbounded version. Setting the max objects will result in spooling on * subsequent puts. @@ -123,8 +129,16 @@ public class LRUMap<K, V> @Override public void clear() { - map.clear(); - list.removeAll(); + lock.lock(); + try + { + map.clear(); + list.removeAll(); + } + finally + { + lock.unlock(); + } } /** @@ -200,7 +214,7 @@ public class LRUMap<K, V> { V retVal = null; - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "getting item for key " + key ); } @@ -244,14 +258,14 @@ public class LRUMap<K, V> LRUElementDescriptor<K, V> me = map.get( key ); if ( me != null ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "LRUMap quiet hit for " + key ); } ce = me.getPayload(); } - else if ( log.isDebugEnabled() ) + else if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "LRUMap quiet miss for " + key ); } @@ -266,18 +280,26 @@ public class LRUMap<K, V> @Override public V remove( Object key ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "removing item for key: " + key ); } // remove single item. - LRUElementDescriptor<K, V> me = map.remove( key ); + lock.lock(); + try + { + LRUElementDescriptor<K, V> me = map.remove(key); - if ( me != null ) + if (me != null) + { + list.remove(me); + return me.getPayload(); + } + } + finally { - list.remove( me ); - return me.getPayload(); + lock.unlock(); } return null; @@ -294,7 +316,8 @@ public class LRUMap<K, V> putCnt++; LRUElementDescriptor<K, V> old = null; - synchronized ( this ) + lock.lock(); + try { // TODO address double synchronization of addFirst, use write lock addFirst( key, value ); @@ -308,13 +331,18 @@ public class LRUMap<K, V> list.remove( old ); } } + finally + { + lock.unlock(); + } int size = map.size(); // If the element limit is reached, we need to spool if ( this.maxObjects >= 0 && size > this.maxObjects ) { - if ( log.isDebugEnabled() ) + final boolean debugEnabled = LOG_HELPER.isDebugEnabled(); + if (debugEnabled) { log.debug( "In memory limit reached, removing least recently used." ); } @@ -322,7 +350,7 @@ public class LRUMap<K, V> // Write the last 'chunkSize' items to disk. int chunkSizeCorrected = Math.min( size, getChunkSize() ); - if ( log.isDebugEnabled() ) + if (debugEnabled) { log.debug( "About to remove the least recently used. map size: " + size + ", max objects: " + this.maxObjects + ", items to spool: " + chunkSizeCorrected ); @@ -334,33 +362,41 @@ public class LRUMap<K, V> for ( int i = 0; i < chunkSizeCorrected; i++ ) { - LRUElementDescriptor<K, V> last = list.getLast(); - if ( last != null ) + lock.lock(); + try { - processRemovedLRU(last.getKey(), last.getPayload()); - if ( map.remove( last.getKey() ) == null ) + LRUElementDescriptor<K, V> last = list.getLast(); + if (last != null) + { + processRemovedLRU(last.getKey(), last.getPayload()); + if (map.remove(last.getKey()) == null) + { + log.warn("update: remove failed for key: " + + last.getKey()); + verifyCache(); + } + list.removeLast(); + } + else { - log.warn( "update: remove failed for key: " - + last.getKey() ); verifyCache(); + throw new Error("update: last is null!"); } - list.remove(last); } - else + finally { - verifyCache(); - throw new Error( "update: last is null!" ); + lock.unlock(); } } - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "update: After spool map size: " + map.size() ); } if ( map.size() != dumpCacheSize() ) { - log.error( "update: After spool, size mismatch: map.size() = " + map.size() + ", linked list size = " - + dumpCacheSize() ); + log.error("update: After spool, size mismatch: map.size() = " + map.size() + ", linked list size = " + + dumpCacheSize()); } } @@ -379,8 +415,16 @@ public class LRUMap<K, V> */ private void addFirst(K key, V val) { - LRUElementDescriptor<K, V> me = new LRUElementDescriptor<K, V>(key, val); - list.addFirst( me ); + lock.lock(); + try + { + LRUElementDescriptor<K, V> me = new LRUElementDescriptor<K, V>(key, val); + list.addFirst( me ); + } + finally + { + lock.unlock(); + } } /** @@ -402,7 +446,7 @@ public class LRUMap<K, V> log.debug( "dumpingCacheEntries" ); for ( LRUElementDescriptor<K, V> me = list.getFirst(); me != null; me = (LRUElementDescriptor<K, V>) me.next ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "dumpCacheEntries> key=" + me.getKey() + ", val=" + me.getPayload() ); } @@ -418,7 +462,7 @@ public class LRUMap<K, V> for (Map.Entry<K, LRUElementDescriptor<K, V>> e : map.entrySet()) { LRUElementDescriptor<K, V> me = e.getValue(); - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "dumpMap> key=" + e.getKey() + ", val=" + me.getPayload() ); } @@ -432,7 +476,7 @@ public class LRUMap<K, V> @SuppressWarnings("unchecked") // No generics for public fields protected void verifyCache() { - if ( !log.isDebugEnabled() ) + if ( !LOG_HELPER.isDebugEnabled() ) { return; } @@ -524,7 +568,7 @@ public class LRUMap<K, V> @SuppressWarnings("unchecked") // No generics for public fields protected void verifyCache( Object key ) { - if ( !log.isDebugEnabled() ) + if ( !LOG_HELPER.isDebugEnabled() ) { return; } @@ -556,7 +600,7 @@ public class LRUMap<K, V> */ protected void processRemovedLRU(K key, V value ) { - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "Removing key: [" + key + "] from LRUMap store, value = [" + value + "]" ); log.debug( "LRUMap store size: '" + this.size() + "'." ); @@ -615,18 +659,25 @@ public class LRUMap<K, V> @Override public Set<Map.Entry<K, V>> entrySet() { - // todo, we should return a defensive copy - Set<Map.Entry<K, LRUElementDescriptor<K, V>>> entries = map.entrySet(); + lock.lock(); + try + { + // todo, we should return a defensive copy + Set<Map.Entry<K, LRUElementDescriptor<K, V>>> entries = map.entrySet(); - Set<Map.Entry<K, V>> unWrapped = new HashSet<Map.Entry<K, V>>(); + Set<Map.Entry<K, V>> unWrapped = new HashSet<Map.Entry<K, V>>(); - for (Map.Entry<K, LRUElementDescriptor<K, V>> pre : entries ) + for (Map.Entry<K, LRUElementDescriptor<K, V>> pre : entries) { + Map.Entry<K, V> post = new LRUMapEntry<K, V>(pre.getKey(), pre.getValue().getPayload()); + unWrapped.add(post); + } + + return unWrapped; + } + finally { - Map.Entry<K, V> post = new LRUMapEntry<K, V>(pre.getKey(), pre.getValue().getPayload()); - unWrapped.add(post); + lock.unlock(); } - - return unWrapped; } /** Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SingleLinkedList.java URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SingleLinkedList.java?rev=1598071&r1=1598070&r2=1598071&view=diff ============================================================================== --- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SingleLinkedList.java (original) +++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SingleLinkedList.java Wed May 28 17:06:12 2014 @@ -19,6 +19,7 @@ package org.apache.commons.jcs.utils.str * under the License. */ +import org.apache.commons.jcs.utils.logger.LogHelper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -32,6 +33,7 @@ public class SingleLinkedList<T> { /** The logger */ private static final Log log = LogFactory.getLog( SingleLinkedList.class ); + private static final LogHelper LOG_HELPER = new LogHelper(log); /** for sync */ private final Object lock = new Object(); @@ -64,7 +66,7 @@ public class SingleLinkedList<T> T value = node.payload; - if ( log.isDebugEnabled() ) + if ( LOG_HELPER.isDebugEnabled() ) { log.debug( "head.payload = " + head.payload ); log.debug( "node.payload = " + node.payload );