Skip to content

Commit

Permalink
JCache: add active expiration when using ExpiryPolicy (fixes #265)
Browse files Browse the repository at this point in the history
Previously, an JCache expiration was implemented as a fully lazy
wrapper and relied on size eviction to eventually discard the entry
if not accessed again. This matched the implementations by the spec
authors. However, JSR-107 doesn't offer a standard configuration for
setting a maximum size. This would allow an expiry-only cache to run
out of memory due to the lack of cleanup. The authors assumed a size
would have to be applied, and we followed their examples.

Caffeine later added its own variable expiration. This is not fully
compatible due to slight semantic issues. For example, JCache TCK
asserts that inserting an entry with an immediate expiration
duration is never actually inserted into the cache. That means
evaluating the expiration time during a cache compute and
returning a null mapping. Caffeine's logic will insert and
immediately discard. This has the same effect, except callbacks
for listener events receive the create and delete, whereas
JCache requires that both are suppressed. This causes the
integration to become difficult due to small lifecycle differences
that the TCK asserts.

Instead, we continue to use a lazy wrapper but more proactively
clean up via Caffeine's native support. This is already an option
if setting any of the native options explicitly. Now if none of
the native settings are used, the adapter will default to use
variable expiration that reads the wrapper's timestamp. This will
allow the cache maintenance to discard the entry regardless of if
a maximum size is set.
  • Loading branch information
ben-manes committed Jan 2, 2021
1 parent 1c818eb commit c991a50
Show file tree
Hide file tree
Showing 14 changed files with 205 additions and 67 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ Powering infrastructure near you:
Download from [Maven Central][maven] or depend via Gradle:

```gradle
compile 'com.github.ben-manes.caffeine:caffeine:2.8.8'
implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8'
// Optional extensions
compile 'com.github.ben-manes.caffeine:guava:2.8.8'
compile 'com.github.ben-manes.caffeine:jcache:2.8.8'
implementation 'com.github.ben-manes.caffeine:guava:2.8.8'
implementation 'com.github.ben-manes.caffeine:jcache:2.8.8'
```

See the [release notes][releases] for details of the changes.
Expand Down
8 changes: 4 additions & 4 deletions gradle/dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
ext {
versions = [
akka: '2.6.10',
cache2k: '2.0.0.Final',
cache2k: '2.1.1.Alpha',
checkerFramework: '3.8.0',
coherence: '20.06',
collision: '0.3.3',
Expand Down Expand Up @@ -57,7 +57,7 @@ ext {
nullaway: '0.7.9',
ohc: '0.6.1',
osgiComponentAnnotations: '1.4.0',
picocli: '4.5.2',
picocli: '4.6.0',
slf4j: '1.7.30',
tcache: '2.0.1',
stream: '2.9.8',
Expand All @@ -77,14 +77,14 @@ ext {
paxExam: '4.13.4',
testng: '7.3.0',
truth: '0.24',
felix: '6.0.4',
felix: '7.0.0',
felixScr: '2.1.24',
osgiUtilFunction: '1.1.0',
osgiUtilPromise: '1.1.1',
]
pluginVersions = [
bnd: '5.2.0',
checkstyle: '8.38',
checkstyle: '8.39',
coveralls: '2.8.4',
coverity: '1.0.10',
errorprone: '1.3.0',
Expand Down
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
distributionUrl=https\://services.gradle.org/distributions/gradle-6.8-rc-3-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-6.8-rc-4-bin.zip
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@
import javax.cache.configuration.CompleteConfiguration;
import javax.cache.configuration.Configuration;
import javax.cache.configuration.Factory;
import javax.cache.expiry.EternalExpiryPolicy;
import javax.cache.expiry.ExpiryPolicy;
import javax.cache.integration.CacheLoader;

import org.checkerframework.checker.nullness.qual.Nullable;

import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.Expiry;
import com.github.benmanes.caffeine.cache.Scheduler;
import com.github.benmanes.caffeine.cache.Ticker;
import com.github.benmanes.caffeine.cache.Weigher;
import com.github.benmanes.caffeine.jcache.configuration.CaffeineConfiguration;
Expand Down Expand Up @@ -128,6 +130,7 @@ private static final class Builder<K, V> {
final Ticker ticker;
final String cacheName;
final Executor executor;
final Scheduler scheduler;
final CacheManager cacheManager;
final ExpiryPolicy expiryPolicy;
final EventDispatcher<K, V> dispatcher;
Expand All @@ -143,11 +146,13 @@ private static final class Builder<K, V> {
this.statistics = new JCacheStatisticsMXBean();
this.ticker = config.getTickerFactory().create();
this.executor = config.getExecutorFactory().create();
this.scheduler = config.getSchedulerFactory().create();
this.expiryPolicy = config.getExpiryPolicyFactory().create();
this.dispatcher = new EventDispatcher<>(executor);

caffeine.ticker(ticker);
caffeine.executor(executor);
caffeine.scheduler(scheduler);
config.getCacheEntryListenerConfigurations().forEach(dispatcher::register);
}

Expand All @@ -156,16 +161,21 @@ public CacheProxy<K, V> build() {
boolean evicts = false;
evicts |= configureMaximumSize();
evicts |= configureMaximumWeight();
evicts |= configureExpireAfterWrite();
evicts |= configureExpireAfterAccess();
evicts |= configureExpireVariably();

boolean expires = false;
expires |= configureExpireAfterWrite();
expires |= configureExpireAfterAccess();
expires |= configureExpireVariably();
if (!expires) {
expires = configureJCacheExpiry();
}

if (config.isNativeStatisticsEnabled()) {
caffeine.recordStats();
}

JCacheEvictionListener<K, V> evictionListener = null;
if (evicts) {
if (evicts || expires) {
evictionListener = new JCacheEvictionListener<>(dispatcher, statistics);
caffeine.writer(evictionListener);
}
Expand Down Expand Up @@ -234,38 +244,36 @@ private boolean configureMaximumWeight() {
private boolean configureExpireAfterWrite() {
if (config.getExpireAfterWrite().isPresent()) {
caffeine.expireAfterWrite(config.getExpireAfterWrite().getAsLong(), TimeUnit.NANOSECONDS);
return true;
}
return config.getExpireAfterWrite().isPresent();
return false;
}

/** Configures the access expiration and returns if set. */
@SuppressWarnings("PreferJavaTimeOverload")
private boolean configureExpireAfterAccess() {
if (config.getExpireAfterAccess().isPresent()) {
caffeine.expireAfterAccess(config.getExpireAfterAccess().getAsLong(), TimeUnit.NANOSECONDS);
return true;
}
return config.getExpireAfterAccess().isPresent();
return false;
}

/** Configures the custom expiration and returns if set. */
private boolean configureExpireVariably() {
config.getExpiryFactory().ifPresent(factory -> {
Expiry<K, V> expiry = factory.create();
caffeine.expireAfter(new Expiry<K, Expirable<V>>() {
@Override public long expireAfterCreate(K key, Expirable<V> expirable, long currentTime) {
return expiry.expireAfterCreate(key, expirable.get(), currentTime);
}
@Override public long expireAfterUpdate(K key, Expirable<V> expirable,
long currentTime, long currentDuration) {
return expiry.expireAfterUpdate(key, expirable.get(), currentTime, currentDuration);
}
@Override public long expireAfterRead(K key, Expirable<V> expirable,
long currentTime, long currentDuration) {
return expiry.expireAfterRead(key, expirable.get(), currentTime, currentDuration);
}
});
});
return config.getExpireAfterWrite().isPresent();
if (config.getExpiryFactory().isPresent()) {
caffeine.expireAfter(new ExpiryAdapter<>(config.getExpiryFactory().get().create()));
return true;
}
return false;
}

private boolean configureJCacheExpiry() {
if (!(expiryPolicy instanceof EternalExpiryPolicy)) {
caffeine.expireAfter(new ExpirableToExpiry<>(ticker));
return true;
}
return false;
}

@SuppressWarnings("PreferJavaTimeOverload")
Expand All @@ -275,4 +283,50 @@ private void configureRefreshAfterWrite() {
}
}
}

private static final class ExpiryAdapter<K, V> implements Expiry<K, Expirable<V>> {
private final Expiry<K, V> expiry;

public ExpiryAdapter(Expiry<K, V> expiry) {
this.expiry = requireNonNull(expiry);
}
@Override public long expireAfterCreate(K key, Expirable<V> expirable, long currentTime) {
return expiry.expireAfterCreate(key, expirable.get(), currentTime);
}
@Override public long expireAfterUpdate(K key, Expirable<V> expirable,
long currentTime, long currentDuration) {
return expiry.expireAfterUpdate(key, expirable.get(), currentTime, currentDuration);
}
@Override public long expireAfterRead(K key, Expirable<V> expirable,
long currentTime, long currentDuration) {
return expiry.expireAfterRead(key, expirable.get(), currentTime, currentDuration);
}
}

private static final class ExpirableToExpiry<K, V> implements Expiry<K, Expirable<V>> {
private final Ticker ticker;

public ExpirableToExpiry(Ticker ticker) {
this.ticker = requireNonNull(ticker);
}
@Override public long expireAfterCreate(K key, Expirable<V> expirable, long currentTime) {
return toNanos(expirable);
}
@Override public long expireAfterUpdate(K key, Expirable<V> expirable,
long currentTime, long currentDuration) {
return toNanos(expirable);
}
@Override public long expireAfterRead(K key, Expirable<V> expirable,
long currentTime, long currentDuration) {
return toNanos(expirable);
}
private long toNanos(Expirable<V> expirable) {
if (expirable.getExpireTimeMS() == 0L) {
return -1L;
} else if (expirable.isEternal()) {
return Long.MAX_VALUE;
}
return TimeUnit.MILLISECONDS.toNanos(expirable.getExpireTimeMS()) - ticker.read();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public boolean containsKey(K key) {
start = millis = 0L;
}

setAccessExpirationTime(expirable, millis);
setAccessExpirationTime(key, expirable, millis);
V value = copyValue(expirable);
if (statsEnabled) {
statistics.recordHits(1L);
Expand Down Expand Up @@ -225,7 +225,7 @@ protected Map<K, Expirable<V>> getAndFilterExpiredEntries(
return true;
}
if (updateAccessTime) {
setAccessExpirationTime(entry.getValue(), millis[0]);
setAccessExpirationTime(entry.getKey(), entry.getValue(), millis[0]);
}
return false;
});
Expand Down Expand Up @@ -530,7 +530,7 @@ public boolean remove(K key, V oldValue) {
removed[0] = true;
return null;
}
setAccessExpirationTime(expirable, millis);
setAccessExpirationTime(key, expirable, millis);
return expirable;
});
dispatcher.awaitSynchronous();
Expand Down Expand Up @@ -606,7 +606,7 @@ public boolean replace(K key, V oldValue, V newValue) {
replaced[0] = true;
} else {
result = expirable;
setAccessExpirationTime(expirable, millis);
setAccessExpirationTime(key, expirable, millis);
}
return result;
});
Expand Down Expand Up @@ -816,7 +816,7 @@ public CaffeineConfiguration<K, V> getConfiguration() {
}
return expirable;
case READ: {
setAccessExpirationTime(expirable, 0L);
setAccessExpirationTime(entry.getKey(), expirable, 0L);
return expirable;
}
case CREATED:
Expand Down Expand Up @@ -1112,10 +1112,11 @@ protected static long nanosToMillis(long nanos) {
/**
* Sets the access expiration time.
*
* @param key the entry's key
* @param expirable the entry that was operated on
* @param currentTimeMS the current time, or 0 if not read yet
*/
protected final void setAccessExpirationTime(Expirable<?> expirable, long currentTimeMS) {
protected final void setAccessExpirationTime(K key, Expirable<?> expirable, long currentTimeMS) {
try {
Duration duration = expiry.getExpiryForAccess();
if (duration == null) {
Expand All @@ -1131,6 +1132,9 @@ protected final void setAccessExpirationTime(Expirable<?> expirable, long curren
long expireTimeMS = duration.getAdjustedTime(currentTimeMS);
expirable.setExpireTimeMS(expireTimeMS);
}
cache.policy().expireVariably().ifPresent(policy -> {
policy.setExpiresAfter(key, duration.getDurationAmount(), duration.getTimeUnit());
});
} catch (Exception e) {
logger.log(Level.WARNING, "Failed to set the entry's expiration time", e);
}
Expand Down Expand Up @@ -1172,7 +1176,7 @@ public boolean hasNext() {
Map.Entry<K, Expirable<V>> entry = delegate.next();
long millis = entry.getValue().isEternal() ? 0L : currentTimeMillis();
if (!entry.getValue().hasExpired(millis)) {
setAccessExpirationTime(entry.getValue(), millis);
setAccessExpirationTime(entry.getKey(), entry.getValue(), millis);
cursor = entry;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@
import java.util.concurrent.Executor;
import java.util.stream.Collectors;

import org.checkerframework.checker.nullness.qual.Nullable;
import javax.cache.Cache;
import javax.cache.CacheException;
import javax.cache.CacheManager;
import javax.cache.expiry.ExpiryPolicy;
import javax.cache.integration.CacheLoader;
import javax.cache.integration.CompletionListener;

import org.checkerframework.checker.nullness.qual.Nullable;

import com.github.benmanes.caffeine.cache.LoadingCache;
import com.github.benmanes.caffeine.cache.Ticker;
import com.github.benmanes.caffeine.jcache.configuration.CaffeineConfiguration;
Expand Down Expand Up @@ -104,7 +105,7 @@ public LoadingCacheProxy(String name, Executor executor, CacheManager cacheManag

V value = null;
if (expirable != null) {
setAccessExpirationTime(expirable, millis);
setAccessExpirationTime(key, expirable, millis);
value = copyValue(expirable);
}
if (statsEnabled) {
Expand Down
Loading

0 comments on commit c991a50

Please sign in to comment.