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 \