Skip to content

Commit

Permalink
Fix b/80241237 to correctly *not* change segment weight, nor report a…
Browse files Browse the repository at this point in the history
…n eviction, when a compute() call does not change the present value.

RELNOTES=Fix a bug where Cache.asMap.compute* methods could cause nonsensical weights to be stored, breaking cache eviction.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198618238
  • Loading branch information
lowasser authored and ronshapiro committed May 31, 2018
1 parent 3ac6f72 commit 15764d7
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
Expand Up @@ -55,6 +55,30 @@ public void testComputeIfAbsent() {
assertEquals(1, cache.size());
}

public void testComputeIfAbsentEviction() {
// b/80241237

Cache<String, String> c = CacheBuilder.newBuilder().maximumSize(1).build();

assertThat(c.asMap().computeIfAbsent("hash-1", k -> "")).isEqualTo("");
assertThat(c.asMap().computeIfAbsent("hash-1", k -> "")).isEqualTo("");
assertThat(c.asMap().computeIfAbsent("hash-1", k -> "")).isEqualTo("");
assertThat(c.size()).isEqualTo(1);
assertThat(c.asMap().computeIfAbsent("hash-2", k -> "")).isEqualTo("");
}

public void testComputeEviction() {
// b/80241237

Cache<String, String> c = CacheBuilder.newBuilder().maximumSize(1).build();

assertThat(c.asMap().compute("hash-1", (k, v) -> "a")).isEqualTo("a");
assertThat(c.asMap().compute("hash-1", (k, v) -> "b")).isEqualTo("b");
assertThat(c.asMap().compute("hash-1", (k, v) -> "c")).isEqualTo("c");
assertThat(c.size()).isEqualTo(1);
assertThat(c.asMap().computeIfAbsent("hash-2", k -> "")).isEqualTo("");
}

public void testComputeIfPresent() {
cache.put(key, "1");
// simultaneous update for same key, expect count successful updates
Expand Down
3 changes: 2 additions & 1 deletion guava/src/com/google/common/cache/LocalCache.java
Expand Up @@ -2243,7 +2243,8 @@ V compute(K key, int hash, BiFunction<? super K, ? super V, ? extends V> functio
if (newValue != null) {
if (valueReference != null && newValue == valueReference.get()) {
loadingValueReference.set(newValue);
setValue(e, key, newValue, now);
e.setValueReference(valueReference);
recordWrite(e, 0, now); // no change in weight
return newValue;
}
try {
Expand Down

0 comments on commit 15764d7

Please sign in to comment.