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
This commit is contained in:
@@ -88,6 +88,8 @@ Apache2.0
|
||||
* openid:xerces
|
||||
* polymer_externs:polymer_closure
|
||||
* blame-cache
|
||||
* caffeine
|
||||
* caffeine-guava
|
||||
* gson
|
||||
* guava
|
||||
* guava-failureaccess
|
||||
|
25
WORKSPACE
25
WORKSPACE
@@ -241,6 +241,31 @@ maven_jar(
|
||||
sha1 = "1dcf1de382a0bf95a3d8b0849546c88bac1292c9",
|
||||
)
|
||||
|
||||
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",
|
||||
|
@@ -8,6 +8,8 @@ java_library(
|
||||
"//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:jgit",
|
||||
"//lib/guice",
|
||||
|
@@ -17,12 +17,15 @@ package com.google.gerrit.server.cache.mem;
|
||||
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 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory {
|
||||
|
||||
@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,11 +103,22 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory {
|
||||
}
|
||||
|
||||
@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 (key, value) -> 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);
|
||||
}
|
||||
}
|
||||
|
25
lib/BUILD
25
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-*",
|
||||
@@ -110,6 +110,29 @@ java_library(
|
||||
],
|
||||
)
|
||||
|
||||
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"],
|
||||
|
Reference in New Issue
Block a user