Cache TaskConfigFactory per thread
TaskConfigFactory is a cache of task configs. But its instance could be
duplicated in the same thread when a class injects it, which defeats
the purpose of it being a cache. Thus cache TaskConfigFactory instance
per thread. Also rename the class to TaskConfigCache instead of
TaskConfigFactory to better reflect its purpose.
Update gerrit docker image to 3.5.6 so that all the core per thread
cache fixes are available.
Change-Id: I381c4c8f239da19449eb70074fe59bc65567ee45
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java b/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
index a97abf7..73ed927 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
@@ -35,6 +35,7 @@
public static class Module extends FactoryModule {
@Override
protected void configure() {
+ install(new TaskConfigCache.Module());
bind(CapabilityDefinition.class)
.annotatedWith(Exports.named(ViewPathsCapability.VIEW_PATHS))
.to(ViewPathsCapability.class);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java b/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
index e996193..e1ce7ad 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.task;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.task.TaskConfig.Task;
import com.googlesource.gerrit.plugins.task.cli.PatchSetArgument;
import com.googlesource.gerrit.plugins.task.statistics.HitHashMap;
@@ -38,7 +39,7 @@
protected long preloadedFromDefinition;
}
- protected final TaskConfigFactory taskConfigFactory;
+ protected final TaskConfigCache taskConfigCache;
protected final TaskExpression.Factory taskExpressionFactory;
protected final StatisticsMap<TaskExpressionKey, Optional<Task>> optionalTaskByExpression =
new HitHashMap<>();
@@ -47,17 +48,18 @@
@Inject
public Preloader(
- TaskConfigFactory taskConfigFactory, TaskExpression.Factory taskExpressionFactory) {
- this.taskConfigFactory = taskConfigFactory;
+ Provider<TaskConfigCache> taskConfigCacheProvider,
+ TaskExpression.Factory taskExpressionFactory) {
+ this.taskConfigCache = taskConfigCacheProvider.get();
this.taskExpressionFactory = taskExpressionFactory;
}
public List<Task> getRootTasks() throws IOException, ConfigInvalidException {
- return getTasks(taskConfigFactory.getRootConfig(), TaskConfig.SECTION_ROOT);
+ return getTasks(taskConfigCache.getRootConfig(), TaskConfig.SECTION_ROOT);
}
public List<Task> getTasks(FileKey file) throws IOException, ConfigInvalidException {
- return getTasks(taskConfigFactory.getTaskConfig(file), TaskConfig.SECTION_TASK);
+ return getTasks(taskConfigCache.getTaskConfig(file), TaskConfig.SECTION_TASK);
}
protected List<Task> getTasks(TaskConfig cfg, String type) throws IOException {
@@ -160,11 +162,11 @@
}
protected Optional<Task> getOptionalTask(TaskKey key) throws IOException, ConfigInvalidException {
- return taskConfigFactory.getTaskConfig(key.subSection().file()).getOptionalTask(key.task());
+ return taskConfigCache.getTaskConfig(key.subSection().file()).getOptionalTask(key.task());
}
public void masquerade(PatchSetArgument psa) {
- taskConfigFactory.masquerade(psa);
+ taskConfigCache.masquerade(psa);
}
protected static <S, K, V> void preloadField(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigCache.java
similarity index 77%
rename from src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
rename to src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigCache.java
index 5240ccc..1f73a1d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigCache.java
@@ -18,8 +18,10 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.cache.PerThreadCache;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -34,7 +36,40 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Repository;
-public class TaskConfigFactory {
+public class TaskConfigCache {
+ public static class Module extends FactoryModule {
+ @Override
+ protected void configure() {
+ factory(TaskConfigCache.Factory.class);
+ bind(TaskConfigCache.class).toProvider(TaskConfigCache.Provider.class);
+ }
+ }
+
+ public static class Provider implements com.google.inject.Provider<TaskConfigCache> {
+ protected static final PerThreadCache.Key<TaskConfigCache> TASK_CONFIG_CACHE_KEY =
+ PerThreadCache.Key.create(TaskConfigCache.class);
+
+ protected final TaskConfigCache.Factory taskConfigCacheFactory;
+
+ @Inject
+ Provider(TaskConfigCache.Factory taskConfigCacheFactory) {
+ this.taskConfigCacheFactory = taskConfigCacheFactory;
+ }
+
+ @Override
+ public TaskConfigCache get() {
+ PerThreadCache perThreadCache = PerThreadCache.get();
+ if (perThreadCache != null) {
+ return perThreadCache.get(TASK_CONFIG_CACHE_KEY, () -> taskConfigCacheFactory.create());
+ }
+ return taskConfigCacheFactory.create();
+ }
+ }
+
+ private interface Factory {
+ TaskConfigCache create();
+ }
+
private static final FluentLogger log = FluentLogger.forEnclosingClass();
protected final GitRepositoryManager gitMgr;
@@ -47,7 +82,7 @@
protected final Map<FileKey, TaskConfig> taskCfgByFile = new HashMap<>();
@Inject
- protected TaskConfigFactory(
+ protected TaskConfigCache(
AllProjectsNameProvider allProjectsNameProvider,
GitRepositoryManager gitMgr,
PermissionBackend permissionBackend,
diff --git a/test/docker/gerrit/Dockerfile b/test/docker/gerrit/Dockerfile
index 72132d7..a407854 100755
--- a/test/docker/gerrit/Dockerfile
+++ b/test/docker/gerrit/Dockerfile
@@ -1,4 +1,4 @@
-FROM gerritcodereview/gerrit:3.5.0.1-ubuntu20
+FROM gerritcodereview/gerrit:3.5.6-ubuntu20
ENV GERRIT_SITE /var/gerrit
RUN git config -f "$GERRIT_SITE/etc/gerrit.config" auth.type \
