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)); }