This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-jcs.git
commit 7a7dfd1a559c5e7c2e2b9043ad036f8cffbbacfe Author: Thomas Vandahl <[email protected]> AuthorDate: Mon Sep 14 16:34:14 2026 +0200 Remove locking from map operations --- .../jcs4/engine/memory/AbstractMemoryCache.java | 261 +++++++-------------- .../jcs4/engine/memory/lru/LHMLRUMemoryCache.java | 168 ++++++++++--- .../memory/soft/SoftReferenceMemoryCache.java | 68 +++--- 3 files changed, 251 insertions(+), 246 deletions(-) diff --git a/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/AbstractMemoryCache.java b/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/AbstractMemoryCache.java index be04c76d..2090917f 100644 --- a/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/AbstractMemoryCache.java +++ b/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/AbstractMemoryCache.java @@ -27,8 +27,6 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -65,10 +63,7 @@ public abstract class AbstractMemoryCache<K, V> /** The cache region name this store is associated with */ private String cacheName; - /** The lock */ - protected final ReadWriteLock lock = new ReentrantReadWriteLock(); - - /** Map where items are stored by key. This is created by the concrete child class. */ + /** Sharded map where items are stored by key. This is created by the concrete child class. */ private ConcurrentMap<K, MemoryElementDescriptor<K, V>> map; /** Number of hits */ @@ -97,33 +92,6 @@ public abstract class AbstractMemoryCache<K, V> return Collections.unmodifiableMap(map); } - /** - * This instructs the memory cache to remove the <em>numberToFree</em> according to its eviction - * policy. For example, the LRUMemoryCache will remove the <em>numberToFree</em> least recently - * used items. These will be spooled to disk if a disk auxiliary is available. - * - * @param numberToFree - * @return The number that were removed. if you ask to free 5, but there are only 3, you will - * get 3. - */ - @Override - public int freeElements(final int numberToFree) throws IOException - { - int freed = 0; - - lock.writeLock().lock(); - try - { - freed = lockedFreeElements(numberToFree); - } - finally - { - lock.writeLock().unlock(); - } - - return freed; - } - /** * Prepares for shutdown. Reset statistics * @@ -139,6 +107,7 @@ public abstract class AbstractMemoryCache<K, V> putCnt.set(0); log.info( "Memory Cache dispose called." ); } + /** * Dump the cache map for debugging. */ @@ -161,24 +130,9 @@ public abstract class AbstractMemoryCache<K, V> @Override public ICacheElement<K, V> get(final K key) { - MemoryElementDescriptor<K, V> me = null; - log.debug("{0}: getting item for key {1}", this::getCacheName, () -> key); - lock.writeLock().lock(); - try - { - me = map.get(key); - - if (me != null) - { - lockedGetElement(me); - } - } - finally - { - lock.writeLock().unlock(); - } + MemoryElementDescriptor<K, V> me = map.get(key); if (me == null) { @@ -188,6 +142,7 @@ public abstract class AbstractMemoryCache<K, V> } else { + adjustGetElement(me); hitCnt.incrementAndGet(); log.debug("{0}: MemoryCache hit for {1}", this::getCacheName, () -> key); return me.getCacheElement(); @@ -223,15 +178,7 @@ public abstract class AbstractMemoryCache<K, V> @Override public Set<K> getKeySet() { - lock.readLock().lock(); - try - { - return Collections.unmodifiableSet(map.keySet()); - } - finally - { - lock.readLock().unlock(); - } + return Collections.unmodifiableSet(map.keySet()); } /** @@ -267,17 +214,7 @@ public abstract class AbstractMemoryCache<K, V> @Override public ICacheElement<K, V> getQuiet( final K key ) { - MemoryElementDescriptor<K, V> me = null; - - lock.readLock().lock(); - try - { - me = map.get( key ); - } - finally - { - lock.readLock().unlock(); - } + MemoryElementDescriptor<K, V> me = map.get(key); if (me == null) { @@ -299,15 +236,7 @@ public abstract class AbstractMemoryCache<K, V> @Override public int getSize() { - lock.readLock().lock(); - try - { - return this.map.size(); - } - finally - { - lock.readLock().unlock(); - } + return map.size(); } /** @@ -347,6 +276,32 @@ public abstract class AbstractMemoryCache<K, V> log.info("initialized {0} for {1}", cacheImplementationName, cacheName); } + /** + * This instructs the memory cache to remove the <em>numberToFree</em> according to its eviction + * policy. For example, the LRUMemoryCache will remove the <em>numberToFree</em> least recently + * used items. These will be spooled to disk if a disk auxiliary is available. + * + * @param numberToFree + * @return The number that were removed. if you ask to free 5, but there are only 3, you will + * get 3. + */ + @Override + public int freeElements(final int numberToFree) throws IOException + { + int freed = 0; + + for (; freed < numberToFree; freed++) + { + final ICacheElement<K, V> element = freeElement(); + if (element == null) + { + break; + } + } + + return freed; + } + /** * Wrap the cache element into an appropriate memory element descriptor * @@ -357,46 +312,39 @@ public abstract class AbstractMemoryCache<K, V> /** * Update control structures after get - * (guarded by the lock) * * @param me The memory element descriptor */ - protected abstract void lockedGetElement(MemoryElementDescriptor<K, V> me); + protected abstract void adjustGetElement(MemoryElementDescriptor<K, V> me); /** * Update control structures after update - * (guarded by the lock) * * @param newNode The memory element descriptor of the current cache element * @throws IOException if spooling operation fails */ - protected abstract void lockedUpdateElement(MemoryElementDescriptor<K, V> newNode) throws IOException; + protected abstract void adjustUpdateElement(MemoryElementDescriptor<K, V> newNode) throws IOException; /** * Removes all cached items from the cache control structures. - * (guarded by the lock) */ - protected abstract void lockedRemoveAll(); + protected abstract void adjustRemoveAll(); /** * Remove element from control structure - * (guarded by the lock) * * @param me The memory element descriptor */ - protected abstract void lockedRemoveElement(MemoryElementDescriptor<K, V> me); + protected abstract void adjustRemoveElement(MemoryElementDescriptor<K, V> me); /** - * This instructs the memory cache to remove the <em>numberToFree</em> according to its eviction - * policy. For example, the LRUMemoryCache will remove the <em>numberToFree</em> least recently - * used items. These will be spooled to disk if a disk auxiliary is available. - * (guarded by the lock) + * This instructs the memory cache to remove the last element according to its eviction + * policy. For example, the LRUMemoryCache will remove the least recently + * used item. These will be spooled to disk if a disk auxiliary is available. * - * @param numberToFree - * @return The number that were removed. if you ask to free 5, but there are only 3, you will - * get 3. + * @return the element that was spooled, null if none */ - protected abstract int lockedFreeElements(final int numberToFree) throws IOException; + protected abstract ICacheElement<K, V> freeElement() throws IOException; /** * Removes an item from the cache. This method handles hierarchical removal. If the key is a @@ -426,19 +374,12 @@ public abstract class AbstractMemoryCache<K, V> else { // remove single item. - lock.writeLock().lock(); - try - { - final MemoryElementDescriptor<K, V> me = map.remove(key); - if (me != null) - { - lockedRemoveElement(me); - removed = true; - } - } - finally + final MemoryElementDescriptor<K, V> me = map.remove(key); + + if (me != null) { - lock.writeLock().unlock(); + removed = true; + adjustRemoveElement(me); } } @@ -451,16 +392,8 @@ public abstract class AbstractMemoryCache<K, V> @Override public void removeAll() { - lock.writeLock().lock(); - try - { - map.clear(); - lockedRemoveAll(); - } - finally - { - lock.writeLock().unlock(); - } + map.clear(); + adjustRemoveAll(); } /** @@ -470,26 +403,20 @@ public abstract class AbstractMemoryCache<K, V> */ protected boolean removeByGroup(final GroupId groupId) { - lock.writeLock().lock(); - try - { - // remove all keys of the same group hierarchy. - return map.entrySet().removeIf(entry -> { - final K k = entry.getKey(); - - if (k instanceof GroupAttrName kgan && kgan.groupId().equals(groupId)) - { - lockedRemoveElement(entry.getValue()); - return true; - } - - return false; - }); - } - finally - { - lock.writeLock().unlock(); - } + // remove all keys of the same group hierarchy. + boolean removed = map.entrySet().removeIf(entry -> { + final K k = entry.getKey(); + + if (k instanceof GroupAttrName kgan && kgan.groupId().equals(groupId)) + { + adjustRemoveElement(entry.getValue()); + return true; + } + + return false; + }); + + return removed; } /** @@ -500,26 +427,20 @@ public abstract class AbstractMemoryCache<K, V> */ protected boolean removeByHierarchy(final String keyString) { - lock.writeLock().lock(); - try - { - // remove all keys of the same name hierarchy. - return map.entrySet().removeIf(entry -> { - final K k = entry.getKey(); + // remove all keys of the same name hierarchy. + boolean removed = map.entrySet().removeIf(entry -> { + final K k = entry.getKey(); - if (k instanceof String s && s.startsWith(keyString)) - { - lockedRemoveElement(entry.getValue()); - return true; - } + if (k instanceof String s && s.startsWith(keyString)) + { + adjustRemoveElement(entry.getValue()); + return true; + } - return false; - }); - } - finally - { - lock.writeLock().unlock(); - } + return false; + }); + + return removed; } /** @@ -533,27 +454,19 @@ public abstract class AbstractMemoryCache<K, V> throws IOException { putCnt.incrementAndGet(); + final MemoryElementDescriptor<K, V> newNode = map.compute(ce.key(), (k, v) -> { + if (v == null) + { + return wrap(ce); + } + else + { + v.setCacheElement(ce); + return v; + } + }); - lock.writeLock().lock(); - try - { - final MemoryElementDescriptor<K, V> newNode = map.compute(ce.key(), (k, v) -> { - if (v == null) - { - return wrap(ce); - } - else - { - v.setCacheElement(ce); - return v; - } - }); - lockedUpdateElement(newNode); - } - finally - { - lock.writeLock().unlock(); - } + adjustUpdateElement(newNode); } /** diff --git a/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/lru/LHMLRUMemoryCache.java b/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/lru/LHMLRUMemoryCache.java index 51fedea7..e56de9e8 100644 --- a/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/lru/LHMLRUMemoryCache.java +++ b/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/lru/LHMLRUMemoryCache.java @@ -20,8 +20,11 @@ package org.apache.commons.jcs4.engine.memory.lru; */ import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentMap; import org.apache.commons.jcs4.engine.behavior.ICacheElement; @@ -46,11 +49,14 @@ public class LHMLRUMemoryCache<K, V> /** * Implements removeEldestEntry from {@link LinkedHashMap}. */ - protected class LHMSpooler extends LinkedHashMap<K, MemoryElementDescriptor<K, V>> + private class LHMSpooler implements ConcurrentMap<K, MemoryElementDescriptor<K, V>> { - /** Don't change. */ - private static final long serialVersionUID = -1255907868906762484L; + /** cache size */ + private final int maxObjects; + + /** LinkedHashMap delegate */ + private final Map<K, MemoryElementDescriptor<K, V>> delegate; /** * Initialize to a small size--for now, 1/2 of max 3rd variable "true" indicates that it @@ -58,32 +64,133 @@ public class LHMLRUMemoryCache<K, V> */ public LHMSpooler() { - super( (int) ( getCacheAttributes().MaxObjects() * .5 ), .75F, true ); + this.maxObjects = getCacheAttributes().MaxObjects(); + this.delegate = Collections.synchronizedMap( + new LinkedHashMap<>((int) (maxObjects * .5), .75F, true) + { + /** Don't change. */ + private static final long serialVersionUID = -1255907868906762484L; + + /** + * Remove eldest. Automatically called by LinkedHashMap. + * + * @param eldest + * @return true if removed + */ + @Override + protected boolean removeEldestEntry(final Map.Entry<K, MemoryElementDescriptor<K, V>> eldest) + { + final ICacheElement<K, V> element = eldest.getValue().getCacheElement(); + + if (maxObjects < 0 || size() <= maxObjects) + { + return false; + } + log.debug( "LHMLRU max size: {0}. Spooling element, key: {1}", + () -> getCacheAttributes().MaxObjects(), element::key); + + waterfall(element); + + log.debug("LHMLRU size: {0}", this::size); + return true; + } + }); + } + + @Override + public int size() + { + return delegate.size(); + } + + @Override + public boolean isEmpty() + { + return delegate.isEmpty(); + } + + @Override + public boolean containsKey(Object key) + { + return delegate.containsKey(key); + } + + @Override + public boolean containsValue(Object value) + { + return delegate.containsValue(value); + } + + @Override + public MemoryElementDescriptor<K, V> get(Object key) + { + return delegate.get(key); + } + + @Override + public MemoryElementDescriptor<K, V> put(K key, MemoryElementDescriptor<K, V> value) + { + return delegate.put(key, value); + } + + @Override + public MemoryElementDescriptor<K, V> remove(Object key) + { + return delegate.remove(key); + } + + @Override + public void putAll(Map<? extends K, ? extends MemoryElementDescriptor<K, V>> m) + { + delegate.putAll(m); + } + + @Override + public void clear() + { + delegate.clear(); + } + + @Override + public Set<K> keySet() + { + return delegate.keySet(); + } + + @Override + public Collection<MemoryElementDescriptor<K, V>> values() + { + return delegate.values(); + } + + @Override + public Set<Entry<K, MemoryElementDescriptor<K, V>>> entrySet() + { + return delegate.entrySet(); } - /** - * Remove eldest. Automatically called by LinkedHashMap. - * - * @param eldest - * @return true if removed - */ @Override - protected boolean removeEldestEntry( final Map.Entry<K, MemoryElementDescriptor<K, V>> eldest ) + public MemoryElementDescriptor<K, V> putIfAbsent(K key, MemoryElementDescriptor<K, V> value) { - final ICacheElement<K, V> element = eldest.getValue().getCacheElement(); - final int maxObjects = getCacheAttributes().MaxObjects(); + return delegate.putIfAbsent(key, value); + } - if (maxObjects < 0 || size() <= maxObjects) - { - return false; - } - log.debug( "LHMLRU max size: {0}. Spooling element, key: {1}", - () -> getCacheAttributes().MaxObjects(), element::key); + @Override + public boolean remove(Object key, Object value) + { + return delegate.remove(key, value); + } - waterfall(element); + @Override + public boolean replace(K key, MemoryElementDescriptor<K, V> oldValue, MemoryElementDescriptor<K, V> newValue) + { + return delegate.replace(key, oldValue, newValue); + } - log.debug("LHMLRU size: {0}", getSize()); - return true; + @Override + public MemoryElementDescriptor<K, V> replace(K key, MemoryElementDescriptor<K, V> value) + { + return delegate.replace(key, value); } } @@ -117,7 +224,7 @@ public class LHMLRUMemoryCache<K, V> * @param me The memory element descriptor */ @Override - protected void lockedGetElement(final MemoryElementDescriptor<K, V> me) + protected void adjustGetElement(final MemoryElementDescriptor<K, V> me) { // empty } @@ -129,7 +236,7 @@ public class LHMLRUMemoryCache<K, V> * @param newNode The memory element descriptor of the current cache element */ @Override - protected void lockedUpdateElement(MemoryElementDescriptor<K, V> newNode) + protected void adjustUpdateElement(MemoryElementDescriptor<K, V> newNode) { // empty } @@ -139,7 +246,7 @@ public class LHMLRUMemoryCache<K, V> * (guarded by the lock) */ @Override - protected void lockedRemoveAll() + protected void adjustRemoveAll() { // empty } @@ -151,21 +258,20 @@ public class LHMLRUMemoryCache<K, V> * @param me The memory element descriptor */ @Override - protected void lockedRemoveElement(final MemoryElementDescriptor<K, V> me) + protected void adjustRemoveElement(final MemoryElementDescriptor<K, V> me) { // empty } /** - * This can't be implemented. + * Cannot be implemented * - * @param numberToFree - * @return 0 + * @return ICacheElement<K, V> if there was a last element, else null. * @throws IOException */ @Override - protected int lockedFreeElements(final int numberToFree) throws IOException + protected ICacheElement<K, V> freeElement() throws IOException { - return 0; + return null; } } diff --git a/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/soft/SoftReferenceMemoryCache.java b/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/soft/SoftReferenceMemoryCache.java index 3910eb1f..82d1a5b4 100644 --- a/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/soft/SoftReferenceMemoryCache.java +++ b/commons-jcs4-core/src/main/java/org/apache/commons/jcs4/engine/memory/soft/SoftReferenceMemoryCache.java @@ -75,18 +75,10 @@ public class SoftReferenceMemoryCache<K, V> extends AbstractMemoryCache<K, V> @Override public Set<K> getKeySet() { - lock.readLock().lock(); - try - { - return getMapView().entrySet().stream() - .filter(e -> e.getValue().getCacheElement() != null) - .map(e -> e.getKey()) - .collect(Collectors.toSet()); - } - finally - { - lock.readLock().unlock(); - } + return getMapView().entrySet().stream() + .filter(e -> e.getValue().getCacheElement() != null) + .map(e -> e.getKey()) + .collect(Collectors.toSet()); } /** @@ -97,19 +89,11 @@ public class SoftReferenceMemoryCache<K, V> extends AbstractMemoryCache<K, V> @Override public int getSize() { - lock.readLock().lock(); - try - { - long size = getMapView().values().stream() - .filter(v -> v.getCacheElement() != null) - .count(); + long size = getMapView().values().stream() + .filter(v -> v.getCacheElement() != null) + .count(); - return (int) size; - } - finally - { - lock.readLock().unlock(); - } + return (int) size; } /** @@ -153,15 +137,13 @@ public class SoftReferenceMemoryCache<K, V> extends AbstractMemoryCache<K, V> /** * Update control structures after get - * (guarded by the lock) * * @param me The memory element descriptor */ @Override - protected void lockedGetElement(final MemoryElementDescriptor<K, V> me) + protected void adjustGetElement(final MemoryElementDescriptor<K, V> me) { final ICacheElement<K, V> val = me.getCacheElement(); - val.elementAttributes().setLastAccessTimeNow(); // update the ordering of the strong references strongReferences.add(val); @@ -175,10 +157,9 @@ public class SoftReferenceMemoryCache<K, V> extends AbstractMemoryCache<K, V> * @param newNode The memory element descriptor of the current cache element */ @Override - protected void lockedUpdateElement(MemoryElementDescriptor<K, V> newNode) + protected void adjustUpdateElement(MemoryElementDescriptor<K, V> newNode) { final ICacheElement<K, V> val = newNode.getCacheElement(); - val.elementAttributes().setLastAccessTimeNow(); // update the ordering of the strong references strongReferences.add(val); @@ -190,34 +171,32 @@ public class SoftReferenceMemoryCache<K, V> extends AbstractMemoryCache<K, V> * (guarded by the lock) */ @Override - protected void lockedRemoveAll() + protected void adjustRemoveAll() { strongReferences.clear(); } /** * Remove element from control structure - * (guarded by the lock) * * @param me The memory element descriptor */ @Override - protected void lockedRemoveElement(final MemoryElementDescriptor<K, V> me) + protected void adjustRemoveElement(final MemoryElementDescriptor<K, V> me) { strongReferences.remove(me.getCacheElement()); } /** - * This can't be implemented. + * Cannot be implemented * - * @param numberToFree - * @return 0 + * @return ICacheElement<K, V> if there was a last element, else null. * @throws IOException */ @Override - protected int lockedFreeElements(final int numberToFree) throws IOException + protected ICacheElement<K, V> freeElement() throws IOException { - return 0; + return null; } /** @@ -229,12 +208,19 @@ public class SoftReferenceMemoryCache<K, V> extends AbstractMemoryCache<K, V> final int max = getCacheAttributes().MaxObjects(); final int startsize = strongReferences.size(); - for (int cursize = startsize; cursize > max; cursize--) + if (max > 0) { - final ICacheElement<K, V> ce = strongReferences.poll(); - if (ce != null) + for (int cursize = startsize; cursize > max; cursize--) { - waterfall(ce); + final ICacheElement<K, V> ce = strongReferences.poll(); + if (ce != null) + { + waterfall(ce); + } + else + { + break; + } } } }
