ensure that the expiration ticker is not used for stats (fixes #1678)

.gitignore

@@ -14,6 +14,7 @@ jitwatch.out
 .vscode
 .idea
 *.iml
+*.hprof
 hs_err_pid*.log
 replay_pid*.log
 simulator/src/main/resources/application.conf

caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java

@@ -860,7 +860,7 @@ public Caffeine<K, V> ticker(Ticker ticker) {
 
   Ticker getTicker() {
     boolean useTicker = expiresVariable() || expiresAfterAccess()
-        || expiresAfterWrite() || refreshAfterWrite() || isRecordingStats();
+        || expiresAfterWrite() || refreshAfterWrite();
     return useTicker
         ? (ticker == null) ? Ticker.systemTicker() : ticker
         : Ticker.disabledTicker();

caffeine/src/main/java/com/github/benmanes/caffeine/cache/UnboundedLocalCache.java

@@ -72,7 +72,6 @@ final class UnboundedLocalCache<K, V> implements LocalCache<K, V> {
   final boolean isRecordingStats;
   final Executor executor;
   final boolean isAsync;
-  final Ticker ticker;
 
   @Nullable Set<K> keySet;
   @Nullable Collection<V> values;
@@ -85,7 +84,6 @@ final class UnboundedLocalCache<K, V> implements LocalCache<K, V> {
     this.removalListener = builder.getRemovalListener(isAsync);
     this.isRecordingStats = builder.isRecordingStats();
     this.executor = builder.getExecutor();
-    this.ticker = builder.getTicker();
     this.isAsync = isAsync;
   }
 
@@ -237,7 +235,7 @@ void discardRefresh(Object keyReference) {
 
   @Override
   public Ticker statsTicker() {
-    return ticker;
+    return isRecordingStats ? Ticker.systemTicker() : Ticker.disabledTicker();
   }
 
   /* --------------- JDK8+ Map extensions --------------- */
@@ -1056,7 +1054,6 @@ Object writeReplace() {
       SerializationProxy<K, V> proxy = new SerializationProxy<>();
       proxy.isRecordingStats = cache.isRecordingStats;
       proxy.removalListener = cache.removalListener;
-      proxy.ticker = cache.ticker;
       return proxy;
     }
   }
@@ -1208,7 +1205,6 @@ Object writeReplace() {
       SerializationProxy<K, V> proxy = new SerializationProxy<>();
       proxy.isRecordingStats = cache.isRecordingStats;
       proxy.removalListener = cache.removalListener;
-      proxy.ticker = cache.ticker;
       proxy.async = true;
       return proxy;
     }
@@ -1264,7 +1260,6 @@ Object writeReplace() {
       proxy.isRecordingStats = cache.isRecordingStats();
       proxy.removalListener = cache.removalListener;
       proxy.cacheLoader = cacheLoader;
-      proxy.ticker = cache.ticker;
       proxy.async = true;
       return proxy;
     }

caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java

@@ -29,6 +29,10 @@
 import static org.junit.Assert.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
 import static org.slf4j.event.Level.ERROR;
 import static org.slf4j.event.Level.WARN;
 
@@ -37,6 +41,7 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -49,6 +54,7 @@
 import java.util.stream.IntStream;
 
 import org.apache.commons.lang3.tuple.Triple;
+import org.mockito.Mockito;
 import org.mockito.stubbing.Answer;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
@@ -67,11 +73,14 @@
 import com.github.benmanes.caffeine.cache.testing.CacheProvider;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.CacheExecutor;
+import com.github.benmanes.caffeine.cache.testing.CacheSpec.CacheExpiry;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Compute;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.ExecutorFailure;
+import com.github.benmanes.caffeine.cache.testing.CacheSpec.Expire;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Implementation;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Listener;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Population;
+import com.github.benmanes.caffeine.cache.testing.CacheSpec.Stats;
 import com.github.benmanes.caffeine.cache.testing.CacheValidationListener;
 import com.github.benmanes.caffeine.cache.testing.CheckMaxLogLevel;
 import com.github.benmanes.caffeine.cache.testing.CheckNoEvictions;
@@ -859,6 +868,39 @@ public void removalListener_submit_error_log(Cache<Int, Int> cache, CacheContext
     assertThat(event.getThrowable().orElseThrow()).isInstanceOf(RejectedExecutionException.class);
   }
 
+  @CheckNoStats
+  @Test(dataProvider = "caches")
+  @CacheSpec(population = Population.EMPTY, refreshAfterWrite = Expire.DISABLED,
+      expireAfterAccess = Expire.DISABLED, expireAfterWrite = Expire.DISABLED,
+      expiry = CacheExpiry.DISABLED, stats = Stats.DISABLED)
+  public void ticker_noStats(CacheContext context) {
+    var caffeineTicker = Mockito.mock(Ticker.class);
+    var guavaTicker = Mockito.mock(com.google.common.base.Ticker.class);
+    when(guavaTicker.read()).thenAnswer(invocation -> context.ticker().read());
+    when(caffeineTicker.read()).thenAnswer(invocation -> context.ticker().read());
+    context.ticker().setAutoIncrementStep(Duration.ofSeconds(1));
+
+    if (context.isGuava()) {
+      context.guava().ticker(guavaTicker);
+    } else {
+      context.caffeine().ticker(caffeineTicker);
+    }
+
+    Cache<Int, Int> cache = context.build(key -> key);
+    cache.getIfPresent(context.absentKey());
+    cache.get(context.absentKey(), key -> null);
+    cache.get(context.absentKey(), key -> key);
+    cache.getIfPresent(context.absentKey());
+
+    assertThat(cache.estimatedSize()).isEqualTo(1);
+    if (context.isGuava()) {
+      // Used for expiration timestamp and ignored
+      verify(guavaTicker, times(4)).read();
+    } else {
+      verifyNoInteractions(caffeineTicker);
+    }
+  }
+
   /* --------------- serialize --------------- */
 
   @CacheSpec

caffeine/src/test/java/com/github/benmanes/caffeine/cache/LocalCacheSubject.java

@@ -18,6 +18,7 @@
 import static com.github.benmanes.caffeine.cache.LinkedDequeSubject.deque;
 import static com.github.benmanes.caffeine.testing.Awaits.await;
 import static com.github.benmanes.caffeine.testing.MapSubject.map;
+import static com.google.common.truth.Truth.assertThat;
 
 import java.util.Collection;
 import java.util.Map;
@@ -36,6 +37,7 @@
 import com.github.benmanes.caffeine.cache.UnboundedLocalCache.UnboundedLocalAsyncCache;
 import com.github.benmanes.caffeine.cache.UnboundedLocalCache.UnboundedLocalAsyncLoadingCache;
 import com.github.benmanes.caffeine.cache.UnboundedLocalCache.UnboundedLocalManualCache;
+import com.github.benmanes.caffeine.cache.stats.StatsCounter;
 import com.github.benmanes.caffeine.cache.testing.Weighers;
 import com.google.common.collect.ImmutableTable;
 import com.google.common.collect.Sets;
@@ -110,13 +112,24 @@ public void isValid() {
     }
   }
 
+  private void checkStats(LocalCache<?, ?> cache) {
+    if (cache.isRecordingStats()) {
+      assertThat(cache.statsTicker()).isSameInstanceAs(Ticker.systemTicker());
+      assertThat(cache.statsCounter()).isNotSameInstanceAs(StatsCounter.disabledStatsCounter());
+    } else {
+      assertThat(cache.statsTicker()).isSameInstanceAs(Ticker.disabledTicker());
+      assertThat(cache.statsCounter()).isSameInstanceAs(StatsCounter.disabledStatsCounter());
+    }
+  }
+
   /* --------------- Bounded --------------- */
 
   private void checkBounded(BoundedLocalCache<Object, Object> bounded) {
     drain(bounded);
     checkReadBuffer(bounded);
 
     checkCache(bounded);
+    checkStats(bounded);
     checkTimerWheel(bounded);
     checkEvictionDeque(bounded);
   }
@@ -409,5 +422,6 @@ private void checkUnbounded(UnboundedLocalCache<?, ?> unbounded) {
       check("value").that(value).isNotNull();
       checkIfAsyncValue(value);
     });
+    checkStats(unbounded);
   }
 }

caffeine/src/test/java/com/github/benmanes/caffeine/cache/ReserializableSubject.java

@@ -253,7 +253,6 @@ private void checkUnboundedAsyncLocalLoadingCache(
 
   private void checkUnboundedLocalCache(
       UnboundedLocalCache<?, ?> original, UnboundedLocalCache<?, ?> copy) {
-    check("ticker").that(copy.ticker).isEqualTo(original.ticker);
     check("isRecordingStats").that(copy.isRecordingStats).isEqualTo(original.isRecordingStats);
 
     if (original.removalListener == null) {