From 15764d7f37160d81c3489c860c868c3e92d6c637 Mon Sep 17 00:00:00 2001 From: lowasser Date: Wed, 30 May 2018 14:19:31 -0700 Subject: [PATCH] Fix b/80241237 to correctly *not* change segment weight, nor report an 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 --- .../cache/LocalCacheMapComputeTest.java | 24 +++++++++++++++++++ .../com/google/common/cache/LocalCache.java | 3 ++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java b/guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java index 94fbf82907e6..c49b80e1e6bb 100644 --- a/guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java +++ b/guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java @@ -55,6 +55,30 @@ public void testComputeIfAbsent() { assertEquals(1, cache.size()); } + public void testComputeIfAbsentEviction() { + // b/80241237 + + Cache 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 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 diff --git a/guava/src/com/google/common/cache/LocalCache.java b/guava/src/com/google/common/cache/LocalCache.java index 6d21f97a6166..87e9d5c755f1 100644 --- a/guava/src/com/google/common/cache/LocalCache.java +++ b/guava/src/com/google/common/cache/LocalCache.java @@ -2243,7 +2243,8 @@ V compute(K key, int hash, BiFunction 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 {