From 50af7d79e10a40da038e79cd0c8898e45e7ab824 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. --- .github/workflows/build.yml | 6 ++++-- .../caffeine/cache/BoundedLocalCache.java | 1 + .../benmanes/caffeine/cache/EvictionTest.java | 17 +++++++++++++++++ checksum.xml | 3 +++ gradle/codeQuality.gradle | 10 ---------- gradle/dependencies.gradle | 6 +++--- 6 files changed, 28 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c5b6c95ded..c420ff2962 100644 --- a/.github/workflows/build.yml +++ b/.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 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/codeQuality.gradle b/gradle/codeQuality.gradle index a12d2a45df..a3c32ebb3f 100644 --- a/gradle/codeQuality.gradle +++ b/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 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', ]