You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by tv...@apache.org on 2019/05/28 13:48:54 UTC
[commons-jcs] 06/09: Remove synchronized sections
This is an automated email from the ASF dual-hosted git repository.
tv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jcs.git
commit 6b6cf13d8f0709ea8b6029416805d68df594bbd2
Author: Thomas Vandahl <tv...@apache.org>
AuthorDate: Tue May 28 15:43:20 2019 +0200
Remove synchronized sections
---
.../commons/jcs/engine/control/CompositeCache.java | 472 ++++++++++-----------
1 file changed, 227 insertions(+), 245 deletions(-)
diff --git a/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java b/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java
index cf49552..8f2efda 100644
--- a/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java
+++ b/commons-jcs-core/src/main/java/org/apache/commons/jcs/engine/control/CompositeCache.java
@@ -29,7 +29,7 @@ import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -95,22 +95,22 @@ public class CompositeCache<K, V>
private ICompositeCacheAttributes cacheAttr;
/** How many times update was called. */
- private AtomicInteger updateCount;
+ private AtomicLong updateCount;
/** How many times remove was called. */
- private AtomicInteger removeCount;
+ private AtomicLong removeCount;
/** Memory cache hit count */
- private AtomicInteger hitCountRam;
+ private AtomicLong hitCountRam;
/** Auxiliary cache hit count (number of times found in ANY auxiliary) */
- private AtomicInteger hitCountAux;
+ private AtomicLong hitCountAux;
/** Count of misses where element was not found. */
- private AtomicInteger missCountNotFound;
+ private AtomicLong missCountNotFound;
/** Count of misses where element was expired. */
- private AtomicInteger missCountExpired;
+ private AtomicLong missCountExpired;
/** Cache manager. */
private CompositeCacheManager cacheManager = null;
@@ -137,12 +137,12 @@ public class CompositeCache<K, V>
this.attr = attr;
this.cacheAttr = cattr;
this.alive = new AtomicBoolean(true);
- this.updateCount = new AtomicInteger(0);
- this.removeCount = new AtomicInteger(0);
- this.hitCountRam = new AtomicInteger(0);
- this.hitCountAux = new AtomicInteger(0);
- this.missCountNotFound = new AtomicInteger(0);
- this.missCountExpired = new AtomicInteger(0);
+ this.updateCount = new AtomicLong(0);
+ this.removeCount = new AtomicLong(0);
+ this.hitCountRam = new AtomicLong(0);
+ this.hitCountAux = new AtomicLong(0);
+ this.missCountNotFound = new AtomicLong(0);
+ this.missCountExpired = new AtomicLong(0);
createMemoryCache(cattr);
@@ -260,12 +260,8 @@ public class CompositeCache<K, V>
}
updateCount.incrementAndGet();
-
- synchronized (this)
- {
- memCache.update(cacheElement);
- updateAuxiliaries(cacheElement, localOnly);
- }
+ memCache.update(cacheElement);
+ updateAuxiliaries(cacheElement, localOnly);
cacheElement.getElementAttributes().setLastAccessTimeNow();
}
@@ -510,113 +506,110 @@ public class CompositeCache<K, V>
log.debug("get: key = " + key + ", localOnly = " + localOnly);
}
- synchronized (this)
+ try
{
- try
- {
- // First look in memory cache
- element = memCache.get(key);
+ // First look in memory cache
+ element = memCache.get(key);
- if (element != null)
+ if (element != null)
+ {
+ // Found in memory cache
+ if (isExpired(element))
{
- // Found in memory cache
- if (isExpired(element))
+ if (log.isDebugEnabled())
{
- if (log.isDebugEnabled())
- {
- log.debug(cacheAttr.getCacheName() + " - Memory cache hit, but element expired");
- }
-
- doExpires(element);
- element = null;
+ log.debug(cacheAttr.getCacheName() + " - Memory cache hit, but element expired");
}
- else
- {
- if (log.isDebugEnabled())
- {
- log.debug(cacheAttr.getCacheName() + " - Memory cache hit");
- }
- // Update counters
- hitCountRam.incrementAndGet();
+ doExpires(element);
+ element = null;
+ }
+ else
+ {
+ if (log.isDebugEnabled())
+ {
+ log.debug(cacheAttr.getCacheName() + " - Memory cache hit");
}
- found = true;
+ // Update counters
+ hitCountRam.incrementAndGet();
}
- else
+
+ found = true;
+ }
+ else
+ {
+ // Item not found in memory. If local invocation look in aux
+ // caches, even if not local look in disk auxiliaries
+ for (AuxiliaryCache<K, V> aux : auxCaches)
{
- // Item not found in memory. If local invocation look in aux
- // caches, even if not local look in disk auxiliaries
- for (AuxiliaryCache<K, V> aux : auxCaches)
+ if (aux != null)
{
- if (aux != null)
- {
- CacheType cacheType = aux.getCacheType();
+ CacheType cacheType = aux.getCacheType();
- if (!localOnly || cacheType == CacheType.DISK_CACHE)
+ if (!localOnly || cacheType == CacheType.DISK_CACHE)
+ {
+ if (log.isDebugEnabled())
{
- if (log.isDebugEnabled())
- {
- log.debug("Attempting to get from aux [" + aux.getCacheName() + "] which is of type: "
- + cacheType);
- }
-
- try
- {
- element = aux.get(key);
- }
- catch (IOException e)
- {
- log.error("Error getting from aux", e);
- }
+ log.debug("Attempting to get from aux [" + aux.getCacheName() + "] which is of type: "
+ + cacheType);
}
- if (log.isDebugEnabled())
+ try
+ {
+ element = aux.get(key);
+ }
+ catch (IOException e)
{
- log.debug("Got CacheElement: " + element);
+ log.error("Error getting from aux", e);
}
+ }
+
+ if (log.isDebugEnabled())
+ {
+ log.debug("Got CacheElement: " + element);
+ }
- // Item found in one of the auxiliary caches.
- if (element != null)
+ // Item found in one of the auxiliary caches.
+ if (element != null)
+ {
+ if (isExpired(element))
{
- if (isExpired(element))
+ if (log.isDebugEnabled())
{
- if (log.isDebugEnabled())
- {
- log.debug(cacheAttr.getCacheName() + " - Aux cache[" + aux.getCacheName() + "] hit, but element expired.");
- }
-
- // This will tell the remotes to remove the item
- // based on the element's expiration policy. The elements attributes
- // associated with the item when it created govern its behavior
- // everywhere.
- doExpires(element);
- element = null;
+ log.debug(cacheAttr.getCacheName() + " - Aux cache[" + aux.getCacheName() + "] hit, but element expired.");
}
- else
+
+ // This will tell the remotes to remove the item
+ // based on the element's expiration policy. The elements attributes
+ // associated with the item when it created govern its behavior
+ // everywhere.
+ doExpires(element);
+ element = null;
+ }
+ else
+ {
+ if (log.isDebugEnabled())
{
- if (log.isDebugEnabled())
- {
- log.debug(cacheAttr.getCacheName() + " - Aux cache[" + aux.getCacheName() + "] hit");
- }
-
- // Update counters
- hitCountAux.incrementAndGet();
- copyAuxiliaryRetrievedItemToMemory(element);
+ log.debug(cacheAttr.getCacheName() + " - Aux cache[" + aux.getCacheName() + "] hit");
}
- found = true;
-
- break;
+ // Update counters
+ hitCountAux.incrementAndGet();
+ copyAuxiliaryRetrievedItemToMemory(element);
}
+
+ found = true;
+
+ break;
}
}
}
}
- catch (IOException e)
- {
- log.error("Problem encountered getting element.", e);
- }
+ }
+ catch (IOException e)
+ {
+ log.error("Problem encountered getting element.", e);
}
if (!found)
@@ -1150,52 +1143,49 @@ public class CompositeCache<K, V>
boolean removed = false;
- synchronized (this)
+ try
{
- try
+ removed = memCache.remove(key);
+ }
+ catch (IOException e)
+ {
+ log.error(e);
+ }
+
+ // Removes from all auxiliary caches.
+ for (ICache<K, V> aux : auxCaches)
+ {
+ if (aux == null)
{
- removed = memCache.remove(key);
+ continue;
}
- catch (IOException e)
+
+ CacheType cacheType = aux.getCacheType();
+
+ // for now let laterals call remote remove but not vice versa
+ if (localOnly && (cacheType == CacheType.REMOTE_CACHE || cacheType == CacheType.LATERAL_CACHE))
{
- log.error(e);
+ continue;
}
-
- // Removes from all auxiliary caches.
- for (ICache<K, V> aux : auxCaches)
+ try
{
- if (aux == null)
+ if (log.isDebugEnabled())
{
- continue;
+ log.debug("Removing " + key + " from cacheType" + cacheType);
}
- CacheType cacheType = aux.getCacheType();
+ boolean b = aux.remove(key);
- // for now let laterals call remote remove but not vice versa
- if (localOnly && (cacheType == CacheType.REMOTE_CACHE || cacheType == CacheType.LATERAL_CACHE))
- {
- continue;
- }
- try
+ // Don't take the remote removal into account.
+ if (!removed && cacheType != CacheType.REMOTE_CACHE)
{
- if (log.isDebugEnabled())
- {
- log.debug("Removing " + key + " from cacheType" + cacheType);
- }
-
- boolean b = aux.remove(key);
-
- // Don't take the remote removal into account.
- if (!removed && cacheType != CacheType.REMOTE_CACHE)
- {
- removed = b;
- }
- }
- catch (IOException ex)
- {
- log.error("Failure removing from aux", ex);
+ removed = b;
}
}
+ catch (IOException ex)
+ {
+ log.error("Failure removing from aux", ex);
+ }
}
return removed;
@@ -1235,40 +1225,37 @@ public class CompositeCache<K, V>
protected void removeAll(boolean localOnly)
throws IOException
{
- synchronized (this)
+ try
{
- try
- {
- memCache.removeAll();
+ memCache.removeAll();
- if (log.isDebugEnabled())
- {
- log.debug("Removed All keys from the memory cache.");
- }
- }
- catch (IOException ex)
+ if (log.isDebugEnabled())
{
- log.error("Trouble updating memory cache.", ex);
+ log.debug("Removed All keys from the memory cache.");
}
+ }
+ catch (IOException ex)
+ {
+ log.error("Trouble updating memory cache.", ex);
+ }
- // Removes from all auxiliary disk caches.
- for (ICache<K, V> aux : auxCaches)
+ // Removes from all auxiliary disk caches.
+ for (ICache<K, V> aux : auxCaches)
+ {
+ if (aux != null && (aux.getCacheType() == CacheType.DISK_CACHE || !localOnly))
{
- if (aux != null && (aux.getCacheType() == CacheType.DISK_CACHE || !localOnly))
+ try
{
- try
- {
- if (log.isDebugEnabled())
- {
- log.debug("Removing All keys from cacheType" + aux.getCacheType());
- }
-
- aux.removeAll();
- }
- catch (IOException ex)
+ if (log.isDebugEnabled())
{
- log.error("Failure removing all from aux", ex);
+ log.debug("Removing All keys from cacheType" + aux.getCacheType());
}
+
+ aux.removeAll();
+ }
+ catch (IOException ex)
+ {
+ log.error("Failure removing all from aux", ex);
}
}
}
@@ -1303,93 +1290,90 @@ public class CompositeCache<K, V>
log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] fromRemote [" + fromRemote + "]");
}
- synchronized (this)
+ // Remove us from the cache managers list
+ // This will call us back but exit immediately
+ if (cacheManager != null)
{
- // Remove us from the cache managers list
- // This will call us back but exit immediately
- if (cacheManager != null)
- {
- cacheManager.freeCache(getCacheName(), fromRemote);
- }
+ cacheManager.freeCache(getCacheName(), fromRemote);
+ }
- // Try to stop shrinker thread
- if (future != null)
- {
- future.cancel(true);
- }
+ // Try to stop shrinker thread
+ if (future != null)
+ {
+ future.cancel(true);
+ }
- // Now, shut down the event queue
- if (elementEventQ != null)
- {
- elementEventQ.dispose();
- elementEventQ = null;
- }
+ // Now, shut down the event queue
+ if (elementEventQ != null)
+ {
+ elementEventQ.dispose();
+ elementEventQ = null;
+ }
- // Dispose of each auxiliary cache, Remote auxiliaries will be
- // skipped if 'fromRemote' is true.
- for (ICache<K, V> aux : auxCaches)
+ // Dispose of each auxiliary cache, Remote auxiliaries will be
+ // skipped if 'fromRemote' is true.
+ for (ICache<K, V> aux : auxCaches)
+ {
+ try
{
- try
+ // Skip this auxiliary if:
+ // - The auxiliary is null
+ // - The auxiliary is not alive
+ // - The auxiliary is remote and the invocation was remote
+ if (aux == null || aux.getStatus() != CacheStatus.ALIVE
+ || (fromRemote && aux.getCacheType() == CacheType.REMOTE_CACHE))
{
- // Skip this auxiliary if:
- // - The auxiliary is null
- // - The auxiliary is not alive
- // - The auxiliary is remote and the invocation was remote
- if (aux == null || aux.getStatus() != CacheStatus.ALIVE
- || (fromRemote && aux.getCacheType() == CacheType.REMOTE_CACHE))
- {
- if (log.isInfoEnabled())
- {
- log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] SKIPPING auxiliary [" + aux.getCacheName() + "] fromRemote ["
- + fromRemote + "]");
- }
- continue;
- }
-
if (log.isInfoEnabled())
{
- log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] auxiliary [" + aux.getCacheName() + "]");
- }
-
- // IT USED TO BE THE CASE THAT (If the auxiliary is not a lateral, or the cache
- // attributes
- // have 'getUseLateral' set, all the elements currently in
- // memory are written to the lateral before disposing)
- // I changed this. It was excessive. Only the disk cache needs the items, since only
- // the disk cache is in a situation to not get items on a put.
- if (aux.getCacheType() == CacheType.DISK_CACHE)
- {
- int numToFree = memCache.getSize();
- memCache.freeElements(numToFree);
-
- if (log.isInfoEnabled())
- {
- log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] put " + numToFree + " into auxiliary " + aux.getCacheName());
- }
+ log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] SKIPPING auxiliary [" + aux.getCacheName() + "] fromRemote ["
+ + fromRemote + "]");
}
+ continue;
+ }
- // Dispose of the auxiliary
- aux.dispose();
+ if (log.isInfoEnabled())
+ {
+ log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] auxiliary [" + aux.getCacheName() + "]");
}
- catch (IOException ex)
+
+ // IT USED TO BE THE CASE THAT (If the auxiliary is not a lateral, or the cache
+ // attributes
+ // have 'getUseLateral' set, all the elements currently in
+ // memory are written to the lateral before disposing)
+ // I changed this. It was excessive. Only the disk cache needs the items, since only
+ // the disk cache is in a situation to not get items on a put.
+ if (aux.getCacheType() == CacheType.DISK_CACHE)
{
- log.error("Failure disposing of aux.", ex);
+ int numToFree = memCache.getSize();
+ memCache.freeElements(numToFree);
+
+ if (log.isInfoEnabled())
+ {
+ log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] put " + numToFree + " into auxiliary " + aux.getCacheName());
+ }
}
- }
- if (log.isInfoEnabled())
- {
- log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] disposing of memory cache.");
- }
- try
- {
- memCache.dispose();
+ // Dispose of the auxiliary
+ aux.dispose();
}
catch (IOException ex)
{
- log.error("Failure disposing of memCache", ex);
+ log.error("Failure disposing of aux.", ex);
}
}
+
+ if (log.isInfoEnabled())
+ {
+ log.info("In DISPOSE, [" + this.cacheAttr.getCacheName() + "] disposing of memory cache.");
+ }
+ try
+ {
+ memCache.dispose();
+ }
+ catch (IOException ex)
+ {
+ log.error("Failure disposing of memCache", ex);
+ }
}
/**
@@ -1404,31 +1388,29 @@ public class CompositeCache<K, V>
return;
}
- synchronized (this)
+ for (ICache<K, V> aux : auxCaches)
{
- for (ICache<K, V> aux : auxCaches)
+ try
{
- try
+ if (aux.getStatus() == CacheStatus.ALIVE)
{
- if (aux.getStatus() == CacheStatus.ALIVE)
+ for (K key : memCache.getKeySet())
{
- for (K key : memCache.getKeySet())
- {
- ICacheElement<K, V> ce = memCache.get(key);
+ ICacheElement<K, V> ce = memCache.get(key);
- if (ce != null)
- {
- aux.update(ce);
- }
+ if (ce != null)
+ {
+ aux.update(ce);
}
}
}
- catch (IOException ex)
- {
- log.error("Failure saving aux caches.", ex);
- }
+ }
+ catch (IOException ex)
+ {
+ log.error("Failure saving aux caches.", ex);
}
}
+
if (log.isDebugEnabled())
{
log.debug("Called save for [" + cacheAttr.getCacheName() + "]");
@@ -1493,8 +1475,8 @@ public class CompositeCache<K, V>
// store the composite cache stats first
ArrayList<IStatElement<?>> elems = new ArrayList<IStatElement<?>>();
- elems.add(new StatElement<Integer>("HitCountRam", Integer.valueOf(getHitCountRam())));
- elems.add(new StatElement<Integer>("HitCountAux", Integer.valueOf(getHitCountAux())));
+ elems.add(new StatElement<Long>("HitCountRam", Long.valueOf(getHitCountRam())));
+ elems.add(new StatElement<Long>("HitCountAux", Long.valueOf(getHitCountAux())));
stats.setStatElements(elems);
@@ -1755,7 +1737,7 @@ public class CompositeCache<K, V>
* <p>
* @return number of hits in memory
*/
- public int getHitCountRam()
+ public long getHitCountRam()
{
return hitCountRam.get();
}
@@ -1764,7 +1746,7 @@ public class CompositeCache<K, V>
* Number of times a requested item was found in and auxiliary cache.
* @return number of auxiliary hits.
*/
- public int getHitCountAux()
+ public long getHitCountAux()
{
return hitCountAux.get();
}
@@ -1773,7 +1755,7 @@ public class CompositeCache<K, V>
* Number of times a requested element was not found.
* @return number of misses.
*/
- public int getMissCountNotFound()
+ public long getMissCountNotFound()
{
return missCountNotFound.get();
}
@@ -1782,7 +1764,7 @@ public class CompositeCache<K, V>
* Number of times a requested element was found but was expired.
* @return number of found but expired gets.
*/
- public int getMissCountExpired()
+ public long getMissCountExpired()
{
return missCountExpired.get();
}
@@ -1790,7 +1772,7 @@ public class CompositeCache<K, V>
/**
* @return Returns the updateCount.
*/
- public int getUpdateCount()
+ public long getUpdateCount()
{
return updateCount.get();
}