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.

.github/workflows/build.yml

@@ -38,7 +38,8 @@ jobs:
           && endsWith(github.ref, github.event.repository.default_branch)
         env:
           COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
-        run: ./gradlew coveralls
+        run: ./gradlew coveralls -S
+        continue-on-error: true
       - name: SonarQube
         if: >
           matrix.java == env.MAX_JVM
@@ -47,7 +48,8 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
-        run: ./gradlew sonarqube
+        run: ./gradlew sonarqube -S
+        continue-on-error: true
       - name: Publish Snapshot
         if: >
           matrix.java == env.MIN_JVM

caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java

@@ -1630,6 +1630,7 @@ void reorderProbation(Node<K, V> node) {
       // Ignore stale accesses for an entry that is no longer present
       return;
     } else if (node.getPolicyWeight() > mainProtectedMaximum()) {
+      reorder(accessOrderProbationDeque(), node);
       return;
     }
 

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<Integer, List<Integer>> 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<Integer, List<Integer>> 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,

checksum.xml

@@ -387,6 +387,9 @@
     <dependency group='gradle.plugin.com.github.spotbugs.snom' module='spotbugs-gradle-plugin' version='4.6.2'>
       <sha512>A9789681117B1D01021884DB7D0F6367CA3B7F4AC6A4630BC6769DFDF6D1003E49CD7A894D803BFA6244AEE5135F610EEAA4E2E1AD67F8C5196C8B7A1CEEE451</sha512>
     </dependency>
+    <dependency group='gradle.plugin.com.github.spotbugs.snom' module='spotbugs-gradle-plugin' version='4.7.0'>
+      <sha512>4D2D1FBDC1C8EBF905F8BCE824B3B87DEF250BFE77465D68A24A5FB348B40AED821AFD24060132A7320920F7C87B3A4B8373ECFD033C08D4660A625C6CD8B5C5</sha512>
+    </dependency>
     <dependency group='gradle.plugin.com.github.spotbugs' module='spotbugs-gradle-plugin' version='2.0.0'>
       <sha512>B3BFAD07E6A3D4D73CBCE802D8614CF4AC84E589166D243D41028DC077F84C027DF4D514F145360405F37DA73A8F2E7B65D90877A9EE1151174D2440530F9051</sha512>
     </dependency>

gradle/codeQuality.gradle

@@ -98,24 +98,14 @@ jacocoTestReport {
   }
 }
 
-task jacocoMerge(type: JacocoMerge) {
-  executionData tasks.withType(Test)
-
-  doFirst {
-    executionData = files(executionData.findAll { it.exists() })
-  }
-}
-
 sonarqube {
   properties {
     property 'sonar.organization', 'caffeine'
     property 'sonar.projectKey', 'caffeine:caffeine'
     property 'sonar.login', System.env.'SONAR_TOKEN'
     property 'sonar.host.url', 'https://sonarcloud.io'
-    property 'sonar.jacoco.reportPath', jacocoMerge.destinationFile
   }
 }
-tasks.sonarqube.dependsOn(jacocoMerge)
 
 tasks.withType(JavaCompile).configureEach {
   dependsOn downloadCaffeineLocal

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',
   ]