Guava cache sync

Fix some rawtypes in com.google.common.cache google/guava@310de86 Saturate LongAdder.sum() results to Long.MAX_VALUE during an overflow. google/guava@b28a260 Co-authored-by: Kurt Alfred Kluever <[email protected]>
ben-manes · Jul 9, 2019 · 96edea8 · 96edea8
1 parent e5f3d19
commit 96edea8
Show file tree

Hide file tree

Showing 6 changed files with 171 additions and 20 deletions.
diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalManualCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalManualCache.java
@@ -15,6 +15,8 @@
  */
 package com.github.benmanes.caffeine.cache;
 
+import static java.util.Objects.requireNonNull;
+
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -68,8 +70,10 @@ default Map<K, V> getAllPresent(Iterable<?> keys) {
   @Override
   default Map<K, V> getAll(Iterable<? extends K> keys,
       Function<Iterable<? extends K>, Map<K, V>> mappingFunction) {
+    requireNonNull(mappingFunction);
+
     Set<K> keysToLoad = new LinkedHashSet<>();
-    Map<K, V> found = getAllPresent(keys);
+    Map<K, V> found = cache().getAllPresent(keys);
     Map<K, V> result = new LinkedHashMap<>(found.size());
     for (K key : keys) {
       V value = found.get(key);

diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/stats/ConcurrentStatsCounter.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/stats/ConcurrentStatsCounter.java
@@ -71,13 +71,13 @@ public void recordLoadFailure(long loadTime) {
     totalLoadTime.add(loadTime);
   }
 
-  @Override
+  @Override @Deprecated
   @SuppressWarnings("deprecation")
   public void recordEviction() {
     evictionCount.increment();
   }
 
-  @Override
+  @Override @Deprecated
   @SuppressWarnings("deprecation")
   public void recordEviction(int weight) {
     evictionCount.increment();
@@ -93,17 +93,18 @@ public void recordEviction(int weight, RemovalCause cause) {
   @Override
   public CacheStats snapshot() {
     return new CacheStats(
-        saturated(hitCount.sum()),
-        saturated(missCount.sum()),
-        saturated(loadSuccessCount.sum()),
-        saturated(loadFailureCount.sum()),
-        saturated(totalLoadTime.sum()),
-        saturated(evictionCount.sum()),
-        saturated(evictionWeight.sum()));
+        negativeToMaxValue(hitCount.sum()),
+        negativeToMaxValue(missCount.sum()),
+        negativeToMaxValue(loadSuccessCount.sum()),
+        negativeToMaxValue(loadFailureCount.sum()),
+        negativeToMaxValue(totalLoadTime.sum()),
+        negativeToMaxValue(evictionCount.sum()),
+        negativeToMaxValue(evictionWeight.sum()));
   }
 
-  private static long saturated(long count) {
-    return (count >= 0) ? count : Long.MAX_VALUE;
+  /** Returns {@code value}, if non-negative. Otherwise, returns {@link Long#MAX_VALUE}. */
+  private static long negativeToMaxValue(long value) {
+    return (value >= 0) ? value : Long.MAX_VALUE;
   }
 
   /**

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/stats/StatsCounterTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/stats/StatsCounterTest.java
@@ -132,4 +132,22 @@ public void guarded_exception() {
     verify(statsCounter).recordLoadSuccess(1);
     verify(statsCounter).recordLoadFailure(1);
   }
+
+  @Test
+  public void overflow_loadSuccess() {
+    StatsCounter counter = new ConcurrentStatsCounter();
+    counter.recordLoadSuccess(Long.MAX_VALUE);
+    counter.recordLoadSuccess(1);
+    CacheStats stats = counter.snapshot();
+    assertThat(stats.totalLoadTime(), is(Long.MAX_VALUE));
+  }
+
+  @Test
+  public void overflow_loadFailure() {
+    StatsCounter counter = new ConcurrentStatsCounter();
+    counter.recordLoadFailure(Long.MAX_VALUE);
+    counter.recordLoadFailure(1);
+    CacheStats stats = counter.snapshot();
+    assertThat(stats.totalLoadTime(), is(Long.MAX_VALUE));
+  }
 }
diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
@@ -50,7 +50,7 @@ ext {
     jcacheTck: '1.1.1',
     jctools: '2.1.2',
     junit: '4.12',
-    mockito: '2.28.2',
+    mockito: '3.0.0',
     paxExam: '4.13.1',
     testng: '6.14.3',
     truth: '0.24',

diff --git a/guava/src/test/java/com/google/common/cache/AbstractCacheTest.java b/guava/src/test/java/com/google/common/cache/AbstractCacheTest.java
@@ -0,0 +1,133 @@
+/*
+ * Copyright (C) 2011 The Guava Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.common.cache;
+
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.github.benmanes.caffeine.cache.stats.CacheStats;
+import com.github.benmanes.caffeine.cache.stats.ConcurrentStatsCounter;
+import com.github.benmanes.caffeine.cache.stats.StatsCounter;
+
+import junit.framework.TestCase;
+
+/**
+ * Unit test for {@link AbstractCache}.
+ *
+ * @author Charles Fry
+ */
+public class AbstractCacheTest extends TestCase {
+
+  public void testEmptySimpleStats() {
+    StatsCounter counter = new ConcurrentStatsCounter();
+    CacheStats stats = counter.snapshot();
+    assertEquals(0, stats.requestCount());
+    assertEquals(0, stats.hitCount());
+    assertEquals(1.0, stats.hitRate());
+    assertEquals(0, stats.missCount());
+    assertEquals(0.0, stats.missRate());
+    assertEquals(0, stats.loadSuccessCount());
+    assertEquals(0, stats.loadFailureCount());
+    assertEquals(0, stats.loadCount());
+    assertEquals(0, stats.totalLoadTime());
+    assertEquals(0.0, stats.averageLoadPenalty());
+    assertEquals(0, stats.evictionCount());
+  }
+
+  public void testSingleSimpleStats() {
+    StatsCounter counter = new ConcurrentStatsCounter();
+    for (int i = 0; i < 11; i++) {
+      counter.recordHits(1);
+    }
+    for (int i = 0; i < 13; i++) {
+      counter.recordLoadSuccess(i);
+    }
+    for (int i = 0; i < 17; i++) {
+      counter.recordLoadFailure(i);
+    }
+    for (int i = 0; i < 23; i++) {
+      counter.recordMisses(1);
+    }
+    for (int i = 0; i < 27; i++) {
+      counter.recordEviction(1, RemovalCause.SIZE);
+    }
+    CacheStats stats = counter.snapshot();
+    int requestCount = 11 + 23;
+    assertEquals(requestCount, stats.requestCount());
+    assertEquals(11, stats.hitCount());
+    assertEquals(11.0 / requestCount, stats.hitRate());
+    int missCount = 23;
+    assertEquals(missCount, stats.missCount());
+    assertEquals(((double) missCount) / requestCount, stats.missRate());
+    assertEquals(13, stats.loadSuccessCount());
+    assertEquals(17, stats.loadFailureCount());
+    assertEquals(13 + 17, stats.loadCount());
+    assertEquals(214, stats.totalLoadTime());
+    assertEquals(214.0 / (13 + 17), stats.averageLoadPenalty());
+    assertEquals(27, stats.evictionCount());
+  }
+
+  public void testSimpleStatsOverflow() {
+    StatsCounter counter = new ConcurrentStatsCounter();
+    counter.recordLoadSuccess(Long.MAX_VALUE);
+    counter.recordLoadSuccess(1);
+    CacheStats stats = counter.snapshot();
+    assertEquals(Long.MAX_VALUE, stats.totalLoadTime());
+  }
+
+  public void testSimpleStatsIncrementBy() {
+    long totalLoadTime = 0;
+
+    ConcurrentStatsCounter counter1 = new ConcurrentStatsCounter();
+    for (int i = 0; i < 11; i++) {
+      counter1.recordHits(1);
+    }
+    for (int i = 0; i < 13; i++) {
+      counter1.recordLoadSuccess(i);
+      totalLoadTime += i;
+    }
+    for (int i = 0; i < 17; i++) {
+      counter1.recordLoadFailure(i);
+      totalLoadTime += i;
+    }
+    for (int i = 0; i < 19; i++) {
+      counter1.recordMisses(1);
+    }
+    for (int i = 0; i < 23; i++) {
+      counter1.recordEviction(1, RemovalCause.SIZE);
+    }
+
+    ConcurrentStatsCounter counter2 = new ConcurrentStatsCounter();
+    for (int i = 0; i < 27; i++) {
+      counter2.recordHits(1);
+    }
+    for (int i = 0; i < 31; i++) {
+      counter2.recordLoadSuccess(i);
+      totalLoadTime += i;
+    }
+    for (int i = 0; i < 37; i++) {
+      counter2.recordLoadFailure(i);
+      totalLoadTime += i;
+    }
+    for (int i = 0; i < 41; i++) {
+      counter2.recordMisses(1);
+    }
+    for (int i = 0; i < 43; i++) {
+      counter1.recordEviction(1, RemovalCause.SIZE);
+    }
+
+    counter1.incrementBy(counter2);
+    assertEquals(new CacheStats(38, 60, 44, 54, totalLoadTime, 66, 66), counter1.snapshot());
+  }
+}
diff --git a/guava/src/test/java/com/google/common/cache/PopulatedCachesTest.java b/guava/src/test/java/com/google/common/cache/PopulatedCachesTest.java
@@ -243,20 +243,15 @@ public void testValues_populated() {
     }
   }
 
-  @SuppressWarnings("rawtypes")
   public void testEntrySet_populated() {
     for (LoadingCache<Object, Object> cache : caches()) {
       Set<Entry<Object, Object>> entries = cache.asMap().entrySet();
       List<Entry<Object, Object>> warmed = warmUp(cache, WARMUP_MIN, WARMUP_MAX);
 
       Set<?> expected = Maps.newHashMap(cache.asMap()).entrySet();
       assertThat(entries).containsExactlyElementsIn(expected);
-      assertThat(entries.toArray())
-          .asList()
-          .containsExactlyElementsIn(expected);
-      assertThat(entries.toArray(new Entry[0]))
-          .asList()
-          .containsExactlyElementsIn(expected);
+      assertThat(entries.toArray()).asList().containsExactlyElementsIn(expected);
+      assertThat(entries.toArray(new Object[0])).asList().containsExactlyElementsIn(expected);
 
       new EqualsTester()
           .addEqualityGroup(cache.asMap().entrySet(), entries)