Postpone call to get() when loading from the in-memory cache
Caffeine's get() method relies on the `ConcurrentHashMap.compute()`
java11 implementation, which locks concurrent access in [1].
The effect of this on cache-chroniclemap is that when multiple threads
try to access the in-memory cache concurrently, they result in being
BLOCKED, as documented by the `ConcurrentHashMap.compute()` method:
```
Some attempted update operations on this map by other threads
may be blocked while computation is in progress, so the
computation should be short and simple, and must not attempt to
update any other mappings of this Map.
```
This, in turn, causes high consumption of Gerrit threads, which might
eventually, make it unresponsive.
Workaround this problem by first trying to get the key _if present_ and
only calling get() in case no value could be loaded.
This works because getIfPresent(), as opposed to, get(), does not try to
`compute` entries in the cache and thus decreases the chances of threads
blocking in case of cache hits.
[1] https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/master/src/java.base/share/classes/java/util/concurrent/ConcurrentHashMap.java#L1923
Bug: Issue 15645
Change-Id: I7021d93d4011e252c5c022ffcd3eecee22820ff5
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java
index 96b75da..d69be5b 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java
@@ -59,15 +59,15 @@
@Override
public TimedValue<V> get(K key) throws ExecutionException {
- if (loadingFromSource) {
- return ((LoadingCache<K, TimedValue<V>>) loadingFromStoreCache).get(key);
- }
-
TimedValue<V> cachedValue = getIfPresent(key);
if (cachedValue != null) {
return cachedValue;
}
+ if (loadingFromSource) {
+ return ((LoadingCache<K, TimedValue<V>>) loadingFromStoreCache).get(key);
+ }
+
throw new UnsupportedOperationException(
String.format("Could not load value for %s without any loader", key));
}
