diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Async.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Async.java index 3af148844c..40cba95913 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Async.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Async.java @@ -15,6 +15,7 @@ */ package com.github.benmanes.caffeine.cache; +import static com.github.benmanes.caffeine.cache.BoundedLocalCache.MAXIMUM_EXPIRY; import static java.util.Objects.requireNonNull; import java.io.Serializable; @@ -31,7 +32,7 @@ * @author ben.manes@gmail.com (Ben Manes) */ final class Async { - static final long MAXIMUM_EXPIRY = (Long.MAX_VALUE >> 1); // 150 years + static final long ASYNC_EXPIRY = (Long.MAX_VALUE >> 1) + (Long.MAX_VALUE >> 2); // 220 years private Async() {} @@ -118,7 +119,7 @@ Object writeReplace() { /** * An expiry for asynchronous computations. When the value is being loaded this expiry returns - * {@code Long.MAX_VALUE} to indicate that the entry should not be evicted due to an expiry + * {@code ASYNC_EXPIRY} to indicate that the entry should not be evicted due to an expiry * constraint. If the value is computed successfully the entry must be reinserted so that the * expiration is updated and the expiration timeouts reflect the value once present. The value * maximum range is reserved to coordinate the asynchronous life cycle. @@ -138,7 +139,7 @@ public long expireAfterCreate(K key, CompletableFuture future, long currentTi long duration = delegate.expireAfterCreate(key, future.join(), currentTime); return Math.min(duration, MAXIMUM_EXPIRY); } - return Long.MAX_VALUE; + return ASYNC_EXPIRY; } @Override @@ -150,7 +151,7 @@ public long expireAfterUpdate(K key, CompletableFuture future, : delegate.expireAfterUpdate(key, future.join(), currentTime, currentDuration); return Math.min(duration, MAXIMUM_EXPIRY); } - return Long.MAX_VALUE; + return ASYNC_EXPIRY; } @Override @@ -160,7 +161,7 @@ public long expireAfterRead(K key, CompletableFuture future, long duration = delegate.expireAfterRead(key, future.join(), currentTime, currentDuration); return Math.min(duration, MAXIMUM_EXPIRY); } - return Long.MAX_VALUE; + return ASYNC_EXPIRY; } Object writeReplace() { 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 fbedc3c0b9..dc34ca5c3d 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 @@ -15,6 +15,7 @@ */ package com.github.benmanes.caffeine.cache; +import static com.github.benmanes.caffeine.cache.Async.ASYNC_EXPIRY; import static com.github.benmanes.caffeine.cache.Caffeine.requireArgument; import static com.github.benmanes.caffeine.cache.Caffeine.requireState; import static com.github.benmanes.caffeine.cache.Node.EDEN; @@ -154,6 +155,8 @@ abstract class BoundedLocalCache extends BLCHeader.DrainStatusRef static final double PERCENT_MAIN_PROTECTED = 0.80d; /** The maximum time window between entry updates before the expiration must be reordered. */ static final long EXPIRE_WRITE_TOLERANCE = TimeUnit.SECONDS.toNanos(1); + /** The maximum duration before an entry expires. */ + static final long MAXIMUM_EXPIRY = (Long.MAX_VALUE >> 1); // 150 years final ConcurrentHashMap> data; @Nullable final CacheLoader cacheLoader; @@ -724,13 +727,11 @@ void expireVariableEntries(long now) { } /** Returns if the entry has expired. */ + @SuppressWarnings("ShortCircuitBoolean") boolean hasExpired(Node node, long now) { - if (isComputingAsync(node)) { - return false; - } return (expiresAfterAccess() && (now - node.getAccessTime() >= expiresAfterAccessNanos())) - || (expiresAfterWrite() && (now - node.getWriteTime() >= expiresAfterWriteNanos())) - || (expiresVariable() && (now - node.getVariableTime() >= 0)); + | (expiresAfterWrite() && (now - node.getWriteTime() >= expiresAfterWriteNanos())) + | (expiresVariable() && (now - node.getVariableTime() >= 0)); } /** @@ -866,7 +867,7 @@ void refreshIfNeeded(Node node, long now) { K key; V oldValue; long oldWriteTime = node.getWriteTime(); - long refreshWriteTime = (now + Async.MAXIMUM_EXPIRY); + long refreshWriteTime = (now + ASYNC_EXPIRY); if (((now - oldWriteTime) > refreshAfterWriteNanos()) && ((key = node.getKey()) != null) && ((oldValue = node.getValue()) != null) && node.casWriteTime(oldWriteTime, refreshWriteTime)) { @@ -941,7 +942,7 @@ void refreshIfNeeded(Node node, long now) { long expireAfterCreate(@Nullable K key, @Nullable V value, Expiry expiry, long now) { if (expiresVariable() && (key != null) && (value != null)) { long duration = expiry.expireAfterCreate(key, value, now); - return (now + duration); + return isAsync ? (now + duration) : (now + Math.min(duration, MAXIMUM_EXPIRY)); } return 0L; } @@ -961,7 +962,7 @@ long expireAfterUpdate(Node node, @Nullable K key, if (expiresVariable() && (key != null) && (value != null)) { long currentDuration = Math.max(1, node.getVariableTime() - now); long duration = expiry.expireAfterUpdate(key, value, now, currentDuration); - return (now + duration); + return isAsync ? (now + duration) : (now + Math.min(duration, MAXIMUM_EXPIRY)); } return 0L; } @@ -981,7 +982,7 @@ long expireAfterRead(Node node, @Nullable K key, if (expiresVariable() && (key != null) && (value != null)) { long currentDuration = Math.max(1, node.getVariableTime() - now); long duration = expiry.expireAfterRead(key, value, now, currentDuration); - return (now + duration); + return isAsync ? (now + duration) : (now + Math.min(duration, MAXIMUM_EXPIRY)); } return 0L; } @@ -1340,7 +1341,7 @@ public void run() { if (isComputingAsync(node)) { synchronized (node) { if (!Async.isReady((CompletableFuture) node.getValue())) { - long expirationTime = expirationTicker().read() + Long.MAX_VALUE; + long expirationTime = expirationTicker().read() + ASYNC_EXPIRY; setVariableTime(node, expirationTime); setAccessTime(node, expirationTime); setWriteTime(node, expirationTime); @@ -3255,7 +3256,7 @@ final class BoundedVarExpiration implements VarExpiration { long durationNanos = TimeUnit.NANOSECONDS.convert(duration, unit); synchronized (node) { now = cache.expirationTicker().read(); - node.setVariableTime(now + durationNanos); + node.setVariableTime(now + Math.min(durationNanos, MAXIMUM_EXPIRY)); } cache.afterRead(node, now, /* recordHit */ false); } diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncTest.java index 3086aa0e03..94e7bbc938 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncTest.java @@ -15,7 +15,8 @@ */ package com.github.benmanes.caffeine.cache; -import static com.github.benmanes.caffeine.cache.Async.MAXIMUM_EXPIRY; +import static com.github.benmanes.caffeine.cache.Async.ASYNC_EXPIRY; +import static com.github.benmanes.caffeine.cache.BoundedLocalCache.MAXIMUM_EXPIRY; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -112,13 +113,13 @@ public void asyncExpiry_pending() { AsyncExpiry expiry = makeAsyncExpiry(ONE_MINUTE, ONE_MINUTE, ONE_MINUTE); CompletableFuture future = new CompletableFuture(); - assertThat(expiry.expireAfterCreate(0, future, 1L), is(Long.MAX_VALUE)); + assertThat(expiry.expireAfterCreate(0, future, 1L), is(ASYNC_EXPIRY)); verify(expiry.delegate, never()).expireAfterCreate(any(), any(), anyLong()); - assertThat(expiry.expireAfterUpdate(0, future, 1L, 2L), is(Long.MAX_VALUE)); + assertThat(expiry.expireAfterUpdate(0, future, 1L, 2L), is(ASYNC_EXPIRY)); verify(expiry.delegate, never()).expireAfterUpdate(any(), any(), anyLong(), anyLong()); - assertThat(expiry.expireAfterRead(0, future, 1L, 2L), is(Long.MAX_VALUE)); + assertThat(expiry.expireAfterRead(0, future, 1L, 2L), is(ASYNC_EXPIRY)); verify(expiry.delegate, never()).expireAfterRead(any(), any(), anyLong(), anyLong()); } 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 21ffc8af83..a6b6c72bfa 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 @@ -17,6 +17,7 @@ import static com.github.benmanes.caffeine.cache.testing.CacheWriterVerifier.verifyWriter; import static com.github.benmanes.caffeine.cache.testing.HasRemovalNotifications.hasRemovalNotifications; +import static com.github.benmanes.caffeine.testing.Awaits.await; import static com.github.benmanes.caffeine.testing.IsEmptyMap.emptyMap; import static com.github.benmanes.caffeine.testing.IsFutureValue.futureOf; import static java.util.concurrent.TimeUnit.NANOSECONDS; @@ -27,6 +28,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; @@ -42,6 +44,7 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import org.testng.annotations.Listeners; import org.testng.annotations.Test; @@ -59,6 +62,7 @@ import com.github.benmanes.caffeine.cache.testing.CacheValidationListener; import com.github.benmanes.caffeine.cache.testing.CheckNoStats; import com.github.benmanes.caffeine.cache.testing.CheckNoWriter; +import com.github.benmanes.caffeine.testing.ConcurrentTestHarness; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; @@ -72,6 +76,33 @@ @Test(dataProviderClass = CacheProvider.class) public final class ExpireAfterVarTest { + @Test(dataProvider = "caches") + @CacheSpec(expiryTime = Expire.FOREVER, + expiry = { CacheExpiry.CREATE, CacheExpiry.WRITE, CacheExpiry.ACCESS }) + public void expiry_bounds(Cache cache, CacheContext context) { + context.ticker().advance(System.nanoTime()); + AtomicBoolean running = new AtomicBoolean(); + AtomicBoolean done = new AtomicBoolean(); + Integer key = context.absentKey(); + cache.put(key, key); + + try { + ConcurrentTestHarness.execute(() -> { + while (!done.get()) { + context.ticker().advance(1, TimeUnit.MINUTES); + cache.get(key, Integer::new); + running.set(true); + } + }); + await().untilTrue(running); + cache.cleanUp(); + + assertThat(cache.get(key, Integer::new), sameInstance(key)); + } finally { + done.set(true); + } + } + /* ---------------- Create -------------- */ @Test(dataProvider = "caches") diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index c54915907b..35912cf6fc 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -28,7 +28,7 @@ ext { akka: '2.5.9', commonsCompress: '1.16.1', commonsLang3: '3.7', - config: '1.3.2', + config: '1.3.3', errorProne: '2.2.0', fastutil: '8.1.1', flipTables: '1.0.2', @@ -59,7 +59,7 @@ ext { concurrentlinkedhashmap: '1.4.2', ehcache2: '2.10.4', ehcache3: '3.4.0', - elasticSearch: '6.2.1', + elasticSearch: '6.2.2', expiringMap: '0.5.8', jackrabbit: '1.8.2', jamm: '0.3.2',