diff --git a/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddToString.java b/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddToString.java index 50dee4e84f..c53a8a2c4c 100644 --- a/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddToString.java +++ b/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddToString.java @@ -33,7 +33,7 @@ protected boolean applies() { @Override protected void execute() { - String statement = "return String.format(\"%s=[key=%s, value=%s, weight=%d, moveCount=%,d, " + String statement = "return String.format(\"%s=[key=%s, value=%s, weight=%d, queueType=%,d, " + "accessTimeNS=%,d, \"\n+ \"writeTimeNS=%,d, prevInAccess=%s, nextInAccess=%s, " + "prevInWrite=%s, nextInWrite=%s]\",\ngetClass().getSimpleName(), getKey(), getValue(), " + "getWeight(), getQueueType(), \ngetAccessTime(), getWriteTime(), " diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java index 52993dc178..146cf445e7 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java @@ -582,34 +582,31 @@ void expireEntries() { expireAfterWriteEntries(now); } - /** Expires entries on the access-order queue. */ + /** Expires entries in the access-order queue. */ @GuardedBy("evictionLock") void expireAfterAccessEntries(long now) { - if (expiresAfterAccess()) { - long expirationTime = now - expiresAfterAccessNanos(); - for (;;) { - final Node node = accessOrderEdenDeque().peekFirst(); - if ((node == null) || (node.getAccessTime() > expirationTime)) { - break; - } - evictEntry(node, RemovalCause.EXPIRED, now); - } - if (evicts()) { - for (;;) { - final Node node = accessOrderProbationDeque().peekFirst(); - if ((node == null) || (node.getAccessTime() > expirationTime)) { - break; - } - evictEntry(node, RemovalCause.EXPIRED, now); - } - for (;;) { - final Node node = accessOrderProtectedDeque().peekFirst(); - if ((node == null) || (node.getAccessTime() > expirationTime)) { - break; - } - evictEntry(node, RemovalCause.EXPIRED, now); - } + if (!expiresAfterAccess()) { + return; + } + + long expirationTime = (now - expiresAfterAccessNanos()); + expireAfterAccessEntries(accessOrderEdenDeque(), expirationTime, now); + if (evicts()) { + expireAfterAccessEntries(accessOrderProbationDeque(), expirationTime, now); + expireAfterAccessEntries(accessOrderProtectedDeque(), expirationTime, now); + } + } + + /** Expires entries in an access-order queue. */ + @GuardedBy("evictionLock") + void expireAfterAccessEntries(AccessOrderDeque> accessOrderDeque, + long expirationTime, long now) { + for (;;) { + final Node node = accessOrderDeque.peekFirst(); + if ((node == null) || (node.getAccessTime() > expirationTime)) { + break; } + evictEntry(node, RemovalCause.EXPIRED, now); } } @@ -693,10 +690,10 @@ void evictEntry(Node node, RemovalCause cause, long now) { // the addition that triggered this eviction. The victim is eagerly unlinked before the removal // task so that if an eviction is still required then a new victim will be chosen for removal. makeDead(node); - if (node.isEden() && (evicts() || expiresAfterAccess())) { + if (node.inEden() && (evicts() || expiresAfterAccess())) { accessOrderEdenDeque().remove(node); } else if (evicts()) { - if (node.isMainProbation()) { + if (node.inMainProbation()) { accessOrderProbationDeque().remove(node); } else { accessOrderProtectedDeque().remove(node); @@ -916,33 +913,10 @@ void onAccess(Node node) { return; } frequencySketch().increment(key); - if (node.isEden()) { + if (node.inEden()) { reorder(accessOrderEdenDeque(), node); - } else if (node.isMainProbation()) { - if (!accessOrderProbationDeque().contains(node)) { - // Ignore stale accesses for an entry that is no longer present - return; - } else if (node.getPolicyWeight() > mainProtectedMaximum()) { - return; - } - - long mainProtectedWeightedSize = mainProtectedWeightedSize() + node.getPolicyWeight(); - accessOrderProbationDeque().remove(node); - accessOrderProtectedDeque().add(node); - node.makeMainProtected(); - - long mainProtectedMaximum = mainProtectedMaximum(); - while (mainProtectedWeightedSize > mainProtectedMaximum) { - Node demoted = accessOrderProtectedDeque().pollFirst(); - if (demoted == null) { - break; - } - demoted.makeMainProbation(); - accessOrderProbationDeque().add(demoted); - mainProtectedWeightedSize -= node.getPolicyWeight(); - } - - lazySetMainProtectedWeightedSize(mainProtectedWeightedSize); + } else if (node.inMainProbation()) { + reorderProbation(node); } else { reorder(accessOrderProtectedDeque(), node); } @@ -951,6 +925,35 @@ void onAccess(Node node) { } } + /** Updates the node's location in the probation queue. */ + @GuardedBy("evictionLock") + void reorderProbation(Node node) { + if (!accessOrderProbationDeque().contains(node)) { + // Ignore stale accesses for an entry that is no longer present + return; + } else if (node.getPolicyWeight() > mainProtectedMaximum()) { + return; + } + + long mainProtectedWeightedSize = mainProtectedWeightedSize() + node.getPolicyWeight(); + accessOrderProbationDeque().remove(node); + accessOrderProtectedDeque().add(node); + node.makeMainProtected(); + + long mainProtectedMaximum = mainProtectedMaximum(); + while (mainProtectedWeightedSize > mainProtectedMaximum) { + Node demoted = accessOrderProtectedDeque().pollFirst(); + if (demoted == null) { + break; + } + demoted.makeMainProbation(); + accessOrderProbationDeque().add(demoted); + mainProtectedWeightedSize -= node.getPolicyWeight(); + } + + lazySetMainProtectedWeightedSize(mainProtectedWeightedSize); + } + /** Updates the node's location in the policy's deque. */ static void reorder(LinkedDeque> deque, Node node) { // An entry may be scheduled for reordering despite having been removed. This can occur when the @@ -989,9 +992,9 @@ void makeDead(Node node) { // The node's policy weight may be out of sync due to a pending update waiting to be // processed. At this point the node's weight is finalized, so the weight can be safely // taken from the node's perspective and the sizes will be adjusted correctly. - if (node.isEden()) { + if (node.inEden()) { lazySetEdenWeightedSize(edenWeightedSize() - node.getWeight()); - } else if (node.isMainProtected()) { + } else if (node.inMainProtected()) { lazySetMainProtectedWeightedSize(mainProtectedWeightedSize() - node.getWeight()); } lazySetWeightedSize(weightedSize() - node.getWeight()); @@ -1074,10 +1077,10 @@ final class RemovalTask implements Runnable { @GuardedBy("evictionLock") public void run() { // add may not have been processed yet - if (node.isEden() && (evicts() || expiresAfterAccess())) { + if (node.inEden() && (evicts() || expiresAfterAccess())) { accessOrderEdenDeque().remove(node); } else if (evicts()) { - if (node.isMainProbation()) { + if (node.inMainProbation()) { accessOrderProbationDeque().remove(node); } else { accessOrderProtectedDeque().remove(node); @@ -1104,9 +1107,9 @@ public UpdateTask(Node node, int weightDifference) { @GuardedBy("evictionLock") public void run() { if (evicts()) { - if (node.isEden()) { + if (node.inEden()) { lazySetEdenWeightedSize(edenWeightedSize() + weightDifference); - } else if (node.isMainProtected()) { + } else if (node.inMainProtected()) { lazySetMainProtectedWeightedSize(mainProtectedMaximum() + weightDifference); } lazySetWeightedSize(weightedSize() + weightDifference); @@ -2156,7 +2159,7 @@ public K next() { @Override public void remove() { - Caffeine.requireState(current != null); + requireState(current != null); BoundedLocalCache.this.remove(current); current = null; } @@ -2292,7 +2295,7 @@ public Entry next() { @Override public void remove() { - Caffeine.requireState(removalKey != null); + requireState(removalKey != null); BoundedLocalCache.this.remove(removalKey); removalKey = null; } @@ -2527,11 +2530,13 @@ final class BoundedRefreshAfterWrite implements Expiration { cache.setRefreshAfterWriteNanos(unit.toNanos(duration)); cache.executor().execute(cache.drainBuffersTask); } + @SuppressWarnings("PMD.SimplifiedTernary") // false positive (#1424) @Override public Map oldest(int limit) { return cache.expiresAfterWrite() ? expireAfterWrite().get().oldest(limit) : sortedByWriteTime(limit, true); } + @SuppressWarnings("PMD.SimplifiedTernary") // false positive (#1424) @Override public Map youngest(int limit) { return cache.expiresAfterWrite() ? expireAfterWrite().get().youngest(limit) diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java index 1f748c98ff..ae2c36c9a6 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java @@ -548,8 +548,7 @@ public boolean replace(K key, V oldValue, V newValue) { requireNonNull(newValue); CompletableFuture oldValueFuture = delegate.get(key); return oldValue.equals(Async.getIfReady(oldValueFuture)) - ? delegate.replace(key, oldValueFuture, CompletableFuture.completedFuture(newValue)) - : false; + && delegate.replace(key, oldValueFuture, CompletableFuture.completedFuture(newValue)); } @Override @@ -560,8 +559,7 @@ public boolean remove(Object key, Object value) { } CompletableFuture oldValueFuture = delegate.get(key); return value.equals(Async.getIfReady(oldValueFuture)) - ? delegate.remove(key, oldValueFuture) - : false; + && delegate.remove(key, oldValueFuture); } @Override diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Node.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Node.java index 56ca371f8d..f76c5ae176 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Node.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Node.java @@ -125,17 +125,17 @@ default void setPolicyWeight(int weight) {} int PROTECTED = 2; /** Returns if the entry is in the Eden or Main space. */ - default boolean isEden() { + default boolean inEden() { return getQueueType() == EDEN; } /** Returns if the entry is in the Main space's probation queue. */ - default boolean isMainProbation() { + default boolean inMainProbation() { return getQueueType() == PROBATION; } /** Returns if the entry is in the Main space's protected queue. */ - default boolean isMainProtected() { + default boolean inMainProtected() { return getQueueType() == PROTECTED; } diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheValidationListener.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheValidationListener.java index 8a5aeff980..36f9538855 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheValidationListener.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheValidationListener.java @@ -33,6 +33,7 @@ import java.util.Map; import java.util.Objects; +import org.apache.commons.lang3.StringUtils; import org.testng.IInvokedMethod; import org.testng.IInvokedMethodListener; import org.testng.ITestResult; @@ -73,12 +74,18 @@ public void afterInvocation(IInvokedMethod method, ITestResult testResult) { } } catch (Throwable caught) { testResult.setStatus(ITestResult.FAILURE); - testResult.setThrowable(caught); + testResult.setThrowable(new AssertionError(getTestName(method), caught)); } finally { cleanUp(testResult); } } + /** Returns the name of the executed test. */ + private static String getTestName(IInvokedMethod method) { + return StringUtils.substringAfterLast(method.getTestMethod().getTestClass().getName(), ".") + + "#" + method.getTestMethod().getConstructorOrMethod().getName(); + } + /** Checks whether the {@link TrackingExecutor} had unexpected failures. */ private static void checkExecutor(ITestResult testResult, CacheContext context) { Method testMethod = testResult.getMethod().getConstructorOrMethod().getMethod(); diff --git a/config/pmd/rulesSets.xml b/config/pmd/rulesSets.xml index 8e43825ed4..f9f8ec89aa 100644 --- a/config/pmd/rulesSets.xml +++ b/config/pmd/rulesSets.xml @@ -6,7 +6,6 @@ - @@ -28,6 +27,7 @@ + diff --git a/gradle/code_quality.gradle b/gradle/code_quality.gradle index 62137c08a7..be8bcaccdf 100644 --- a/gradle/code_quality.gradle +++ b/gradle/code_quality.gradle @@ -35,7 +35,7 @@ findbugs { pmd { ruleSets = [] - toolVersion = '5.3.3' + toolVersion = '5.4.1' ruleSetConfig = resources.text.fromFile(file("${rootDir}/config/pmd/rulesSets.xml")) } diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java index ce6f1894fc..271cb558f4 100644 --- a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java +++ b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java @@ -116,7 +116,6 @@ private final class Builder { CacheLoader cacheLoader; JCacheEvictionListener evictionListener; - @SuppressWarnings("PMD.StringToString") Builder(String cacheName, CaffeineConfiguration config) { this.config = config; this.cacheName = cacheName; diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/CaffeineConfiguration.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/CaffeineConfiguration.java index a461dce9de..fdbf076c7b 100644 --- a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/CaffeineConfiguration.java +++ b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/CaffeineConfiguration.java @@ -38,8 +38,8 @@ * A JCache configuration with Caffeine specific settings. *

* The initial settings disable store by value so that entries are not copied when crossing - * the {@link javax.cache.Cache} API boundary. If enabled and the {@link Copier} is explicitly set, - * then the {@link JavaSerializationCopier} will be used. This differs from + * the {@link javax.cache.Cache} API boundary. If enabled and the {@link Copier} is not explicitly + * set, then the {@link JavaSerializationCopier} will be used. This differs from * {@link MutableConfiguration} which enables store by value at construction. * * @author ben.manes@gmail.com (Ben Manes) diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java index 37ccea60ec..ac7d6b153f 100644 --- a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java +++ b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java @@ -43,6 +43,7 @@ * * @author ben.manes@gmail.com (Ben Manes) */ +@SuppressWarnings("PMD.AvoidDuplicateLiterals") public final class TypesafeConfigurator { static final Logger logger = Logger.getLogger(TypesafeConfigurator.class.getName());