commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject 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/
Date Fri, 30 May 2014 23:19:04 GMT
PING

On 29 May 2014 03:00, sebb <sebbaz@gmail.com> wrote:
> On 28 May 2014 18:06,  <rmannibucau@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


Mime
View raw message