From 9d9b0c404b66b82aeeff0efb56ffe328f3926abc Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Wed, 10 Mar 2021 17:11:27 -0800 Subject: [PATCH] Improved eviction reordering for weighted caches (fixes #513) For weighted caches, if an entry in the probation queue is too large to be promoted to the protected queue, it was skipped. This could leave it as the victim, making it eligable for eviction. In small caches, often used by test code, the premature eviction can be surprising if one still assumes LRU. To reduce this confusion, we can reorder within the probation queue and treat it as an LRU instead of FIFO. This test case already passes in v3 due to changes in how it selects candidates in order to handle excessively large entries. However, codifying the intent seems worthwhile and, if it helps reduce unit test confusion, then a nice benefit. --- .../caffeine/cache/BoundedLocalCache.java | 1 + .../benmanes/caffeine/cache/EvictionTest.java | 17 +++++++++++++++++ checksum.xml | 3 +++ gradle/dependencies.gradle | 6 +++--- 4 files changed, 24 insertions(+), 3 deletions(-) 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 f586d56214..b25c4aa3ef 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 @@ -1630,6 +1630,7 @@ void reorderProbation(Node node) { // Ignore stale accesses for an entry that is no longer present return; } else if (node.getPolicyWeight() > mainProtectedMaximum()) { + reorder(accessOrderProbationDeque(), node); return; } diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java index 354745d8a1..dda1f4e435 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java @@ -22,6 +22,7 @@ import static com.github.benmanes.caffeine.testing.IsEmptyMap.emptyMap; import static java.util.Arrays.asList; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; @@ -158,6 +159,22 @@ public void evict_weighted(Cache> cache, verifyStats(context, verifier -> verifier.evictions(3).evictionWeight(13)); } + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY, + initialCapacity = InitialCapacity.EXCESSIVE, maximumSize = Maximum.TEN, + weigher = CacheWeigher.COLLECTION, keys = ReferenceType.STRONG, values = ReferenceType.STRONG) + public void evict_weighted_reorder(Cache> cache, + CacheContext context, Eviction eviction) { + eviction.setMaximum(3); + for (int i = 1; i <= 3; i++) { + cache.put(i, List.of(1)); + } + cache.asMap().computeIfPresent(1, (k, v) -> List.of(1, 2)); + assertThat(cache.getIfPresent(1), hasSize(2)); + assertThat(cache.asMap(), is(aMapWithSize(2))); + assertThat(eviction.weightedSize().getAsLong(), is(3L)); + } + @Test(dataProvider = "caches") @CacheSpec(population = Population.EMPTY, removalListener = Listener.CONSUMING, keys = ReferenceType.STRONG, values = ReferenceType.STRONG, diff --git a/checksum.xml b/checksum.xml index f6f92c2708..d9c14f6afd 100644 --- a/checksum.xml +++ b/checksum.xml @@ -387,6 +387,9 @@ A9789681117B1D01021884DB7D0F6367CA3B7F4AC6A4630BC6769DFDF6D1003E49CD7A894D803BFA6244AEE5135F610EEAA4E2E1AD67F8C5196C8B7A1CEEE451 + + 4D2D1FBDC1C8EBF905F8BCE824B3B87DEF250BFE77465D68A24A5FB348B40AED821AFD24060132A7320920F7C87B3A4B8373ECFD033C08D4660A625C6CD8B5C5 + B3BFAD07E6A3D4D73CBCE802D8614CF4AC84E589166D243D41028DC077F84C027DF4D514F145360405F37DA73A8F2E7B65D90877A9EE1151174D2440530F9051 diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index babb48a267..e6ff5421ca 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -37,7 +37,7 @@ ext { ehcache3: '3.9.2', errorprone: '2.5.1', errorproneJavac: '9+181-r4173-1', - elasticSearch: '7.11.1', + elasticSearch: '7.11.2', expiringMap: '0.5.9', fastfilter: '1.0', fastutil: '8.5.2', @@ -62,7 +62,7 @@ ext { univocityParsers: '2.9.1', ycsb: '0.17.0', xz: '1.8', - zstd: '1.4.8-7', + zstd: '1.4.9-1', ] testVersions = [ awaitility: '4.0.3', @@ -97,7 +97,7 @@ ext { shadow: '6.1.0', sonarqube: '3.1.1', spotbugs: '4.2.2', - spotbugsPlugin: '4.6.2', + spotbugsPlugin: '4.7.0', stats: '0.2.2', versions: '0.38.0', ]