You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2014/06/01 19:45:00 UTC
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
PING
On 29 May 2014 03:00, sebb <se...@gmail.com> wrote:
> On 28 May 2014 18:06, <rm...@apache.org> wrote:
>> 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
>
> -1
>
> This commit mixes two completely different changes:
> - re-entrant locks
> - addition of LogHelper
>
> I think the LogHelper class is a bad idea. It is also not thread-safe.
> If the cache is enabled, then it is not possible to change the logging
> level during a run.
>
>> 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 );
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Ok great, thank you then
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-06-24 12:15 GMT+02:00 sebb <se...@gmail.com>:
> On 24 June 2014 10:17, Romain Manni-Bucau <rm...@gmail.com> wrote:
> > Hi Sebb,
> >
> > saw you reverted few parts,
> >
> > can you give us a status and say me if I can help cleaning up what
> remains?
>
> AFAIK it has all now been reverted apart from the locking changes that
> were documented in the original log message.
>
> I don't think there is anything left to do on this.
>
> > Thanks,
> >
> >
> >
> > Romain Manni-Bucau
> > Twitter: @rmannibucau
> > Blog: http://rmannibucau.wordpress.com/
> > LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > Github: https://github.com/rmannibucau
> >
> >
> > 2014-06-19 22:35 GMT+02:00 Romain Manni-Bucau <rm...@gmail.com>:
> >
> >> Currently far from a computer (holidays) so if you cant wait go for it
> >> otherwise it is on my todo list for the beginning of next month
> >>
> >> Le jeudi 19 juin 2014, sebb <se...@gmail.com> a écrit :
> >>
> >> > On 3 June 2014 23:29, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
> >> >> Hmm, any advice to revert it without loosing next changes and passing
> >> too
> >> >> long to reapply them (some are important)?
> >> >
> >> > Please can you revert the change now?
> >> > This has been outstanding far too long.
> >> >
> >> > If you want me to do it, just let me know.
> >> >
> >> >> Actually it was not combining different things, both were related to
> >> >> scalability and performances.
> >> >
> >> > Those are both general concepts and can be applied to lots of
> different
> >> changes.
> >> >
> >> >>
> >> >>
> >> >> Romain Manni-Bucau
> >> >> Twitter: @rmannibucau
> >> >> Blog: http://rmannibucau.wordpress.com/
> >> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> >> Github: https://github.com/rmannibucau
> >> >>
> >> >>
> >> >> 2014-06-04 0:23 GMT+02:00 sebb <se...@gmail.com>:
> >> >>
> >> >>> Apart from the unresolved issue of logging, I still think the commit
> >> >>> should be reverted because it combines two completely different
> >> >>> changes.
> >> >>>
> >> >>> Please can you revert the commit ASAP?
> >> >>>
> >> >>> On 2 June 2014 18:09, sebb <se...@gmail.com> wrote:
> >> >>> > On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com>
> >> wrote:
> >> >>> >> First about immutabilit thread safety etc: we can use final if it
> >> ends
> >> >>> the
> >> >>> >> topic, it was not cause first version was a field and not a
> >> constant and
> >> >>> >> serializable but now it can be final.
> >> >>> >>
> >> >>> >> Then about isDebugEnabled: overhead is quite important compared
> to a
> >> >>> simple
> >> >>> >> boolean test. Most of the time it is not important but for a
> caching
> >> >>> >> solution (in particular in memory mode) it is impacting since it
> is
> >> done
> >> >>> >> very often.
> >> >>> >>
> >> >>> >> To be convinced of it just debug log4j (1.2) impl for instance.
> >> Really
> >> >>> >> depends the config too but basically you'll end up checking
> >> repository
> >> >>> >> level + potentially browse all logger categories. If config is
> well
> >> >>> done no
> >> >>> >> by overhead by if not that's really too much. If you take JUL
> that's
> >> >>> worse.
> >> >>> >> isDebugEnabled is fast but then log invocation has more check
> >> (record,
> >> >>> >> filter, handlers at least). Actually I think we can do further
> >> >>> proposing a
> >> >>> >> JCS property "verbose" and get rid of logger level for these
> cases.
> >> We
> >> >>> can
> >> >>> >> add a shared MBean to on/off it then.
> >> >>> >>
> >> >>> >>
> >> >>> >> wdyt?
> >> >>> >
> >> >>> > I think we need more proof that some kind of caching really is
> >> needed.
> >> >>> >
> >> >>> >>
> >> >>> >>
> >> >>> >>
> >> >>> >>
> >> >>> >>
> >> >>> >>
> >> >>> >> Romain Manni-Bucau
> >> >>> >> Twitter: @rmannibucau
> >> >>> >> Blog: http://rmannibucau.wordpress.com/
> >> >>> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> >>> >> Github: https://github.com/rmannibucau
> >> >>> >>
> >> >>> >>
> >> >>> >> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
> >> >>> >>
> >> >>> >>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
> >> >>> >>> > Am Sun, 1 Jun 2014 23:43:10 +0100
> >> >>> >>> > schrieb sebb <se...@gmail.com>:
> >> >>> >>> >
> >> >>> >>> >> On 1 June 2014 20:19, Romain Manni-Bucau <
> rmannibucau@gmail.com
> >> >
> >> >>> >>> >> wrote:
> >> >>> >>> >>> well it is for sure thread safe. Not sure I get why final
> and
> >> synch
> >> >>> >>> >>> would be mandatory in this particular case (field will
> maybe be
> >> >>> >>> >>> cached by thread but that's not an issue since the value
> will
> >> be
> >> >>> >>> >>> unique).
> >> >>> >>> >> non-final fields are not guaranteed to be published across
> >> threads
> >> >>> in
> >> >>> >>> >> the absence of sync.
> >> >>> >>> > The two fields wont change, so there is no need for publishing
> >> >>> changes.
> >> >>> >>> > So they dont need to be volatile. They could be made however
> >> final to
> >> >>> >>> > make it clearer that they will not change (but IMHO this does
> not
> >> >>> make
> >> >>> >>> > them more or less thread safe).
> >> >>> >>>
> >> >>> >>> Right, except that the logger is itself mutable and it looks
> like
> >> >>> >>> clients hold onto references to it. What I don't get is why
> it is
> >> >>> >>> so much faster to add the overhead of the helper just to avoid a
> >> >>> >>> call to logger.isDebugEnabled(). I would expect that to return
> >> just
> >> >>> >>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
> >> >>> >>> What am I missing?
> >> >>> >>>
> >> >>> >>> Phil
> >> >>> >>> >
> >> >>> >>> > I feel indifferent about beeing able to turn off trace/debug
> by
> >> >>> >>> > overwriting the underlying logger. If we are really so logger
> >> >>> >>> > agnostic it is probably a good idea. At least when
> >> commons-logging is
> >> >>> >>> > not able to abstract this shortcoming away.
> >> >>> >>> >
> >> >>> >>> > Gruss
> >> >>> >>> > Bernd
> >> >>> >>> >
> >> >>> >>> >
> >> ---------------------------------------------------------------------
> >> >>> >>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> >>> > For additional commands, e-mail: dev-help@commons.apache.org
> >> >>> >>> >
> >> >>> >>> >
> >> >>> >>>
> >> >>> >>>
> >> >>> >>>
> >> ---------------------------------------------------------------------
> >> >>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>> >>>
> >> >>> >>>
> >> >>>
> >> >>>
> ---------------------------------------------------------------------
> >> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>>
> >> >>>
> >> >
> >> > ---------------------------------------------------------------------
> >> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> > For additional commands, e-mail: dev-help@commons.apache.org
> >> >
> >> >
> >>
> >> --
> >>
> >>
> >> Romain Manni-Bucau
> >> Twitter: @rmannibucau
> >> Blog: http://rmannibucau.wordpress.com/
> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> Github: https://github.com/rmannibucau
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by sebb <se...@gmail.com>.
On 24 June 2014 10:17, Romain Manni-Bucau <rm...@gmail.com> wrote:
> Hi Sebb,
>
> saw you reverted few parts,
>
> can you give us a status and say me if I can help cleaning up what remains?
AFAIK it has all now been reverted apart from the locking changes that
were documented in the original log message.
I don't think there is anything left to do on this.
> Thanks,
>
>
>
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
> 2014-06-19 22:35 GMT+02:00 Romain Manni-Bucau <rm...@gmail.com>:
>
>> Currently far from a computer (holidays) so if you cant wait go for it
>> otherwise it is on my todo list for the beginning of next month
>>
>> Le jeudi 19 juin 2014, sebb <se...@gmail.com> a écrit :
>>
>> > On 3 June 2014 23:29, Romain Manni-Bucau <rm...@gmail.com> wrote:
>> >> Hmm, any advice to revert it without loosing next changes and passing
>> too
>> >> long to reapply them (some are important)?
>> >
>> > Please can you revert the change now?
>> > This has been outstanding far too long.
>> >
>> > If you want me to do it, just let me know.
>> >
>> >> Actually it was not combining different things, both were related to
>> >> scalability and performances.
>> >
>> > Those are both general concepts and can be applied to lots of different
>> changes.
>> >
>> >>
>> >>
>> >> Romain Manni-Bucau
>> >> Twitter: @rmannibucau
>> >> Blog: http://rmannibucau.wordpress.com/
>> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> >> Github: https://github.com/rmannibucau
>> >>
>> >>
>> >> 2014-06-04 0:23 GMT+02:00 sebb <se...@gmail.com>:
>> >>
>> >>> Apart from the unresolved issue of logging, I still think the commit
>> >>> should be reverted because it combines two completely different
>> >>> changes.
>> >>>
>> >>> Please can you revert the commit ASAP?
>> >>>
>> >>> On 2 June 2014 18:09, sebb <se...@gmail.com> wrote:
>> >>> > On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>> >>> >> First about immutabilit thread safety etc: we can use final if it
>> ends
>> >>> the
>> >>> >> topic, it was not cause first version was a field and not a
>> constant and
>> >>> >> serializable but now it can be final.
>> >>> >>
>> >>> >> Then about isDebugEnabled: overhead is quite important compared to a
>> >>> simple
>> >>> >> boolean test. Most of the time it is not important but for a caching
>> >>> >> solution (in particular in memory mode) it is impacting since it is
>> done
>> >>> >> very often.
>> >>> >>
>> >>> >> To be convinced of it just debug log4j (1.2) impl for instance.
>> Really
>> >>> >> depends the config too but basically you'll end up checking
>> repository
>> >>> >> level + potentially browse all logger categories. If config is well
>> >>> done no
>> >>> >> by overhead by if not that's really too much. If you take JUL that's
>> >>> worse.
>> >>> >> isDebugEnabled is fast but then log invocation has more check
>> (record,
>> >>> >> filter, handlers at least). Actually I think we can do further
>> >>> proposing a
>> >>> >> JCS property "verbose" and get rid of logger level for these cases.
>> We
>> >>> can
>> >>> >> add a shared MBean to on/off it then.
>> >>> >>
>> >>> >>
>> >>> >> wdyt?
>> >>> >
>> >>> > I think we need more proof that some kind of caching really is
>> needed.
>> >>> >
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >> Romain Manni-Bucau
>> >>> >> Twitter: @rmannibucau
>> >>> >> Blog: http://rmannibucau.wordpress.com/
>> >>> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> >>> >> Github: https://github.com/rmannibucau
>> >>> >>
>> >>> >>
>> >>> >> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
>> >>> >>
>> >>> >>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
>> >>> >>> > Am Sun, 1 Jun 2014 23:43:10 +0100
>> >>> >>> > schrieb sebb <se...@gmail.com>:
>> >>> >>> >
>> >>> >>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rmannibucau@gmail.com
>> >
>> >>> >>> >> wrote:
>> >>> >>> >>> well it is for sure thread safe. Not sure I get why final and
>> synch
>> >>> >>> >>> would be mandatory in this particular case (field will maybe be
>> >>> >>> >>> cached by thread but that's not an issue since the value will
>> be
>> >>> >>> >>> unique).
>> >>> >>> >> non-final fields are not guaranteed to be published across
>> threads
>> >>> in
>> >>> >>> >> the absence of sync.
>> >>> >>> > The two fields wont change, so there is no need for publishing
>> >>> changes.
>> >>> >>> > So they dont need to be volatile. They could be made however
>> final to
>> >>> >>> > make it clearer that they will not change (but IMHO this does not
>> >>> make
>> >>> >>> > them more or less thread safe).
>> >>> >>>
>> >>> >>> Right, except that the logger is itself mutable and it looks like
>> >>> >>> clients hold onto references to it. What I don't get is why it is
>> >>> >>> so much faster to add the overhead of the helper just to avoid a
>> >>> >>> call to logger.isDebugEnabled(). I would expect that to return
>> just
>> >>> >>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
>> >>> >>> What am I missing?
>> >>> >>>
>> >>> >>> Phil
>> >>> >>> >
>> >>> >>> > I feel indifferent about beeing able to turn off trace/debug by
>> >>> >>> > overwriting the underlying logger. If we are really so logger
>> >>> >>> > agnostic it is probably a good idea. At least when
>> commons-logging is
>> >>> >>> > not able to abstract this shortcoming away.
>> >>> >>> >
>> >>> >>> > Gruss
>> >>> >>> > Bernd
>> >>> >>> >
>> >>> >>> >
>> ---------------------------------------------------------------------
>> >>> >>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> >>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >>> >>> >
>> >>> >>> >
>> >>> >>>
>> >>> >>>
>> >>> >>>
>> ---------------------------------------------------------------------
>> >>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> >>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>> >>>
>> >>> >>>
>> >>>
>> >>> ---------------------------------------------------------------------
>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>
>> >>>
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>> >
>>
>> --
>>
>>
>> Romain Manni-Bucau
>> Twitter: @rmannibucau
>> Blog: http://rmannibucau.wordpress.com/
>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> Github: https://github.com/rmannibucau
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi Sebb,
saw you reverted few parts,
can you give us a status and say me if I can help cleaning up what remains?
Thanks,
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-06-19 22:35 GMT+02:00 Romain Manni-Bucau <rm...@gmail.com>:
> Currently far from a computer (holidays) so if you cant wait go for it
> otherwise it is on my todo list for the beginning of next month
>
> Le jeudi 19 juin 2014, sebb <se...@gmail.com> a écrit :
>
> > On 3 June 2014 23:29, Romain Manni-Bucau <rm...@gmail.com> wrote:
> >> Hmm, any advice to revert it without loosing next changes and passing
> too
> >> long to reapply them (some are important)?
> >
> > Please can you revert the change now?
> > This has been outstanding far too long.
> >
> > If you want me to do it, just let me know.
> >
> >> Actually it was not combining different things, both were related to
> >> scalability and performances.
> >
> > Those are both general concepts and can be applied to lots of different
> changes.
> >
> >>
> >>
> >> Romain Manni-Bucau
> >> Twitter: @rmannibucau
> >> Blog: http://rmannibucau.wordpress.com/
> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> Github: https://github.com/rmannibucau
> >>
> >>
> >> 2014-06-04 0:23 GMT+02:00 sebb <se...@gmail.com>:
> >>
> >>> Apart from the unresolved issue of logging, I still think the commit
> >>> should be reverted because it combines two completely different
> >>> changes.
> >>>
> >>> Please can you revert the commit ASAP?
> >>>
> >>> On 2 June 2014 18:09, sebb <se...@gmail.com> wrote:
> >>> > On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
> >>> >> First about immutabilit thread safety etc: we can use final if it
> ends
> >>> the
> >>> >> topic, it was not cause first version was a field and not a
> constant and
> >>> >> serializable but now it can be final.
> >>> >>
> >>> >> Then about isDebugEnabled: overhead is quite important compared to a
> >>> simple
> >>> >> boolean test. Most of the time it is not important but for a caching
> >>> >> solution (in particular in memory mode) it is impacting since it is
> done
> >>> >> very often.
> >>> >>
> >>> >> To be convinced of it just debug log4j (1.2) impl for instance.
> Really
> >>> >> depends the config too but basically you'll end up checking
> repository
> >>> >> level + potentially browse all logger categories. If config is well
> >>> done no
> >>> >> by overhead by if not that's really too much. If you take JUL that's
> >>> worse.
> >>> >> isDebugEnabled is fast but then log invocation has more check
> (record,
> >>> >> filter, handlers at least). Actually I think we can do further
> >>> proposing a
> >>> >> JCS property "verbose" and get rid of logger level for these cases.
> We
> >>> can
> >>> >> add a shared MBean to on/off it then.
> >>> >>
> >>> >>
> >>> >> wdyt?
> >>> >
> >>> > I think we need more proof that some kind of caching really is
> needed.
> >>> >
> >>> >>
> >>> >>
> >>> >>
> >>> >>
> >>> >>
> >>> >>
> >>> >> Romain Manni-Bucau
> >>> >> Twitter: @rmannibucau
> >>> >> Blog: http://rmannibucau.wordpress.com/
> >>> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >>> >> Github: https://github.com/rmannibucau
> >>> >>
> >>> >>
> >>> >> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
> >>> >>
> >>> >>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
> >>> >>> > Am Sun, 1 Jun 2014 23:43:10 +0100
> >>> >>> > schrieb sebb <se...@gmail.com>:
> >>> >>> >
> >>> >>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rmannibucau@gmail.com
> >
> >>> >>> >> wrote:
> >>> >>> >>> well it is for sure thread safe. Not sure I get why final and
> synch
> >>> >>> >>> would be mandatory in this particular case (field will maybe be
> >>> >>> >>> cached by thread but that's not an issue since the value will
> be
> >>> >>> >>> unique).
> >>> >>> >> non-final fields are not guaranteed to be published across
> threads
> >>> in
> >>> >>> >> the absence of sync.
> >>> >>> > The two fields wont change, so there is no need for publishing
> >>> changes.
> >>> >>> > So they dont need to be volatile. They could be made however
> final to
> >>> >>> > make it clearer that they will not change (but IMHO this does not
> >>> make
> >>> >>> > them more or less thread safe).
> >>> >>>
> >>> >>> Right, except that the logger is itself mutable and it looks like
> >>> >>> clients hold onto references to it. What I don't get is why it is
> >>> >>> so much faster to add the overhead of the helper just to avoid a
> >>> >>> call to logger.isDebugEnabled(). I would expect that to return
> just
> >>> >>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
> >>> >>> What am I missing?
> >>> >>>
> >>> >>> Phil
> >>> >>> >
> >>> >>> > I feel indifferent about beeing able to turn off trace/debug by
> >>> >>> > overwriting the underlying logger. If we are really so logger
> >>> >>> > agnostic it is probably a good idea. At least when
> commons-logging is
> >>> >>> > not able to abstract this shortcoming away.
> >>> >>> >
> >>> >>> > Gruss
> >>> >>> > Bernd
> >>> >>> >
> >>> >>> >
> ---------------------------------------------------------------------
> >>> >>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> >>> > For additional commands, e-mail: dev-help@commons.apache.org
> >>> >>> >
> >>> >>> >
> >>> >>>
> >>> >>>
> >>> >>>
> ---------------------------------------------------------------------
> >>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >>> >>>
> >>> >>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>
> >>>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
> --
>
>
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Currently far from a computer (holidays) so if you cant wait go for it
otherwise it is on my todo list for the beginning of next month
Le jeudi 19 juin 2014, sebb <se...@gmail.com> a écrit :
> On 3 June 2014 23:29, Romain Manni-Bucau <rm...@gmail.com> wrote:
>> Hmm, any advice to revert it without loosing next changes and passing too
>> long to reapply them (some are important)?
>
> Please can you revert the change now?
> This has been outstanding far too long.
>
> If you want me to do it, just let me know.
>
>> Actually it was not combining different things, both were related to
>> scalability and performances.
>
> Those are both general concepts and can be applied to lots of different
changes.
>
>>
>>
>> Romain Manni-Bucau
>> Twitter: @rmannibucau
>> Blog: http://rmannibucau.wordpress.com/
>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> Github: https://github.com/rmannibucau
>>
>>
>> 2014-06-04 0:23 GMT+02:00 sebb <se...@gmail.com>:
>>
>>> Apart from the unresolved issue of logging, I still think the commit
>>> should be reverted because it combines two completely different
>>> changes.
>>>
>>> Please can you revert the commit ASAP?
>>>
>>> On 2 June 2014 18:09, sebb <se...@gmail.com> wrote:
>>> > On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com>
wrote:
>>> >> First about immutabilit thread safety etc: we can use final if it
ends
>>> the
>>> >> topic, it was not cause first version was a field and not a constant
and
>>> >> serializable but now it can be final.
>>> >>
>>> >> Then about isDebugEnabled: overhead is quite important compared to a
>>> simple
>>> >> boolean test. Most of the time it is not important but for a caching
>>> >> solution (in particular in memory mode) it is impacting since it is
done
>>> >> very often.
>>> >>
>>> >> To be convinced of it just debug log4j (1.2) impl for instance.
Really
>>> >> depends the config too but basically you'll end up checking
repository
>>> >> level + potentially browse all logger categories. If config is well
>>> done no
>>> >> by overhead by if not that's really too much. If you take JUL that's
>>> worse.
>>> >> isDebugEnabled is fast but then log invocation has more check
(record,
>>> >> filter, handlers at least). Actually I think we can do further
>>> proposing a
>>> >> JCS property "verbose" and get rid of logger level for these cases.
We
>>> can
>>> >> add a shared MBean to on/off it then.
>>> >>
>>> >>
>>> >> wdyt?
>>> >
>>> > I think we need more proof that some kind of caching really is needed.
>>> >
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> Romain Manni-Bucau
>>> >> Twitter: @rmannibucau
>>> >> Blog: http://rmannibucau.wordpress.com/
>>> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>>> >> Github: https://github.com/rmannibucau
>>> >>
>>> >>
>>> >> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
>>> >>
>>> >>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
>>> >>> > Am Sun, 1 Jun 2014 23:43:10 +0100
>>> >>> > schrieb sebb <se...@gmail.com>:
>>> >>> >
>>> >>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
>>> >>> >> wrote:
>>> >>> >>> well it is for sure thread safe. Not sure I get why final and
synch
>>> >>> >>> would be mandatory in this particular case (field will maybe be
>>> >>> >>> cached by thread but that's not an issue since the value will be
>>> >>> >>> unique).
>>> >>> >> non-final fields are not guaranteed to be published across
threads
>>> in
>>> >>> >> the absence of sync.
>>> >>> > The two fields wont change, so there is no need for publishing
>>> changes.
>>> >>> > So they dont need to be volatile. They could be made however
final to
>>> >>> > make it clearer that they will not change (but IMHO this does not
>>> make
>>> >>> > them more or less thread safe).
>>> >>>
>>> >>> Right, except that the logger is itself mutable and it looks like
>>> >>> clients hold onto references to it. What I don't get is why it is
>>> >>> so much faster to add the overhead of the helper just to avoid a
>>> >>> call to logger.isDebugEnabled(). I would expect that to return just
>>> >>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
>>> >>> What am I missing?
>>> >>>
>>> >>> Phil
>>> >>> >
>>> >>> > I feel indifferent about beeing able to turn off trace/debug by
>>> >>> > overwriting the underlying logger. If we are really so logger
>>> >>> > agnostic it is probably a good idea. At least when
commons-logging is
>>> >>> > not able to abstract this shortcoming away.
>>> >>> >
>>> >>> > Gruss
>>> >>> > Bernd
>>> >>> >
>>> >>> >
---------------------------------------------------------------------
>>> >>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> >>> > For additional commands, e-mail: dev-help@commons.apache.org
>>> >>> >
>>> >>> >
>>> >>>
>>> >>>
>>> >>>
---------------------------------------------------------------------
>>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> >>> For additional commands, e-mail: dev-help@commons.apache.org
>>> >>>
>>> >>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by sebb <se...@gmail.com>.
On 3 June 2014 23:29, Romain Manni-Bucau <rm...@gmail.com> wrote:
> Hmm, any advice to revert it without loosing next changes and passing too
> long to reapply them (some are important)?
Please can you revert the change now?
This has been outstanding far too long.
If you want me to do it, just let me know.
> Actually it was not combining different things, both were related to
> scalability and performances.
Those are both general concepts and can be applied to lots of different changes.
>
>
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
> 2014-06-04 0:23 GMT+02:00 sebb <se...@gmail.com>:
>
>> Apart from the unresolved issue of logging, I still think the commit
>> should be reverted because it combines two completely different
>> changes.
>>
>> Please can you revert the commit ASAP?
>>
>> On 2 June 2014 18:09, sebb <se...@gmail.com> wrote:
>> > On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com> wrote:
>> >> First about immutabilit thread safety etc: we can use final if it ends
>> the
>> >> topic, it was not cause first version was a field and not a constant and
>> >> serializable but now it can be final.
>> >>
>> >> Then about isDebugEnabled: overhead is quite important compared to a
>> simple
>> >> boolean test. Most of the time it is not important but for a caching
>> >> solution (in particular in memory mode) it is impacting since it is done
>> >> very often.
>> >>
>> >> To be convinced of it just debug log4j (1.2) impl for instance. Really
>> >> depends the config too but basically you'll end up checking repository
>> >> level + potentially browse all logger categories. If config is well
>> done no
>> >> by overhead by if not that's really too much. If you take JUL that's
>> worse.
>> >> isDebugEnabled is fast but then log invocation has more check (record,
>> >> filter, handlers at least). Actually I think we can do further
>> proposing a
>> >> JCS property "verbose" and get rid of logger level for these cases. We
>> can
>> >> add a shared MBean to on/off it then.
>> >>
>> >>
>> >> wdyt?
>> >
>> > I think we need more proof that some kind of caching really is needed.
>> >
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >> Romain Manni-Bucau
>> >> Twitter: @rmannibucau
>> >> Blog: http://rmannibucau.wordpress.com/
>> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> >> Github: https://github.com/rmannibucau
>> >>
>> >>
>> >> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
>> >>
>> >>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
>> >>> > Am Sun, 1 Jun 2014 23:43:10 +0100
>> >>> > schrieb sebb <se...@gmail.com>:
>> >>> >
>> >>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
>> >>> >> wrote:
>> >>> >>> well it is for sure thread safe. Not sure I get why final and synch
>> >>> >>> would be mandatory in this particular case (field will maybe be
>> >>> >>> cached by thread but that's not an issue since the value will be
>> >>> >>> unique).
>> >>> >> non-final fields are not guaranteed to be published across threads
>> in
>> >>> >> the absence of sync.
>> >>> > The two fields wont change, so there is no need for publishing
>> changes.
>> >>> > So they dont need to be volatile. They could be made however final to
>> >>> > make it clearer that they will not change (but IMHO this does not
>> make
>> >>> > them more or less thread safe).
>> >>>
>> >>> Right, except that the logger is itself mutable and it looks like
>> >>> clients hold onto references to it. What I don't get is why it is
>> >>> so much faster to add the overhead of the helper just to avoid a
>> >>> call to logger.isDebugEnabled(). I would expect that to return just
>> >>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
>> >>> What am I missing?
>> >>>
>> >>> Phil
>> >>> >
>> >>> > I feel indifferent about beeing able to turn off trace/debug by
>> >>> > overwriting the underlying logger. If we are really so logger
>> >>> > agnostic it is probably a good idea. At least when commons-logging is
>> >>> > not able to abstract this shortcoming away.
>> >>> >
>> >>> > Gruss
>> >>> > Bernd
>> >>> >
>> >>> > ---------------------------------------------------------------------
>> >>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >>> >
>> >>> >
>> >>>
>> >>>
>> >>> ---------------------------------------------------------------------
>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>
>> >>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hmm, any advice to revert it without loosing next changes and passing too
long to reapply them (some are important)?
Actually it was not combining different things, both were related to
scalability and performances.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-06-04 0:23 GMT+02:00 sebb <se...@gmail.com>:
> Apart from the unresolved issue of logging, I still think the commit
> should be reverted because it combines two completely different
> changes.
>
> Please can you revert the commit ASAP?
>
> On 2 June 2014 18:09, sebb <se...@gmail.com> wrote:
> > On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com> wrote:
> >> First about immutabilit thread safety etc: we can use final if it ends
> the
> >> topic, it was not cause first version was a field and not a constant and
> >> serializable but now it can be final.
> >>
> >> Then about isDebugEnabled: overhead is quite important compared to a
> simple
> >> boolean test. Most of the time it is not important but for a caching
> >> solution (in particular in memory mode) it is impacting since it is done
> >> very often.
> >>
> >> To be convinced of it just debug log4j (1.2) impl for instance. Really
> >> depends the config too but basically you'll end up checking repository
> >> level + potentially browse all logger categories. If config is well
> done no
> >> by overhead by if not that's really too much. If you take JUL that's
> worse.
> >> isDebugEnabled is fast but then log invocation has more check (record,
> >> filter, handlers at least). Actually I think we can do further
> proposing a
> >> JCS property "verbose" and get rid of logger level for these cases. We
> can
> >> add a shared MBean to on/off it then.
> >>
> >>
> >> wdyt?
> >
> > I think we need more proof that some kind of caching really is needed.
> >
> >>
> >>
> >>
> >>
> >>
> >>
> >> Romain Manni-Bucau
> >> Twitter: @rmannibucau
> >> Blog: http://rmannibucau.wordpress.com/
> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> Github: https://github.com/rmannibucau
> >>
> >>
> >> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
> >>
> >>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
> >>> > Am Sun, 1 Jun 2014 23:43:10 +0100
> >>> > schrieb sebb <se...@gmail.com>:
> >>> >
> >>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
> >>> >> wrote:
> >>> >>> well it is for sure thread safe. Not sure I get why final and synch
> >>> >>> would be mandatory in this particular case (field will maybe be
> >>> >>> cached by thread but that's not an issue since the value will be
> >>> >>> unique).
> >>> >> non-final fields are not guaranteed to be published across threads
> in
> >>> >> the absence of sync.
> >>> > The two fields wont change, so there is no need for publishing
> changes.
> >>> > So they dont need to be volatile. They could be made however final to
> >>> > make it clearer that they will not change (but IMHO this does not
> make
> >>> > them more or less thread safe).
> >>>
> >>> Right, except that the logger is itself mutable and it looks like
> >>> clients hold onto references to it. What I don't get is why it is
> >>> so much faster to add the overhead of the helper just to avoid a
> >>> call to logger.isDebugEnabled(). I would expect that to return just
> >>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
> >>> What am I missing?
> >>>
> >>> Phil
> >>> >
> >>> > I feel indifferent about beeing able to turn off trace/debug by
> >>> > overwriting the underlying logger. If we are really so logger
> >>> > agnostic it is probably a good idea. At least when commons-logging is
> >>> > not able to abstract this shortcoming away.
> >>> >
> >>> > Gruss
> >>> > Bernd
> >>> >
> >>> > ---------------------------------------------------------------------
> >>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> > For additional commands, e-mail: dev-help@commons.apache.org
> >>> >
> >>> >
> >>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>
> >>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by sebb <se...@gmail.com>.
Apart from the unresolved issue of logging, I still think the commit
should be reverted because it combines two completely different
changes.
Please can you revert the commit ASAP?
On 2 June 2014 18:09, sebb <se...@gmail.com> wrote:
> On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com> wrote:
>> First about immutabilit thread safety etc: we can use final if it ends the
>> topic, it was not cause first version was a field and not a constant and
>> serializable but now it can be final.
>>
>> Then about isDebugEnabled: overhead is quite important compared to a simple
>> boolean test. Most of the time it is not important but for a caching
>> solution (in particular in memory mode) it is impacting since it is done
>> very often.
>>
>> To be convinced of it just debug log4j (1.2) impl for instance. Really
>> depends the config too but basically you'll end up checking repository
>> level + potentially browse all logger categories. If config is well done no
>> by overhead by if not that's really too much. If you take JUL that's worse.
>> isDebugEnabled is fast but then log invocation has more check (record,
>> filter, handlers at least). Actually I think we can do further proposing a
>> JCS property "verbose" and get rid of logger level for these cases. We can
>> add a shared MBean to on/off it then.
>>
>>
>> wdyt?
>
> I think we need more proof that some kind of caching really is needed.
>
>>
>>
>>
>>
>>
>>
>> Romain Manni-Bucau
>> Twitter: @rmannibucau
>> Blog: http://rmannibucau.wordpress.com/
>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> Github: https://github.com/rmannibucau
>>
>>
>> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
>>
>>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
>>> > Am Sun, 1 Jun 2014 23:43:10 +0100
>>> > schrieb sebb <se...@gmail.com>:
>>> >
>>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
>>> >> wrote:
>>> >>> well it is for sure thread safe. Not sure I get why final and synch
>>> >>> would be mandatory in this particular case (field will maybe be
>>> >>> cached by thread but that's not an issue since the value will be
>>> >>> unique).
>>> >> non-final fields are not guaranteed to be published across threads in
>>> >> the absence of sync.
>>> > The two fields wont change, so there is no need for publishing changes.
>>> > So they dont need to be volatile. They could be made however final to
>>> > make it clearer that they will not change (but IMHO this does not make
>>> > them more or less thread safe).
>>>
>>> Right, except that the logger is itself mutable and it looks like
>>> clients hold onto references to it. What I don't get is why it is
>>> so much faster to add the overhead of the helper just to avoid a
>>> call to logger.isDebugEnabled(). I would expect that to return just
>>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
>>> What am I missing?
>>>
>>> Phil
>>> >
>>> > I feel indifferent about beeing able to turn off trace/debug by
>>> > overwriting the underlying logger. If we are really so logger
>>> > agnostic it is probably a good idea. At least when commons-logging is
>>> > not able to abstract this shortcoming away.
>>> >
>>> > Gruss
>>> > Bernd
>>> >
>>> > ---------------------------------------------------------------------
>>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> > For additional commands, e-mail: dev-help@commons.apache.org
>>> >
>>> >
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by sebb <se...@gmail.com>.
On 2 June 2014 16:11, Romain Manni-Bucau <rm...@gmail.com> wrote:
> First about immutabilit thread safety etc: we can use final if it ends the
> topic, it was not cause first version was a field and not a constant and
> serializable but now it can be final.
>
> Then about isDebugEnabled: overhead is quite important compared to a simple
> boolean test. Most of the time it is not important but for a caching
> solution (in particular in memory mode) it is impacting since it is done
> very often.
>
> To be convinced of it just debug log4j (1.2) impl for instance. Really
> depends the config too but basically you'll end up checking repository
> level + potentially browse all logger categories. If config is well done no
> by overhead by if not that's really too much. If you take JUL that's worse.
> isDebugEnabled is fast but then log invocation has more check (record,
> filter, handlers at least). Actually I think we can do further proposing a
> JCS property "verbose" and get rid of logger level for these cases. We can
> add a shared MBean to on/off it then.
>
>
> wdyt?
I think we need more proof that some kind of caching really is needed.
>
>
>
>
>
>
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
> 2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
>
>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
>> > Am Sun, 1 Jun 2014 23:43:10 +0100
>> > schrieb sebb <se...@gmail.com>:
>> >
>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
>> >> wrote:
>> >>> well it is for sure thread safe. Not sure I get why final and synch
>> >>> would be mandatory in this particular case (field will maybe be
>> >>> cached by thread but that's not an issue since the value will be
>> >>> unique).
>> >> non-final fields are not guaranteed to be published across threads in
>> >> the absence of sync.
>> > The two fields wont change, so there is no need for publishing changes.
>> > So they dont need to be volatile. They could be made however final to
>> > make it clearer that they will not change (but IMHO this does not make
>> > them more or less thread safe).
>>
>> Right, except that the logger is itself mutable and it looks like
>> clients hold onto references to it. What I don't get is why it is
>> so much faster to add the overhead of the helper just to avoid a
>> call to logger.isDebugEnabled(). I would expect that to return just
>> as fast as the LOG_HELPER inspecting its (even cached) boolean.
>> What am I missing?
>>
>> Phil
>> >
>> > I feel indifferent about beeing able to turn off trace/debug by
>> > overwriting the underlying logger. If we are really so logger
>> > agnostic it is probably a good idea. At least when commons-logging is
>> > not able to abstract this shortcoming away.
>> >
>> > Gruss
>> > Bernd
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>> >
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Romain Manni-Bucau <rm...@gmail.com>.
First about immutabilit thread safety etc: we can use final if it ends the
topic, it was not cause first version was a field and not a constant and
serializable but now it can be final.
Then about isDebugEnabled: overhead is quite important compared to a simple
boolean test. Most of the time it is not important but for a caching
solution (in particular in memory mode) it is impacting since it is done
very often.
To be convinced of it just debug log4j (1.2) impl for instance. Really
depends the config too but basically you'll end up checking repository
level + potentially browse all logger categories. If config is well done no
by overhead by if not that's really too much. If you take JUL that's worse.
isDebugEnabled is fast but then log invocation has more check (record,
filter, handlers at least). Actually I think we can do further proposing a
JCS property "verbose" and get rid of logger level for these cases. We can
add a shared MBean to on/off it then.
wdyt?
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-06-02 16:27 GMT+02:00 Phil Steitz <ph...@gmail.com>:
> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
> > Am Sun, 1 Jun 2014 23:43:10 +0100
> > schrieb sebb <se...@gmail.com>:
> >
> >> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
> >> wrote:
> >>> well it is for sure thread safe. Not sure I get why final and synch
> >>> would be mandatory in this particular case (field will maybe be
> >>> cached by thread but that's not an issue since the value will be
> >>> unique).
> >> non-final fields are not guaranteed to be published across threads in
> >> the absence of sync.
> > The two fields wont change, so there is no need for publishing changes.
> > So they dont need to be volatile. They could be made however final to
> > make it clearer that they will not change (but IMHO this does not make
> > them more or less thread safe).
>
> Right, except that the logger is itself mutable and it looks like
> clients hold onto references to it. What I don't get is why it is
> so much faster to add the overhead of the helper just to avoid a
> call to logger.isDebugEnabled(). I would expect that to return just
> as fast as the LOG_HELPER inspecting its (even cached) boolean.
> What am I missing?
>
> Phil
> >
> > I feel indifferent about beeing able to turn off trace/debug by
> > overwriting the underlying logger. If we are really so logger
> > agnostic it is probably a good idea. At least when commons-logging is
> > not able to abstract this shortcoming away.
> >
> > Gruss
> > Bernd
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Phil Steitz <ph...@gmail.com>.
On 6/1/14, 6:01 PM, Bernd Eckenfels wrote:
> Am Sun, 1 Jun 2014 23:43:10 +0100
> schrieb sebb <se...@gmail.com>:
>
>> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>>> well it is for sure thread safe. Not sure I get why final and synch
>>> would be mandatory in this particular case (field will maybe be
>>> cached by thread but that's not an issue since the value will be
>>> unique).
>> non-final fields are not guaranteed to be published across threads in
>> the absence of sync.
> The two fields wont change, so there is no need for publishing changes.
> So they dont need to be volatile. They could be made however final to
> make it clearer that they will not change (but IMHO this does not make
> them more or less thread safe).
Right, except that the logger is itself mutable and it looks like
clients hold onto references to it. What I don't get is why it is
so much faster to add the overhead of the helper just to avoid a
call to logger.isDebugEnabled(). I would expect that to return just
as fast as the LOG_HELPER inspecting its (even cached) boolean.
What am I missing?
Phil
>
> I feel indifferent about beeing able to turn off trace/debug by
> overwriting the underlying logger. If we are really so logger
> agnostic it is probably a good idea. At least when commons-logging is
> not able to abstract this shortcoming away.
>
> Gruss
> Bernd
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by sebb <se...@gmail.com>.
On 2 June 2014 02:01, Bernd Eckenfels <ec...@zusammenkunft.net> wrote:
> Am Sun, 1 Jun 2014 23:43:10 +0100
> schrieb sebb <se...@gmail.com>:
>
>> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>> > well it is for sure thread safe. Not sure I get why final and synch
>> > would be mandatory in this particular case (field will maybe be
>> > cached by thread but that's not an issue since the value will be
>> > unique).
>>
>> non-final fields are not guaranteed to be published across threads in
>> the absence of sync.
>
> The two fields wont change, so there is no need for publishing changes.
But the initial value still has to be published correctly.
> So they dont need to be volatile. They could be made however final to
> make it clearer that they will not change (but IMHO this does not make
> them more or less thread safe).
Final fields are guaranteed to be published safely.
> I feel indifferent about beeing able to turn off trace/debug by
> overwriting the underlying logger. If we are really so logger
> agnostic it is probably a good idea. At least when commons-logging is
> not able to abstract this shortcoming away.
I was not proposing changing the underlying logger.
But it can sometimes be useful to change the log level during a test run.
The caching prevents this.
> Gruss
> Bernd
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in
/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Bernd Eckenfels <ec...@zusammenkunft.net>.
Am Sun, 1 Jun 2014 23:43:10 +0100
schrieb sebb <se...@gmail.com>:
> On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
> > well it is for sure thread safe. Not sure I get why final and synch
> > would be mandatory in this particular case (field will maybe be
> > cached by thread but that's not an issue since the value will be
> > unique).
>
> non-final fields are not guaranteed to be published across threads in
> the absence of sync.
The two fields wont change, so there is no need for publishing changes.
So they dont need to be volatile. They could be made however final to
make it clearer that they will not change (but IMHO this does not make
them more or less thread safe).
I feel indifferent about beeing able to turn off trace/debug by
overwriting the underlying logger. If we are really so logger
agnostic it is probably a good idea. At least when commons-logging is
not able to abstract this shortcoming away.
Gruss
Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by sebb <se...@gmail.com>.
On 1 June 2014 20:19, Romain Manni-Bucau <rm...@gmail.com> wrote:
> well it is for sure thread safe. Not sure I get why final and synch would be
> mandatory in this particular case (field will maybe be cached by thread but
> that's not an issue since the value will be unique).
non-final fields are not guaranteed to be published across threads in
the absence of sync.
The LogHelper class has two such fields and is therefore not thread-safe.
> I have nothing against a revert/reapply. I'll open a thread on logging btw.
Thanks.
>
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
> 2014-06-01 20:55 GMT+02:00 sebb <se...@gmail.com>:
>
>> On 1 June 2014 18:54, Romain Manni-Bucau <rm...@gmail.com> wrote:
>> >
>> > 2014-06-01 19:45 GMT+02:00 sebb <se...@gmail.com>:
>> >>
>> >> PING
>> >>
>> >
>> > Pong, sorry, missed this one.
>> >
>> >>
>> >> On 29 May 2014 03:00, sebb <se...@gmail.com> wrote:
>> >> > On 28 May 2014 18:06, <rm...@apache.org> wrote:
>> >> >> 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
>> >> >
>> >> > -1
>> >> >
>> >> > This commit mixes two completely different changes:
>> >> > - re-entrant locks
>> >> > - addition of LogHelper
>> >> >
>> >> > I think the LogHelper class is a bad idea. It is also not
>> >> > thread-safe.
>> >> > If the cache is enabled, then it is not possible to change the
>> >> > logging
>> >> > level during a run.
>> >> >
>> >
>> >
>> > How can it be not thread safe? (it is a constant)
>>
>> The class is not thread-safe as it has a non-final non-synch variable
>>
>> > You can disable caching. The main point is jcs tests a lot log debug
>> > level
>> > and depending your backing impl it can be slow (JUL, Log4j 1.2 for what
>> > I
>> > tested). There is a property if you want to disable this feature but
>> > actually in prod you rarely do it.
>> >
>>
>> I have two objections to the commit.
>>
>> 1) the commit mixes two completely different changes
>>
>> 2) the LogHelper class is a bad idea, as it prevents changing the logging
>> level.
>>
>> I suggest reverting the commit and re-applying the locking change on its
>> own.
>> The logging change really needs to be discussed first.
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Romain Manni-Bucau <rm...@gmail.com>.
well it is for sure thread safe. Not sure I get why final and synch would
be mandatory in this particular case (field will maybe be cached by thread
but that's not an issue since the value will be unique).
I have nothing against a revert/reapply. I'll open a thread on logging btw.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-06-01 20:55 GMT+02:00 sebb <se...@gmail.com>:
> On 1 June 2014 18:54, Romain Manni-Bucau <rm...@gmail.com> wrote:
> >
> > 2014-06-01 19:45 GMT+02:00 sebb <se...@gmail.com>:
> >>
> >> PING
> >>
> >
> > Pong, sorry, missed this one.
> >
> >>
> >> On 29 May 2014 03:00, sebb <se...@gmail.com> wrote:
> >> > On 28 May 2014 18:06, <rm...@apache.org> wrote:
> >> >> 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
> >> >
> >> > -1
> >> >
> >> > This commit mixes two completely different changes:
> >> > - re-entrant locks
> >> > - addition of LogHelper
> >> >
> >> > I think the LogHelper class is a bad idea. It is also not thread-safe.
> >> > If the cache is enabled, then it is not possible to change the logging
> >> > level during a run.
> >> >
> >
> >
> > How can it be not thread safe? (it is a constant)
>
> The class is not thread-safe as it has a non-final non-synch variable
>
> > You can disable caching. The main point is jcs tests a lot log debug
> level
> > and depending your backing impl it can be slow (JUL, Log4j 1.2 for what I
> > tested). There is a property if you want to disable this feature but
> > actually in prod you rarely do it.
> >
>
> I have two objections to the commit.
>
> 1) the commit mixes two completely different changes
>
> 2) the LogHelper class is a bad idea, as it prevents changing the logging
> level.
>
> I suggest reverting the commit and re-applying the locking change on its
> own.
> The logging change really needs to be discussed first.
>
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by sebb <se...@gmail.com>.
On 1 June 2014 18:54, Romain Manni-Bucau <rm...@gmail.com> wrote:
>
> 2014-06-01 19:45 GMT+02:00 sebb <se...@gmail.com>:
>>
>> PING
>>
>
> Pong, sorry, missed this one.
>
>>
>> On 29 May 2014 03:00, sebb <se...@gmail.com> wrote:
>> > On 28 May 2014 18:06, <rm...@apache.org> wrote:
>> >> 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
>> >
>> > -1
>> >
>> > This commit mixes two completely different changes:
>> > - re-entrant locks
>> > - addition of LogHelper
>> >
>> > I think the LogHelper class is a bad idea. It is also not thread-safe.
>> > If the cache is enabled, then it is not possible to change the logging
>> > level during a run.
>> >
>
>
> How can it be not thread safe? (it is a constant)
The class is not thread-safe as it has a non-final non-synch variable
> You can disable caching. The main point is jcs tests a lot log debug level
> and depending your backing impl it can be slow (JUL, Log4j 1.2 for what I
> tested). There is a property if you want to disable this feature but
> actually in prod you rarely do it.
>
I have two objections to the commit.
1) the commit mixes two completely different changes
2) the LogHelper class is a bad idea, as it prevents changing the logging level.
I suggest reverting the commit and re-applying the locking change on its own.
The logging change really needs to be discussed first.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1598071 - in /commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs:
auxiliary/disk/ engine/control/ engine/memory/ utils/logger/ utils/struct/
Posted by Romain Manni-Bucau <rm...@gmail.com>.
2014-06-01 19:45 GMT+02:00 sebb <se...@gmail.com>:
> PING
>
>
Pong, sorry, missed this one.
> On 29 May 2014 03:00, sebb <se...@gmail.com> wrote:
> > On 28 May 2014 18:06, <rm...@apache.org> wrote:
> >> 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
> >
> > -1
> >
> > This commit mixes two completely different changes:
> > - re-entrant locks
> > - addition of LogHelper
> >
> > I think the LogHelper class is a bad idea. It is also not thread-safe.
> > If the cache is enabled, then it is not possible to change the logging
> > level during a run.
> >
>
How can it be not thread safe? (it is a constant)
You can disable caching. The main point is jcs tests a lot log debug level
and depending your backing impl it can be slow (JUL, Log4j 1.2 for what I
tested). There is a property if you want to disable this feature but
actually in prod you rarely do it.
> >> 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 );
> >>
> >>
>