Replace guava caches with caffeine
Replacing Guava caches with Caffeine reduces the chances of having the
deadlocks and improves the cache performance.
This was already attempted in: I8d2b17a94d0, but got reverted in:
If65560b4a9b due to recursion in PatchListLoader. This recursion issue
is present on current master. While this change replaces all caches
with Caffeine backend, the follow-up change in this series will switch
back to using Guava backend for PatchListCache implementation.
For seamless integration, the caffeine-guava adapter library is used.
Given that the final artifact for the adapter is also called guava,
there is only the version number that differentiates that artifact from
the guava library itself so that we have a danger for naming collision.
To avoid potential naming collision risk, rename the library name to
caffeine-guava.jar during the fetch from Maven Central.
Alternatives considered is not to use the caffeine-guava adapter
library. But then the Cache and LoadingCache classes and friends would
change the package name from com.google.common.cache package to
com.github.benmanes.caffeine.cache package and this change would also
affect some gerrit plugins and thus considered to be a quite intrusive
change. Still we can consider to do this change in one of the future
gerrit releases.
Bug: Issue 7645
Bug: Issue 11484
Change-Id: I6af4c15d6c15f438becd62409b7d233c309be8de
(cherry picked from commit 0050a9b5f61da67a9f33fda0bb170f8b1df6080a)
diff --git a/WORKSPACE b/WORKSPACE
index 9b17514..cc2435a 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -292,6 +292,31 @@
sha1 = GUAVA_BIN_SHA1,
)
+CAFFEINE_VERS = "2.8.0"
+
+maven_jar(
+ name = "caffeine",
+ artifact = "com.github.ben-manes.caffeine:caffeine:" + CAFFEINE_VERS,
+ sha1 = "6000774d7f8412ced005a704188ced78beeed2bb",
+)
+
+# TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential
+# naming collision between caffeine guava adapater and guava library itself.
+# Remove this renaming procedure, once this upstream issue is fixed:
+# https://github.com/ben-manes/caffeine/issues/364.
+http_file(
+ name = "caffeine-guava-renamed",
+ downloaded_file_path = "caffeine-guava-" + CAFFEINE_VERS + ".jar",
+ sha256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1",
+ urls = [
+ "https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/guava/" +
+ CAFFEINE_VERS +
+ "/guava-" +
+ CAFFEINE_VERS +
+ ".jar",
+ ],
+)
+
maven_jar(
name = "jsch",
artifact = "com.jcraft:jsch:0.1.54",
diff --git a/java/com/google/gerrit/server/cache/mem/BUILD b/java/com/google/gerrit/server/cache/mem/BUILD
index eb0695e..bc5b66a 100644
--- a/java/com/google/gerrit/server/cache/mem/BUILD
+++ b/java/com/google/gerrit/server/cache/mem/BUILD
@@ -8,6 +8,8 @@
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
+ "//lib:caffeine",
+ "//lib:caffeine-guava",
"//lib:guava",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
index ad1d396..0a42ce1 100644
--- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
@@ -17,12 +17,15 @@
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalListener;
+import com.github.benmanes.caffeine.cache.Weigher;
+import com.github.benmanes.caffeine.guava.CaffeinatedGuava;
import com.google.common.base.Strings;
import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
-import com.google.common.cache.Weigher;
+import com.google.common.cache.RemovalNotification;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.server.cache.CacheDef;
import com.google.gerrit.server.cache.ForwardingRemovalListener;
@@ -47,28 +50,21 @@
@Override
public <K, V> Cache<K, V> build(CacheDef<K, V> def) {
- return create(def).build();
+ return CaffeinatedGuava.build(create(def));
}
@Override
public <K, V> LoadingCache<K, V> build(CacheDef<K, V> def, CacheLoader<K, V> loader) {
- return create(def).build(loader);
+ return CaffeinatedGuava.build(create(def), loader);
}
- @SuppressWarnings("unchecked")
- private <K, V> CacheBuilder<K, V> create(CacheDef<K, V> def) {
- CacheBuilder<K, V> builder = newCacheBuilder();
+ private <K, V> Caffeine<K, V> create(CacheDef<K, V> def) {
+ Caffeine<K, V> builder = newCacheBuilder();
builder.recordStats();
builder.maximumWeight(
cfg.getLong("cache", def.configKey(), "memoryLimit", def.maximumWeight()));
-
- builder = builder.removalListener(forwardingRemovalListenerFactory.create(def.name()));
-
- Weigher<K, V> weigher = def.weigher();
- if (weigher == null) {
- weigher = unitWeight();
- }
- builder.weigher(weigher);
+ builder = builder.removalListener(newRemovalListener(def.name()));
+ builder.weigher(newWeigher(def.weigher()));
Duration expireAfterWrite = def.expireAfterWrite();
if (has(def.configKey(), "maxAge")) {
@@ -107,16 +103,22 @@
}
@SuppressWarnings("unchecked")
- private static <K, V> CacheBuilder<K, V> newCacheBuilder() {
- return (CacheBuilder<K, V>) CacheBuilder.newBuilder();
+ private static <K, V> Caffeine<K, V> newCacheBuilder() {
+ return (Caffeine<K, V>) Caffeine.newBuilder();
}
- private static <K, V> Weigher<K, V> unitWeight() {
- return new Weigher<K, V>() {
- @Override
- public int weigh(K key, V value) {
- return 1;
- }
- };
+ @SuppressWarnings("unchecked")
+ private <V, K> RemovalListener<K, V> newRemovalListener(String cacheName) {
+ return (k, v, cause) ->
+ forwardingRemovalListenerFactory
+ .create(cacheName)
+ .onRemoval(
+ RemovalNotification.create(
+ k, v, com.google.common.cache.RemovalCause.valueOf(cause.name())));
+ }
+
+ private static <K, V> Weigher<K, V> newWeigher(
+ com.google.common.cache.Weigher<K, V> guavaWeigher) {
+ return guavaWeigher == null ? Weigher.singletonWeigher() : (k, v) -> guavaWeigher.weigh(k, v);
}
}
diff --git a/lib/BUILD b/lib/BUILD
index 0474171..77b2fcd 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -1,4 +1,4 @@
-load("@rules_java//java:defs.bzl", "java_library")
+load("@rules_java//java:defs.bzl", "java_import", "java_library")
exports_files(glob([
"LICENSE-*",
@@ -99,6 +99,29 @@
)
java_library(
+ name = "caffeine",
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = [
+ "//java/com/google/gerrit/server/cache/mem:__pkg__",
+ ],
+ exports = ["@caffeine//jar"],
+)
+
+java_import(
+ name = "caffeine-guava-renamed",
+ jars = ["@caffeine-guava-renamed//file"],
+)
+
+java_library(
+ name = "caffeine-guava",
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = [
+ "//java/com/google/gerrit/server/cache/mem:__pkg__",
+ ],
+ exports = [":caffeine-guava-renamed"],
+)
+
+java_library(
name = "jsch",
data = ["//lib:LICENSE-jsch"],
visibility = ["//visibility:public"],
