From b5dbe2d9138e25a1073c784de4a73ce0502cd297 Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Tue, 13 Jul 2021 23:32:52 -0700 Subject: [PATCH] Optimize refreshAfterWrite When an entry is eligible for refresh, only one thread should block to schedule it. Previously all readers would block on a map computeIfAbsent operation to obtain the future. While this allows refreshes to be linearizable, it adds a small and unnecessary synchronization point. The change restores the non-blocking behavior of v2.x, while keeping the improvements of v3's rewrite. The write timestamp is CAS'd as a soft lock to allow subsequent readers to skip attempting to refresh. The least significant bit is used as a flag for locking, causing the timestamp to be off by 1ns from the ideal value. (Thanks @Maaartinus for suggesting this idea in https://github.com/ben-manes/caffeine/issues/282#issuecomment-440887934) Also restored from v2 is to suppress and log exceptions if the cache loader fails when producing the refresh future. The inspections to obtain an entry's age were improved, such as not resurrecting an expired entry. --- .../caffeine/cache/node/AddExpiration.java | 13 ++- .../caffeine/cache/BoundedLocalCache.java | 96 ++++++++------- .../benmanes/caffeine/cache/Policy.java | 28 +---- .../caffeine/cache/BoundedLocalCacheTest.java | 2 +- .../caffeine/cache/ExpireAfterAccessTest.java | 20 ++++ .../caffeine/cache/ExpireAfterVarTest.java | 56 +++++++++ .../caffeine/cache/ExpireAfterWriteTest.java | 26 ++++- .../caffeine/cache/RefreshAfterWriteTest.java | 110 +++++++++++++++++- gradle/dependencies.gradle | 6 +- 9 files changed, 275 insertions(+), 82 deletions(-) diff --git a/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddExpiration.java b/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddExpiration.java index 90c5d33612..051f7a49e9 100644 --- a/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddExpiration.java +++ b/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddExpiration.java @@ -115,8 +115,8 @@ private void addAccessExpiration() { .addMethod(newGetter(Strength.STRONG, TypeName.LONG, "accessTime", Visibility.PLAIN)) .addMethod(newSetter(TypeName.LONG, "accessTime", Visibility.PLAIN)); addVarHandle("accessTime", TypeName.get(long.class)); - addTimeConstructorAssignment(context.constructorByKey, "accessTime"); - addTimeConstructorAssignment(context.constructorByKeyRef, "accessTime"); + addTimeConstructorAssignment(context.constructorByKey, "accessTime", "now"); + addTimeConstructorAssignment(context.constructorByKeyRef, "accessTime", "now"); } private void addWriteExpiration() { @@ -127,8 +127,8 @@ private void addWriteExpiration() { .addMethod(newGetter(Strength.STRONG, TypeName.LONG, "writeTime", Visibility.PLAIN)) .addMethod(newSetter(TypeName.LONG, "writeTime", Visibility.PLAIN)); addVarHandle("writeTime", TypeName.get(long.class)); - addTimeConstructorAssignment(context.constructorByKey, "writeTime"); - addTimeConstructorAssignment(context.constructorByKeyRef, "writeTime"); + addTimeConstructorAssignment(context.constructorByKey, "writeTime", "now & ~1L"); + addTimeConstructorAssignment(context.constructorByKeyRef, "writeTime", "now & ~1L"); } } @@ -147,7 +147,8 @@ private void addRefreshExpiration() { } /** Adds a long constructor assignment. */ - private void addTimeConstructorAssignment(MethodSpec.Builder constructor, String field) { - constructor.addStatement("$L.set(this, $N)", varHandleName(field), "now"); + private void addTimeConstructorAssignment( + MethodSpec.Builder constructor, String field, String value) { + constructor.addStatement("$L.set(this, $N)", varHandleName(field), value); } } diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java index 6cfa68d50f..c374f09490 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java @@ -55,7 +55,6 @@ import java.util.Set; import java.util.Spliterator; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executor; @@ -1242,42 +1241,48 @@ void refreshIfNeeded(Node node, long now) { K key; V oldValue; long writeTime = node.getWriteTime(); + long refreshWriteTime = writeTime | 1L; Object keyReference = node.getKeyReference(); if (((now - writeTime) > refreshAfterWriteNanos()) && (keyReference != null) && ((key = node.getKey()) != null) && ((oldValue = node.getValue()) != null) - && !refreshes().containsKey(keyReference)) { + && !refreshes().containsKey(keyReference) + && ((writeTime & 1L) == 0L) && node.casWriteTime(writeTime, refreshWriteTime)) { long[] startTime = new long[1]; @SuppressWarnings({"unchecked", "rawtypes"}) CompletableFuture[] refreshFuture = new CompletableFuture[1]; - refreshes().computeIfAbsent(keyReference, k -> { - try { - startTime[0] = statsTicker().read(); - if (isAsync) { - @SuppressWarnings("unchecked") - CompletableFuture future = (CompletableFuture) oldValue; - if (Async.isReady(future)) { + try { + refreshes().computeIfAbsent(keyReference, k -> { + try { + startTime[0] = statsTicker().read(); + if (isAsync) { + @SuppressWarnings("unchecked") + CompletableFuture future = (CompletableFuture) oldValue; + if (Async.isReady(future)) { + @SuppressWarnings("NullAway") + var refresh = cacheLoader.asyncReload(key, future.join(), executor); + refreshFuture[0] = refresh; + } else { + // no-op if load is pending + return future; + } + } else { @SuppressWarnings("NullAway") - var refresh = cacheLoader.asyncReload(key, future.join(), executor); + var refresh = cacheLoader.asyncReload(key, oldValue, executor); refreshFuture[0] = refresh; - } else { - // no-op if load is pending - return future; } - } else { - @SuppressWarnings("NullAway") - var refresh = cacheLoader.asyncReload(key, oldValue, executor); - refreshFuture[0] = refresh; + return refreshFuture[0]; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + logger.log(Level.WARNING, "Exception thrown when submitting refresh task", e); + return null; + } catch (Throwable e) { + logger.log(Level.WARNING, "Exception thrown when submitting refresh task", e); + return null; } - return refreshFuture[0]; - } catch (RuntimeException e) { - throw e; - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new CompletionException(e); - } catch (Exception e) { - throw new CompletionException(e); - } - }); + }); + } finally { + node.casWriteTime(refreshWriteTime, writeTime); + } if (refreshFuture[0] != null) { refreshFuture[0].whenComplete((newValue, error) -> { @@ -1417,7 +1422,7 @@ void setVariableTime(Node node, long expirationTime) { void setWriteTime(Node node, long now) { if (expiresAfterWrite() || refreshAfterWrite()) { - node.setWriteTime(now); + node.setWriteTime(now & ~1L); } } @@ -3631,14 +3636,14 @@ final class BoundedExpireAfterAccess implements FixedExpiration { requireNonNull(key); requireNonNull(unit); Object lookupKey = cache.nodeFactory.newLookupKey(key); - Node node = cache.data.get(lookupKey); + Node node = cache.data.get(lookupKey); if (node == null) { return OptionalLong.empty(); } - long age = cache.expirationTicker().read() - node.getAccessTime(); - return (age > cache.expiresAfterAccessNanos()) + long now = cache.expirationTicker().read(); + return cache.hasExpired(node, now) ? OptionalLong.empty() - : OptionalLong.of(unit.convert(age, TimeUnit.NANOSECONDS)); + : OptionalLong.of(unit.convert(now - node.getAccessTime(), TimeUnit.NANOSECONDS)); } @Override public long getExpiresAfter(TimeUnit unit) { return unit.convert(cache.expiresAfterAccessNanos(), TimeUnit.NANOSECONDS); @@ -3662,14 +3667,14 @@ final class BoundedExpireAfterWrite implements FixedExpiration { requireNonNull(key); requireNonNull(unit); Object lookupKey = cache.nodeFactory.newLookupKey(key); - Node node = cache.data.get(lookupKey); + Node node = cache.data.get(lookupKey); if (node == null) { return OptionalLong.empty(); } - long age = cache.expirationTicker().read() - node.getWriteTime(); - return (age > cache.expiresAfterWriteNanos()) + long now = cache.expirationTicker().read(); + return cache.hasExpired(node, now) ? OptionalLong.empty() - : OptionalLong.of(unit.convert(age, TimeUnit.NANOSECONDS)); + : OptionalLong.of(unit.convert(now - node.getWriteTime(), TimeUnit.NANOSECONDS)); } @Override public long getExpiresAfter(TimeUnit unit) { return unit.convert(cache.expiresAfterWriteNanos(), TimeUnit.NANOSECONDS); @@ -3693,14 +3698,14 @@ final class BoundedVarExpiration implements VarExpiration { requireNonNull(key); requireNonNull(unit); Object lookupKey = cache.nodeFactory.newLookupKey(key); - Node node = cache.data.get(lookupKey); + Node node = cache.data.get(lookupKey); if (node == null) { return OptionalLong.empty(); } - long duration = node.getVariableTime() - cache.expirationTicker().read(); - return (duration <= 0) + long now = cache.expirationTicker().read(); + return cache.hasExpired(node, now) ? OptionalLong.empty() - : OptionalLong.of(unit.convert(duration, TimeUnit.NANOSECONDS)); + : OptionalLong.of(unit.convert(node.getVariableTime() - now, TimeUnit.NANOSECONDS)); } @Override public void setExpiresAfter(K key, long duration, TimeUnit unit) { requireNonNull(key); @@ -3713,6 +3718,9 @@ final class BoundedVarExpiration implements VarExpiration { long durationNanos = TimeUnit.NANOSECONDS.convert(duration, unit); synchronized (node) { now = cache.expirationTicker().read(); + if (cache.hasExpired(node, now)) { + return; + } node.setVariableTime(now + Math.min(durationNanos, MAXIMUM_EXPIRY)); } cache.afterRead(node, now, /* recordHit */ false); @@ -3816,14 +3824,14 @@ final class BoundedRefreshAfterWrite implements FixedRefresh { requireNonNull(key); requireNonNull(unit); Object lookupKey = cache.nodeFactory.newLookupKey(key); - Node node = cache.data.get(lookupKey); + Node node = cache.data.get(lookupKey); if (node == null) { return OptionalLong.empty(); } - long age = cache.expirationTicker().read() - node.getWriteTime(); - return (age > cache.refreshAfterWriteNanos()) + long now = cache.expirationTicker().read(); + return cache.hasExpired(node, now) ? OptionalLong.empty() - : OptionalLong.of(unit.convert(age, TimeUnit.NANOSECONDS)); + : OptionalLong.of(unit.convert(now - node.getWriteTime(), TimeUnit.NANOSECONDS)); } @Override public long getRefreshesAfter(TimeUnit unit) { return unit.convert(cache.refreshAfterWriteNanos(), TimeUnit.NANOSECONDS); diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Policy.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Policy.java index 9d956863ea..85400ad687 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Policy.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Policy.java @@ -219,8 +219,6 @@ interface FixedExpiration { * An expiration policy uses the age to determine if an entry is fresh or stale by comparing it * to the freshness lifetime. This is calculated as {@code fresh = freshnessLifetime > age} * where {@code freshnessLifetime = expires - currentTime}. - *

- * This method is scheduled for removal in version 3.0.0. * * @param key the key for the entry being queried * @param unit the unit that {@code age} is expressed in @@ -251,8 +249,6 @@ default Optional ageOf(K key) { * to elapsing this time bound. An entry is considered fresh if its age is less than this * duration, and stale otherwise. The expiration policy determines when the entry's age is * reset. - *

- * This method is scheduled for removal in version 3.0.0. * * @param unit the unit that duration is expressed in * @return the length of time after which an entry should be automatically removed @@ -275,8 +271,6 @@ default Duration getExpiresAfter() { /** * Specifies that each entry should be automatically removed from the cache once a fixed * duration has elapsed. The expiration policy determines when the entry's age is reset. - *

- * This method is scheduled for removal in version 3.0.0. * * @param duration the length of time after which an entry should be automatically removed * @param unit the unit that {@code duration} is expressed in @@ -334,8 +328,6 @@ interface VarExpiration { /** * Returns the duration until the entry should be automatically removed. The expiration policy * determines when the entry's duration is reset. - *

- * This method is scheduled for removal in version 3.0.0. * * @param key the key for the entry being queried * @param unit the unit that {@code age} is expressed in @@ -386,8 +378,6 @@ default void setExpiresAfter(K key, Duration duration) { * already associated with a value. This method differs from {@link Map#putIfAbsent} by * substituting the configured {@link Expiry} with the specified write duration, has no effect * on the duration if the entry was present, and returns the success rather than a value. - *

- * This method is scheduled for removal in version 3.0.0. * * @param key the key with which the specified value is to be associated * @param value value to be associated with the specified key @@ -421,8 +411,6 @@ default void setExpiresAfter(K key, Duration duration) { * contained a value associated with the {@code key}, the old value is replaced by the new * {@code value}. This method differs from {@link Cache#put} by substituting the configured * {@link Expiry} with the specified write duration. - *

- * This method is scheduled for removal in version 3.0.0. * * @param key the key with which the specified value is to be associated * @param value value to be associated with the specified key @@ -491,13 +479,11 @@ interface FixedRefresh { /** * Returns the age of the entry based on the refresh policy. The entry's age is the cache's - * estimate of the amount of time since the entry's refresh time was last reset. + * estimate of the amount of time since the entry's refresh period was last reset. *

- * An expiration policy uses the age to determine if an entry is fresh or stale by comparing it + * A refresh policy uses the age to determine if an entry is fresh or stale by comparing it * to the freshness lifetime. This is calculated as {@code fresh = freshnessLifetime > age} * where {@code freshnessLifetime = expires - currentTime}. - *

- * This method is scheduled for removal in version 3.0.0. * * @param key the key for the entry being queried * @param unit the unit that {@code age} is expressed in @@ -506,10 +492,10 @@ interface FixedRefresh { OptionalLong ageOf(K key, TimeUnit unit); /** - * Returns the age of the entry based on the expiration policy. The entry's age is the cache's - * estimate of the amount of time since the entry's expiration was last reset. + * Returns the age of the entry based on the refresh policy. The entry's age is the cache's + * estimate of the amount of time since the entry's refresh period was last reset. *

- * An expiration policy uses the age to determine if an entry is fresh or stale by comparing it + * A refresh policy uses the age to determine if an entry is fresh or stale by comparing it * to the freshness lifetime. This is calculated as {@code fresh = freshnessLifetime > age} * where {@code freshnessLifetime = expires - currentTime}. * @@ -528,8 +514,6 @@ default Optional ageOf(K key) { * to elapsing this time bound. An entry is considered fresh if its age is less than this * duration, and stale otherwise. The refresh policy determines when the entry's age is * reset. - *

- * This method is scheduled for removal in version 3.0.0. * * @param unit the unit that duration is expressed in * @return the length of time after which an entry is eligible to be reloaded @@ -552,8 +536,6 @@ default Duration getRefreshesAfter() { /** * Specifies that each entry should be eligible for reloading once a fixed duration has elapsed. * The refresh policy determines when the entry's age is reset. - *

- * This method is scheduled for removal in version 3.0.0. * * @param duration the length of time after which an entry is eligible to be reloaded * @param unit the unit that {@code duration} is expressed in diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java index 6277906e97..fe12015b54 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java @@ -632,7 +632,7 @@ public void evict_resurrect_expireAfterVar(Cache cache, CacheContext c await().untilTrue(started); var threadState = EnumSet.of(State.BLOCKED, State.WAITING); await().until(() -> threadState.contains(evictor.get().getState())); - cache.policy().expireVariably().get().setExpiresAfter(key, Duration.ofDays(1)); + node.setVariableTime(context.ticker().read() + TimeUnit.DAYS.toNanos(1)); } await().untilTrue(done); diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterAccessTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterAccessTest.java index d30c8adb7b..909efda8d5 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterAccessTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterAccessTest.java @@ -286,6 +286,26 @@ public void ageOf_duration(CacheContext context, assertThat(expireAfterAccess.ageOf(context.firstKey()), is(Optional.empty())); } + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, expireAfterAccess = Expire.ONE_MINUTE) + public void ageOf_absent(CacheContext context, + @ExpireAfterAccess FixedExpiration expireAfterAccess) { + assertThat(expireAfterAccess.ageOf( + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, + expiry = { CacheExpiry.DISABLED, CacheExpiry.CREATE, CacheExpiry.WRITE, CacheExpiry.ACCESS }, + expireAfterAccess = Expire.ONE_MINUTE, population = Population.EMPTY) + public void ageOf_expired(Cache cache, CacheContext context, + @ExpireAfterAccess FixedExpiration expireAfterAccess) { + cache.put(context.absentKey(), context.absentValue()); + context.ticker().advance(2, TimeUnit.MINUTES); + assertThat(expireAfterAccess.ageOf( + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); + } + /* --------------- Policy: oldest --------------- */ @CacheSpec(implementation = Implementation.Caffeine, expireAfterAccess = Expire.ONE_MINUTE) diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterVarTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterVarTest.java index d1708e6cb1..9b6bea2c49 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterVarTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterVarTest.java @@ -15,6 +15,9 @@ */ package com.github.benmanes.caffeine.cache; +import static com.github.benmanes.caffeine.cache.testing.CacheSpec.Expiration.AFTER_ACCESS; +import static com.github.benmanes.caffeine.cache.testing.CacheSpec.Expiration.AFTER_WRITE; +import static com.github.benmanes.caffeine.cache.testing.CacheSpec.Expiration.VARIABLE; import static com.github.benmanes.caffeine.cache.testing.RemovalListenerVerifier.verifyListeners; import static com.github.benmanes.caffeine.testing.Awaits.await; import static com.github.benmanes.caffeine.testing.IsEmptyMap.emptyMap; @@ -22,6 +25,7 @@ import static java.util.concurrent.TimeUnit.NANOSECONDS; import static java.util.function.Function.identity; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -30,6 +34,7 @@ import static org.hamcrest.Matchers.sameInstance; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import java.time.Duration; @@ -40,6 +45,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import org.mockito.Mockito; import org.testng.annotations.Listeners; import org.testng.annotations.Test; @@ -388,6 +394,30 @@ public void getExpiresAfter_duration(Cache cache, is(Optional.of(Duration.ofMinutes(1)))); } + @SuppressWarnings("unchecked") + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, population = Population.FULL, + expiry = CacheExpiry.MOCKITO, expiryTime = Expire.ONE_MINUTE) + public void getExpiresAfter_absent(CacheContext context, VarExpiration expireAfterVar) { + Mockito.reset(context.expiry()); + assertThat(expireAfterVar.getExpiresAfter( + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); + verifyNoInteractions(context.expiry()); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, refreshAfterWrite = Expire.ONE_MINUTE, + expiry = { CacheExpiry.CREATE, CacheExpiry.WRITE, CacheExpiry.ACCESS }, + mustExpireWithAnyOf = { AFTER_ACCESS, AFTER_WRITE, VARIABLE }, expiryTime = Expire.ONE_MINUTE, + population = Population.EMPTY) + public void getExpiresAfter_expired(Cache cache, CacheContext context, + VarExpiration expireAfterVar) { + cache.put(context.absentKey(), context.absentValue()); + context.ticker().advance(2, TimeUnit.MINUTES); + assertThat(expireAfterVar.getExpiresAfter( + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); + } + @Test(dataProvider = "caches") @CacheSpec(implementation = Implementation.Caffeine, population = Population.FULL, expiry = CacheExpiry.MOCKITO, expiryTime = Expire.ONE_MINUTE) @@ -423,6 +453,32 @@ public void setExpiresAfter_duration(Cache cache, assertThat(cache.estimatedSize(), is(1L)); } + @SuppressWarnings("unchecked") + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, population = Population.FULL, + expiry = CacheExpiry.MOCKITO, expiryTime = Expire.ONE_MINUTE) + public void setExpiresAfter_absent(Cache cache, + CacheContext context, VarExpiration expireAfterVar) { + expireAfterVar.setExpiresAfter(context.absentKey(), 1, TimeUnit.SECONDS); + context.ticker().advance(30, TimeUnit.SECONDS); + cache.cleanUp(); + assertThat(cache.asMap(), is(equalTo(context.original()))); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, refreshAfterWrite = Expire.ONE_MINUTE, + expiry = { CacheExpiry.CREATE, CacheExpiry.WRITE, CacheExpiry.ACCESS }, + mustExpireWithAnyOf = { AFTER_ACCESS, AFTER_WRITE, VARIABLE }, expiryTime = Expire.ONE_MINUTE, + population = Population.EMPTY) + public void setExpiresAfter_expired(Cache cache, CacheContext context, + VarExpiration expireAfterVar) { + cache.put(context.absentKey(), context.absentValue()); + context.ticker().advance(2, TimeUnit.MINUTES); + expireAfterVar.setExpiresAfter(context.absentKey(), 1, TimeUnit.MINUTES); + cache.cleanUp(); + assertThat(cache.asMap(), is(anEmptyMap())); + } + /* --------------- Policy: putIfAbsent --------------- */ @CheckNoStats diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterWriteTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterWriteTest.java index fca2c37e67..4a7d13f254 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterWriteTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterWriteTest.java @@ -30,7 +30,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -270,14 +269,33 @@ public void ageOf(CacheContext context, population = { Population.SINGLETON, Population.PARTIAL, Population.FULL }) public void ageOf_duration(CacheContext context, @ExpireAfterWrite FixedExpiration expireAfterWrite) { - assertThat(expireAfterWrite.ageOf(context.firstKey()), is(Optional.of(Duration.ZERO))); + assertThat(expireAfterWrite.ageOf(context.firstKey()).get().toSeconds(), is(0L)); context.ticker().advance(30, TimeUnit.SECONDS); - assertThat(expireAfterWrite.ageOf(context.firstKey()), - is(Optional.of(Duration.ofSeconds(30L)))); + assertThat(expireAfterWrite.ageOf(context.firstKey()).get().toSeconds(), is(30L)); context.ticker().advance(45, TimeUnit.SECONDS); assertThat(expireAfterWrite.ageOf(context.firstKey()).isPresent(), is(false)); } + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, expireAfterWrite = Expire.ONE_MINUTE) + public void ageOf_absent(CacheContext context, + @ExpireAfterWrite FixedExpiration expireAfterWrite) { + assertThat(expireAfterWrite.ageOf( + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, + expiry = { CacheExpiry.DISABLED, CacheExpiry.CREATE, CacheExpiry.WRITE, CacheExpiry.ACCESS }, + expireAfterWrite = Expire.ONE_MINUTE, population = Population.EMPTY) + public void ageOf_expired(Cache cache, CacheContext context, + @ExpireAfterWrite FixedExpiration expireAfterWrite) { + cache.put(context.absentKey(), context.absentValue()); + context.ticker().advance(2, TimeUnit.MINUTES); + assertThat(expireAfterWrite.ageOf( + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); + } + /* --------------- Policy: oldest --------------- */ @CacheSpec(implementation = Implementation.Caffeine, expireAfterWrite = Expire.ONE_MINUTE) diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java index 47382da483..9608d97fbf 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java @@ -15,6 +15,9 @@ */ package com.github.benmanes.caffeine.cache; +import static com.github.benmanes.caffeine.cache.testing.CacheSpec.Expiration.AFTER_ACCESS; +import static com.github.benmanes.caffeine.cache.testing.CacheSpec.Expiration.AFTER_WRITE; +import static com.github.benmanes.caffeine.cache.testing.CacheSpec.Expiration.VARIABLE; import static com.github.benmanes.caffeine.cache.testing.RemovalListenerVerifier.verifyRemovalListener; import static com.github.benmanes.caffeine.cache.testing.StatsVerifier.verifyStats; import static com.github.benmanes.caffeine.testing.Awaits.await; @@ -33,8 +36,11 @@ import java.time.Duration; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import org.testng.annotations.Listeners; @@ -45,6 +51,7 @@ 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.Expire; import com.github.benmanes.caffeine.cache.testing.CacheSpec.Implementation; @@ -56,6 +63,7 @@ import com.github.benmanes.caffeine.cache.testing.RefreshAfterWrite; import com.github.benmanes.caffeine.cache.testing.RemovalNotification; import com.github.benmanes.caffeine.cache.testing.TrackingExecutor; +import com.github.benmanes.caffeine.testing.ConcurrentTestHarness; import com.github.benmanes.caffeine.testing.Int; /** @@ -68,6 +76,73 @@ @Test(dataProviderClass = CacheProvider.class) public final class RefreshAfterWriteTest { + /* --------------- refreshIfNeeded --------------- */ + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY, + refreshAfterWrite = Expire.ONE_MINUTE, executor = CacheExecutor.THREADED) + public void refreshIfNeeded_nonblocking(CacheContext context) { + Int key = context.absentKey(); + Int original = Int.valueOf(1); + Int refresh1 = original.add(1); + Int refresh2 = refresh1.add(1); + var duration = Duration.ofMinutes(2); + + var refresh = new AtomicBoolean(); + var reloads = new AtomicInteger(); + var cache = context.build(new CacheLoader() { + @Override public Int load(Int key) { + throw new IllegalStateException(); + } + @Override + public CompletableFuture asyncReload(Int key, Int oldValue, Executor executor) { + reloads.incrementAndGet(); + await().untilTrue(refresh); + return CompletableFuture.completedFuture(oldValue.add(1)); + } + }); + cache.put(key, original); + context.ticker().advance(duration); + ConcurrentTestHarness.execute(() -> cache.get(key)); + await().untilAtomic(reloads, is(1)); + + assertThat(cache.get(key), is(original)); + refresh.set(true); + + await().until(() -> cache.get(key), is(refresh1)); + assertThat(reloads.get(), is(1)); + + context.ticker().advance(duration); + assertThat(cache.get(key), is(refresh1)); + await().untilAtomic(reloads, is(2)); + await().until(() -> cache.get(key), is(refresh2)); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY, + refreshAfterWrite = Expire.ONE_MINUTE, executor = CacheExecutor.THREADED) + public void refreshIfNeeded_failure(CacheContext context) { + Int key = context.absentKey(); + var reloads = new AtomicInteger(); + var cache = context.build(new CacheLoader() { + @Override public Int load(Int key) { + throw new IllegalStateException(); + } + @Override + public CompletableFuture asyncReload(Int key, Int oldValue, Executor executor) { + reloads.incrementAndGet(); + throw new IllegalStateException(); + } + }); + cache.put(key, key); + + for (int i = 0; i < 5; i++) { + context.ticker().advance(2, TimeUnit.MINUTES); + cache.get(key); + await().untilAtomic(reloads, is(i + 1)); + } + } + /* --------------- getIfPresent --------------- */ @Test(dataProvider = "caches") @@ -433,7 +508,40 @@ public void ageOf(CacheContext context, context.ticker().advance(30, TimeUnit.SECONDS); assertThat(refreshAfterWrite.ageOf(context.firstKey(), TimeUnit.SECONDS).getAsLong(), is(30L)); context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(refreshAfterWrite.ageOf(context.firstKey(), TimeUnit.SECONDS).getAsLong(), is(75L)); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, refreshAfterWrite = Expire.ONE_MINUTE, + population = { Population.SINGLETON, Population.PARTIAL, Population.FULL }) + public void ageOf_duration(CacheContext context, + @RefreshAfterWrite FixedRefresh refreshAfterWrite) { + assertThat(refreshAfterWrite.ageOf(context.firstKey()).get().toSeconds(), is(0L)); + context.ticker().advance(30, TimeUnit.SECONDS); + assertThat(refreshAfterWrite.ageOf(context.firstKey()).get().toSeconds(), is(30L)); + context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(refreshAfterWrite.ageOf(context.firstKey()).get().toSeconds(), is(75L)); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, refreshAfterWrite = Expire.ONE_MINUTE) + public void ageOf_absent(CacheContext context, + @RefreshAfterWrite FixedRefresh refreshAfterWrite) { + assertThat(refreshAfterWrite.ageOf( + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); + } + + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, refreshAfterWrite = Expire.ONE_MINUTE, + expiry = { CacheExpiry.DISABLED, CacheExpiry.CREATE, CacheExpiry.WRITE, CacheExpiry.ACCESS }, + mustExpireWithAnyOf = { AFTER_ACCESS, AFTER_WRITE, VARIABLE }, expiryTime = Expire.ONE_MINUTE, + expireAfterAccess = {Expire.DISABLED, Expire.ONE_MINUTE}, + expireAfterWrite = {Expire.DISABLED, Expire.ONE_MINUTE}, population = Population.EMPTY) + public void ageOf_expired(Cache cache, CacheContext context, + @RefreshAfterWrite FixedRefresh refreshAfterWrite) { + cache.put(context.absentKey(), context.absentValue()); + context.ticker().advance(2, TimeUnit.MINUTES); assertThat(refreshAfterWrite.ageOf( - context.firstKey(), TimeUnit.SECONDS).isPresent(), is(false)); + context.absentKey(), TimeUnit.SECONDS).isPresent(), is(false)); } } diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index f296b8936b..01b27054cc 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -27,12 +27,12 @@ ext { versions = [ akka: '2.6.15', cache2k: '2.1.2.Alpha', - checkerFramework: '3.15.0', + checkerFramework: '3.16.0', coherence: '20.06', - commonsCompress: '1.20', + commonsCompress: '1.21', commonsLang3: '3.12.0', commonsMath3: '3.6.1', - commonsIo: '2.10.0', + commonsIo: '2.11.0', concurrentlinkedhashmap: '1.4.2', config: '1.4.1', ehcache3: '3.9.4',