Skip to content

Commit

Permalink
Improved eviction reordering for weighted caches (fixes #513)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ben-manes committed Mar 11, 2021
1 parent a8c274d commit 9c60b45
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 6 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/build.yml
Expand Up @@ -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
Expand All @@ -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
Expand Down
Expand Up @@ -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;
}

Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions checksum.xml
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion gradle/codeQuality.gradle
Expand Up @@ -112,7 +112,7 @@ sonarqube {
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
property 'sonar.jacoco.reportPaths', jacocoMerge.destinationFile
}
}
tasks.sonarqube.dependsOn(jacocoMerge)
Expand Down
6 changes: 3 additions & 3 deletions gradle/dependencies.gradle
Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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',
]
Expand Down

0 comments on commit 9c60b45

Please sign in to comment.