From b0cd1a5583c612aca4763ee1f8d61be0d2084a8f Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Sat, 19 Mar 2016 15:37:39 -0700 Subject: [PATCH] Do not admit entries that exceed the maximum weight If an entry's weight exceeds the cache's maximum capacity, then evict it immediately. This avoids flushing the probation segment, though it probably does flush the admission window (1% of the total size). Thanks to @blemale for reminding me when I peeked at his WeigherSpec test [1]. This was fixed in Guava [2], but I had forgotten to port that over after v19 was released. Also improved the correction of evaluating the admission candidates when evicting multiple entries. When a candidate is accepted or evicted the count is decremented and the next is set for evaluation. This most often occurs for weighted caches. [1] https://github.com/blemale/scaffeine [2] https://github.com/google/guava/commit/b3855ef79650e282563561368577253d7abcc385 --- .../caffeine/cache/BoundedLocalCache.java | 11 ++++ .../benmanes/caffeine/cache/EvictionTest.java | 22 ++++++++ .../benmanes/caffeine/cache/Stresser.java | 1 + gradle/dependencies.gradle | 6 +-- .../common/cache/CacheEvictionTest.java | 52 ++++++++++++++++++- 5 files changed, 88 insertions(+), 4 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 1164c097ee..a368eb2fb2 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 @@ -562,13 +562,24 @@ void evictFromMain(int candidates) { continue; } + // Evict immediately if the candidate's weight exceeds the maximum + if (candidate.getPolicyWeight() > maximum()) { + candidates--; + Node evict = candidate; + candidate = candidate.getPreviousInAccessOrder(); + evictEntry(evict, RemovalCause.SIZE, 0L); + continue; + } + // Evict the entry with the lowest frequency + candidates--; int victimFreq = frequencySketch().frequency(victimKey); int candidateFreq = frequencySketch().frequency(candidateKey); if (candidateFreq > victimFreq) { Node evict = victim; victim = victim.getNextInAccessOrder(); evictEntry(evict, RemovalCause.SIZE, 0L); + candidate = candidate.getPreviousInAccessOrder(); } else { Node evict = candidate; candidate = candidate.getPreviousInAccessOrder(); 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 d7f8ea0463..85d1c023d2 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 @@ -57,6 +57,7 @@ import com.github.benmanes.caffeine.cache.testing.CheckNoWriter; import com.github.benmanes.caffeine.cache.testing.RejectingCacheWriter.DeleteException; import com.github.benmanes.caffeine.cache.testing.RemovalListeners.RejectingRemovalListener; +import com.github.benmanes.caffeine.cache.testing.RemovalNotification; import com.github.benmanes.caffeine.testing.Awaits; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -170,6 +171,27 @@ public void evict_weighted(Cache> cache, }); } + @Test(dataProvider = "caches") + @CacheSpec(population = Population.EMPTY, removalListener = Listener.CONSUMING, + keys = ReferenceType.STRONG, values = ReferenceType.STRONG, + maximumSize = Maximum.TEN, weigher = CacheWeigher.VALUE) + public void evict_weighted_entryTooBig(Cache cache, CacheContext context) { + cache.put(1, 1); + cache.put(9, 9); + assertThat(cache.estimatedSize(), is(2L)); + cache.policy().eviction().ifPresent(eviction -> { + assertThat(eviction.weightedSize().getAsLong(), is(10L)); + }); + + cache.put(20, 20); + assertThat(cache.estimatedSize(), is(2L)); + cache.policy().eviction().ifPresent(eviction -> { + assertThat(eviction.weightedSize().getAsLong(), is(10L)); + }); + assertThat(context.consumedNotifications(), is(equalTo(ImmutableList.of( + new RemovalNotification<>(20, 20, RemovalCause.SIZE))))); + } + @Test(dataProvider = "caches") @CacheSpec(implementation = Implementation.Caffeine, maximumSize = Maximum.TEN, weigher = CacheWeigher.VALUE, population = Population.EMPTY, diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/Stresser.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/Stresser.java index 4b04437754..a426c2af27 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/Stresser.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/Stresser.java @@ -75,6 +75,7 @@ public Stresser() { cache.put(key, key); return key; }); + cache.cleanUp(); stopwatch = Stopwatch.createStarted(); status(); } diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index d5067fc001..8adacb8ac6 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -32,7 +32,7 @@ ext { error_prone_annotations: '2.0.8', flip_tables: '1.0.2', guava: '19.0', - javapoet: '1.5.1', + javapoet: '1.6.0', jcache: '1.0.0', jsr305: '3.0.1', stream: '2.9.1', @@ -44,7 +44,7 @@ ext { easymock: '3.4', hamcrest: '2.0.0.0', jcache_tck: '1.0.1', - jctools: '1.1', + jctools: '1.2', junit: '4.12', mockito: '2.0.44-beta', pax_exam: '4.8.0', @@ -62,7 +62,7 @@ ext { jamm: '0.3.1', java_object_layout: '0.4', koloboke: '0.6.8', - slf4j: '1.7.18', + slf4j: '1.7.19', tcache: '0.9.0', ] plugin_versions = [ diff --git a/guava/src/test/java/com/google/common/cache/CacheEvictionTest.java b/guava/src/test/java/com/google/common/cache/CacheEvictionTest.java index 5c5f53f780..e178f33f16 100644 --- a/guava/src/test/java/com/google/common/cache/CacheEvictionTest.java +++ b/guava/src/test/java/com/google/common/cache/CacheEvictionTest.java @@ -18,6 +18,7 @@ import static com.google.common.cache.TestingRemovalListeners.countingRemovalListener; import static com.google.common.cache.TestingWeighers.constantWeigher; import static com.google.common.cache.TestingWeighers.intKeyWeigher; +import static com.google.common.cache.TestingWeighers.intValueWeigher; import static com.google.common.truth.Truth.assertThat; import static java.util.Arrays.asList; @@ -159,6 +160,55 @@ public void testEviction_maxWeight_zero() { CacheTesting.checkValidState(cache); } + /** + * Tests that when a single entry exceeds the segment's max weight, the new entry is + * immediately evicted and nothing else. + */ + public void testEviction_maxWeight_entryTooBig() { + CountingRemovalListener removalListener = countingRemovalListener(); + IdentityLoader loader = identityLoader(); + + LoadingCache cache = CaffeinatedGuava.build(Caffeine.newBuilder() + .maximumWeight(4) + .weigher(intValueWeigher()) + .removalListener(removalListener) + .executor(MoreExecutors.directExecutor()), + loader); + + // caches 2 + assertThat(cache.getUnchecked(2)).isEqualTo(2); + assertThat(cache.asMap().keySet()).containsExactly(2); + + assertThat(removalListener.getCount()).isEqualTo(0); + + // caches 3, evicts 2 + assertThat(cache.getUnchecked(3)).isEqualTo(3); + assertThat(cache.asMap().keySet()).containsExactly(3); + + assertThat(removalListener.getCount()).isEqualTo(1); + + // doesn't cache 5, doesn't evict + assertThat(cache.getUnchecked(5)).isEqualTo(5); + assertThat(cache.asMap().keySet()).containsExactly(3); + + assertThat(removalListener.getCount()).isEqualTo(2); + + // caches 1, evicts nothing + assertThat(cache.getUnchecked(1)).isEqualTo(1); + assertThat(cache.asMap().keySet()).containsExactly(3, 1); + + assertThat(removalListener.getCount()).isEqualTo(2); + + // caches 4, evicts 1 and 3 + assertThat(cache.getUnchecked(4)).isEqualTo(4); + assertThat(cache.asMap().keySet()).containsExactly(4); + + assertThat(removalListener.getCount()).isEqualTo(4); + + // Should we pepper more of these calls throughout the above? Where? + CacheTesting.checkValidState(cache); + } + public void testEviction_overflow() { CountingRemovalListener removalListener = countingRemovalListener(); IdentityLoader loader = identityLoader(); @@ -289,7 +339,7 @@ public void testEviction_overweight() { // add an at-the-maximum-weight entry getAll(cache, asList(45)); CacheTesting.drainRecencyQueues(cache); - assertThat(keySet).containsExactly(0, 8, 9); + assertThat(keySet).containsExactly(0, 45); // add an over-the-maximum-weight entry getAll(cache, asList(46));