opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.16' into stable-3.0

Browse Source 
* stable-2.16:
  Switch PatchListCache to using legacy cache backend
  Replace guava caches with caffeine

Add caffeine and caffeine-guava to Documentation/licenses.txt
which did not exist on earlier branches.

Change-Id: I40fb2798a8b6eda2e92e4409c1e57042a8e59ef4
This commit is contained in:
David Pursehouse 2019-12-04 17:43:38 +09:00
commit 791d19e0f4
14 changed files with 243 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Documentation/licenses.txt
View File

@ -90,6 +90,8 @@ Apache2.0
* openid:xerces
* polymer_externs:polymer_closure
* blame-cache
* caffeine
* caffeine-guava
* gson
* guava
* guava-failureaccess

25
WORKSPACE
View File

@ -214,6 +214,31 @@ maven_jar(
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 = "guava-failureaccess",
artifact = "com.google.guava:failureaccess:1.0.1",

25
java/com/google/gerrit/server/cache/CacheBackend.java vendored Normal file
View File

@ -0,0 +1,25 @@
// Copyright (C) 2019 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.cache;
/** Caffeine is used as default cache backend, but can be overridden with Guava backend. */
public enum CacheBackend {
CAFFEINE,
GUAVA;
public boolean isLegacyBackend() {
return this == GUAVA;
}
}

25
java/com/google/gerrit/server/cache/CacheModule.java vendored
View File

@ -68,7 +68,8 @@ public abstract class CacheModule extends FactoryModule {
*/
protected <K, V> CacheBinding<K, V> cache(
String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
CacheProvider<K, V> m = new CacheProvider<>(this, name, keyType, valType);
CacheProvider<K, V> m =
new CacheProvider<>(this, name, keyType, valType, CacheBackend.CAFFEINE);
bindCache(m, name, keyType, valType);
return m;
}
@ -123,7 +124,20 @@ public abstract class CacheModule extends FactoryModule {
*/
protected <K, V> PersistentCacheBinding<K, V> persist(
String name, Class<K> keyType, Class<V> valType) {
return persist(name, TypeLiteral.get(keyType), TypeLiteral.get(valType));
return persist(name, TypeLiteral.get(keyType), TypeLiteral.get(valType), CacheBackend.CAFFEINE);
}
/**
* Declare a named in-memory/on-disk cache.
*
* @param <K> type of key used to lookup entries.
* @param <V> type of value stored by the cache.
* @param backend cache backend.
* @return binding to describe the cache.
*/
protected <K, V> PersistentCacheBinding<K, V> persist(
String name, Class<K> keyType, Class<V> valType, CacheBackend backend) {
return persist(name, TypeLiteral.get(keyType), TypeLiteral.get(valType), backend);
}
/**
@ -135,7 +149,7 @@ public abstract class CacheModule extends FactoryModule {
*/
protected <K, V> PersistentCacheBinding<K, V> persist(
String name, Class<K> keyType, TypeLiteral<V> valType) {
return persist(name, TypeLiteral.get(keyType), valType);
return persist(name, TypeLiteral.get(keyType), valType, CacheBackend.CAFFEINE);
}
/**
@ -146,8 +160,9 @@ public abstract class CacheModule extends FactoryModule {
* @return binding to describe the cache.
*/
protected <K, V> PersistentCacheBinding<K, V> persist(
String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
PersistentCacheProvider<K, V> m = new PersistentCacheProvider<>(this, name, keyType, valType);
String name, TypeLiteral<K> keyType, TypeLiteral<V> valType, CacheBackend backend) {
PersistentCacheProvider<K, V> m =
new PersistentCacheProvider<>(this, name, keyType, valType, backend);
bindCache(m, name, keyType, valType);
Type cacheDefType =

13
java/com/google/gerrit/server/cache/CacheProvider.java vendored
View File

@ -30,6 +30,7 @@ import java.time.Duration;
class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>, CacheDef<K, V> {
private final CacheModule module;
private final CacheBackend backend;
final String name;
private final TypeLiteral<K> keyType;
private final TypeLiteral<V> valType;
@ -44,11 +45,17 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
private MemoryCacheFactory memoryCacheFactory;
private boolean frozen;
CacheProvider(CacheModule module, String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
CacheProvider(
CacheModule module,
String name,
TypeLiteral<K> keyType,
TypeLiteral<V> valType,
CacheBackend backend) {
this.module = module;
this.name = name;
this.keyType = keyType;
this.valType = valType;
this.backend = backend;
}
@Inject(optional = true)
@ -159,7 +166,9 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
public Cache<K, V> get() {
freeze();
CacheLoader<K, V> ldr = loader();
return ldr != null ? memoryCacheFactory.build(this, ldr) : memoryCacheFactory.build(this);
return ldr != null
? memoryCacheFactory.build(this, ldr, backend)
: memoryCacheFactory.build(this, backend);
}
protected void checkNotFrozen() {

5
java/com/google/gerrit/server/cache/MemoryCacheFactory.java vendored
View File

@ -19,7 +19,8 @@ import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
public interface MemoryCacheFactory {
<K, V> Cache<K, V> build(CacheDef<K, V> def);
<K, V> Cache<K, V> build(CacheDef<K, V> def, CacheBackend backend);
<K, V> LoadingCache<K, V> build(CacheDef<K, V> def, CacheLoader<K, V> loader);
<K, V> LoadingCache<K, V> build(
CacheDef<K, V> def, CacheLoader<K, V> loader, CacheBackend backend);
}

5
java/com/google/gerrit/server/cache/PersistentCacheFactory.java vendored
View File

@ -19,9 +19,10 @@ import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
public interface PersistentCacheFactory {
<K, V> Cache<K, V> build(PersistentCacheDef<K, V> def);
<K, V> Cache<K, V> build(PersistentCacheDef<K, V> def, CacheBackend backend);
<K, V> LoadingCache<K, V> build(PersistentCacheDef<K, V> def, CacheLoader<K, V> loader);
<K, V> LoadingCache<K, V> build(
PersistentCacheDef<K, V> def, CacheLoader<K, V> loader, CacheBackend backend);
void onStop(String plugin);
}

17
java/com/google/gerrit/server/cache/PersistentCacheProvider.java vendored
View File

@ -30,6 +30,7 @@ import java.time.Duration;
class PersistentCacheProvider<K, V> extends CacheProvider<K, V>
implements Provider<Cache<K, V>>, PersistentCacheBinding<K, V>, PersistentCacheDef<K, V> {
private final CacheBackend backend;
private int version;
private long diskLimit;
private CacheSerializer<K> keySerializer;
@ -39,9 +40,19 @@ class PersistentCacheProvider<K, V> extends CacheProvider<K, V>
PersistentCacheProvider(
CacheModule module, String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
super(module, name, keyType, valType);
this(module, name, keyType, valType, CacheBackend.CAFFEINE);
}
PersistentCacheProvider(
CacheModule module,
String name,
TypeLiteral<K> keyType,
TypeLiteral<V> valType,
CacheBackend backend) {
super(module, name, keyType, valType, backend);
version = -1;
diskLimit = 128 << 20;
this.backend = backend;
}
@Inject(optional = true)
@ -130,8 +141,8 @@ class PersistentCacheProvider<K, V> extends CacheProvider<K, V>
freeze();
CacheLoader<K, V> ldr = loader();
return ldr != null
? persistentCacheFactory.build(this, ldr)
: persistentCacheFactory.build(this);
? persistentCacheFactory.build(this, ldr, backend)
: persistentCacheFactory.build(this, backend);
}
private static <T> void checkSerializer(

19
java/com/google/gerrit/server/cache/h2/H2CacheFactory.java vendored
View File

@ -21,6 +21,7 @@ import com.google.common.flogger.FluentLogger;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.server.cache.CacheBackend;
import com.google.gerrit.server.cache.MemoryCacheFactory;
import com.google.gerrit.server.cache.PersistentCacheDef;
import com.google.gerrit.server.cache.PersistentCacheFactory;
@ -156,18 +157,21 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener {
@SuppressWarnings({"unchecked"})
@Override
public <K, V> Cache<K, V> build(PersistentCacheDef<K, V> in) {
public <K, V> Cache<K, V> build(PersistentCacheDef<K, V> in, CacheBackend backend) {
long limit = config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit());
if (cacheDir == null || limit <= 0) {
return memCacheFactory.build(in);
return memCacheFactory.build(in, backend);
}
H2CacheDefProxy<K, V> def = new H2CacheDefProxy<>(in);
SqlStore<K, V> store = newSqlStore(def, limit);
H2CacheImpl<K, V> cache =
new H2CacheImpl<>(
executor, store, def.keyType(), (Cache<K, ValueHolder<V>>) memCacheFactory.build(def));
executor,
store,
def.keyType(),
(Cache<K, ValueHolder<V>>) memCacheFactory.build(def, backend));
synchronized (caches) {
caches.add(cache);
}
@ -176,11 +180,12 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener {
@SuppressWarnings("unchecked")
@Override
public <K, V> LoadingCache<K, V> build(PersistentCacheDef<K, V> in, CacheLoader<K, V> loader) {
public <K, V> LoadingCache<K, V> build(
PersistentCacheDef<K, V> in, CacheLoader<K, V> loader, CacheBackend backend) {
long limit = config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit());
if (cacheDir == null || limit <= 0) {
return memCacheFactory.build(in, loader);
return memCacheFactory.build(in, loader, backend);
}
H2CacheDefProxy<K, V> def = new H2CacheDefProxy<>(in);
@ -188,7 +193,9 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener {
Cache<K, ValueHolder<V>> mem =
(Cache<K, ValueHolder<V>>)
memCacheFactory.build(
def, (CacheLoader<K, V>) new H2CacheImpl.Loader<>(executor, store, loader));
def,
(CacheLoader<K, V>) new H2CacheImpl.Loader<>(executor, store, loader),
backend);
H2CacheImpl<K, V> cache = new H2CacheImpl<>(executor, store, def.keyType(), mem);
synchronized (caches) {
caches.add(cache);

2
java/com/google/gerrit/server/cache/mem/BUILD vendored
View File

@ -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/guice",
"//lib/jgit/org.eclipse.jgit:jgit",

86
java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java vendored
View File

@ -17,13 +17,18 @@ 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.CacheBackend;
import com.google.gerrit.server.cache.CacheDef;
import com.google.gerrit.server.cache.ForwardingRemovalListener;
import com.google.gerrit.server.cache.MemoryCacheFactory;
@ -46,25 +51,30 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory {
}
@Override
public <K, V> Cache<K, V> build(CacheDef<K, V> def) {
return create(def).build();
public <K, V> Cache<K, V> build(CacheDef<K, V> def, CacheBackend backend) {
return backend.isLegacyBackend()
? createLegacy(def).build()
: 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);
public <K, V> LoadingCache<K, V> build(
CacheDef<K, V> def, CacheLoader<K, V> loader, CacheBackend backend) {
return backend.isLegacyBackend()
? createLegacy(def).build(loader)
: 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> CacheBuilder<K, V> createLegacy(CacheDef<K, V> def) {
CacheBuilder<K, V> builder = newLegacyCacheBuilder();
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();
com.google.common.cache.Weigher<K, V> weigher = def.weigher();
if (weigher == null) {
weigher = unitWeight();
}
@ -98,6 +108,42 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory {
return builder;
}
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(newRemovalListener(def.name()));
builder.weigher(newWeigher(def.weigher()));
Duration expireAfterWrite = def.expireAfterWrite();
if (has(def.configKey(), "maxAge")) {
builder.expireAfterWrite(
ConfigUtil.getTimeUnit(
cfg, "cache", def.configKey(), "maxAge", toSeconds(expireAfterWrite), SECONDS),
SECONDS);
} else if (expireAfterWrite != null) {
builder.expireAfterWrite(expireAfterWrite.toNanos(), NANOSECONDS);
}
Duration expireAfterAccess = def.expireFromMemoryAfterAccess();
if (has(def.configKey(), "expireFromMemoryAfterAccess")) {
builder.expireAfterAccess(
ConfigUtil.getTimeUnit(
cfg,
"cache",
def.configKey(),
"expireFromMemoryAfterAccess",
toSeconds(expireAfterAccess),
SECONDS),
SECONDS);
} else if (expireAfterAccess != null) {
builder.expireAfterAccess(expireAfterAccess.toNanos(), NANOSECONDS);
}
return builder;
}
private static long toSeconds(@Nullable Duration duration) {
return duration != null ? duration.getSeconds() : 0;
}
@ -107,11 +153,31 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory {
}
@SuppressWarnings("unchecked")
private static <K, V> CacheBuilder<K, V> newCacheBuilder() {
private static <K, V> CacheBuilder<K, V> newLegacyCacheBuilder() {
return (CacheBuilder<K, V>) CacheBuilder.newBuilder();
}
private static <K, V> Weigher<K, V> unitWeight() {
private static <K, V> com.google.common.cache.Weigher<K, V> unitWeight() {
return (key, value) -> 1;
}
@SuppressWarnings("unchecked")
private static <K, V> Caffeine<K, V> newCacheBuilder() {
return (Caffeine<K, V>) Caffeine.newBuilder();
}
@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);
}
}

5
java/com/google/gerrit/server/patch/PatchListCacheImpl.java
View File

@ -22,6 +22,7 @@ import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.cache.CacheBackend;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
@ -45,7 +46,9 @@ public class PatchListCacheImpl implements PatchListCache {
@Override
protected void configure() {
factory(PatchListLoader.Factory.class);
persist(FILE_NAME, PatchListKey.class, PatchList.class)
// TODO(davido): Switch off using legacy cache backend, after fixing PatchListLoader
// to be recursion free.
persist(FILE_NAME, PatchListKey.class, PatchList.class, CacheBackend.GUAVA)
.maximumWeight(10 << 20)
.weigher(PatchListWeigher.class);

21
javatests/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java
View File

@ -39,7 +39,9 @@ import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.Text;
import com.google.inject.Inject;
import com.google.inject.name.Named;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
@ -65,6 +67,25 @@ public class PatchListCacheIT extends AbstractDaemonTest {
@Named("diff")
private Cache<PatchListKey, PatchList> abstractPatchListCache;
@Test
public void ensureLegacyBackendIsUsedForFileCacheBackend() throws Exception {
Field fileCacheField = patchListCache.getClass().getDeclaredField("fileCache");
fileCacheField.setAccessible(true);
// Use the reflection to access "localCache" field that is only present in Guava backend.
assertThat(
Arrays.stream(fileCacheField.get(patchListCache).getClass().getDeclaredFields())
.anyMatch(f -> f.getName().equals("localCache")))
.isTrue();
// intraCache (and all other cache backends) should use Caffeine backend.
Field intraCacheField = patchListCache.getClass().getDeclaredField("intraCache");
intraCacheField.setAccessible(true);
assertThat(
Arrays.stream(intraCacheField.get(patchListCache).getClass().getDeclaredFields())
.noneMatch(f -> f.getName().equals("localCache")))
.isTrue();
}
@Test
public void listPatchesAgainstBase() throws Exception {
commitBuilder().add(FILE_D, "4").message(SUBJECT_1).create();

25
lib/BUILD
View File

@ -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-*",
@ -67,6 +67,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"],