Skip to content

Commit

Permalink
Do not admit entries that exceed the maximum weight
Browse files Browse the repository at this point in the history
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] google/guava@b3855ef
  • Loading branch information
ben-manes committed Mar 20, 2016
1 parent 80ea370 commit b0cd1a5
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<K, V> 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<K, V> evict = victim;
victim = victim.getNextInAccessOrder();
evictEntry(evict, RemovalCause.SIZE, 0L);
candidate = candidate.getPreviousInAccessOrder();
} else {
Node<K, V> evict = candidate;
candidate = candidate.getPreviousInAccessOrder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -170,6 +171,27 @@ public void evict_weighted(Cache<Integer, List<Integer>> 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<Integer, Integer> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public Stresser() {
cache.put(key, key);
return key;
});
cache.cleanUp();
stopwatch = Stopwatch.createStarted();
status();
}
Expand Down
6 changes: 3 additions & 3 deletions gradle/dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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 = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Integer, Integer> removalListener = countingRemovalListener();
IdentityLoader<Integer> loader = identityLoader();

LoadingCache<Integer, Integer> 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<Object, Object> removalListener = countingRemovalListener();
IdentityLoader<Object> loader = identityLoader();
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit b0cd1a5

Please sign in to comment.