Cache nameFactory changes in TaskTree When walking changes, it is likely that many changes will get requeried since many of the changes queried will also be in the output. Cache the output of these mostly small (and thus not very memory intensive) queries to avoid repeating them. In a sample walking ancestors use case this caching for over 33K ChangeNodes results in about 1/3 of the original queries, and saves a small but measurable amount of the total time. In the case of a task.config which walks all dependencies for a change when run with status:open --no-limit --task--applicable the gain can be seen below. Before this change: 4m47s 4m56s 4m22s 5m5s 4m27s After this change: 4m18s 4m4s 4m37s 3m56s 4m5s Change-Id: Ibe04c5fae8807635be378d6627dbdc7d8bd724b9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java index 6533f5a..2821123 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java +++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
@@ -124,7 +124,7 @@ } protected PluginDefinedInfo createWithExceptions(ChangeData c) { - MatchCache matchCache = new MatchCache(predicateCache, c); + MatchCache matchCache = new MatchCache(definitions.predicateCache, c); TaskPluginAttribute a = new TaskPluginAttribute(); try { for (Node node : definitions.getRootNodes(c)) { @@ -321,7 +321,7 @@ } else { MatchCache subMatchCache = matchCache; if (!matchCache.changeData.getId().equals(subNode.getChangeData().getId())) { - subMatchCache = new MatchCache(predicateCache, subNode.getChangeData()); + subMatchCache = new MatchCache(definitions.predicateCache, subNode.getChangeData()); } new AttributeFactory(subNode, subMatchCache).create().ifPresent(t -> subTasks.add(t)); } @@ -351,7 +351,7 @@ public void initStatistics() { if (options.includeStatistics) { statistics = new Statistics(); - predicateCache.initStatistics(); + definitions.predicateCache.initStatistics(); definitions.preloader.initStatistics(); definitions.initStatistics(); } @@ -362,7 +362,7 @@ statistics.numberOfChanges = pluginInfosByChange.size(); statistics.numberOfTaskPluginAttributes = pluginInfosByChange.values().stream().filter(tpa -> tpa != null).count(); - statistics.predicateCache = predicateCache.getStatistics(); + statistics.predicateCache = definitions.predicateCache.getStatistics(); statistics.preloader = definitions.preloader.getStatistics(); statistics.treeCaches = definitions.getStatistics(); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java index 64b333f..2b89f90 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java +++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -70,6 +70,7 @@ public static class Statistics { public Object definitionsPerSubSectionCache; public Object definitionsByBranchBySubSectionCache; + public Object changesByNamesFactoryQueryCache; } protected static final String TASK_DIR = "task"; @@ -77,10 +78,13 @@ protected final AccountResolver accountResolver; protected final AllUsersNameProvider allUsers; protected final CurrentUser user; + protected final PredicateCache predicateCache; protected final Preloader preloader; protected final NodeList root = new NodeList(); protected final Provider<ChangeQueryBuilder> changeQueryBuilderProvider; protected final Provider<ChangeQueryProcessor> changeQueryProcessorProvider; + protected final StatisticsMap<String, List<ChangeData>> changesByNamesFactoryQuery = + new HitHashMap<>(); protected final StatisticsMap<SubSectionKey, List<Task>> definitionsBySubSection = new HitHashMapOfCollection<>(); protected final StatisticsMap<SubSectionKey, Map<BranchNameKey, List<Task>>> @@ -97,12 +101,14 @@ CurrentUser user, Provider<ChangeQueryBuilder> changeQueryBuilderProvider, Provider<ChangeQueryProcessor> changeQueryProcessorProvider, + PredicateCache predicateCache, Preloader preloader) { this.accountResolver = accountResolver; this.allUsers = allUsers; this.user = user != null ? user : anonymousUser; this.changeQueryProcessorProvider = changeQueryProcessorProvider; this.changeQueryBuilderProvider = changeQueryBuilderProvider; + this.predicateCache = predicateCache; this.preloader = preloader; } @@ -411,12 +417,7 @@ throws ConfigInvalidException, IOException, StorageException { try { if (namesFactory.changes != null) { - List<ChangeData> changeDataList = - changeQueryProcessorProvider - .get() - .query(changeQueryBuilderProvider.get().parse(namesFactory.changes)) - .entities(); - for (ChangeData changeData : changeDataList) { + for (ChangeData changeData : query(namesFactory.changes)) { addPreloaded( preloader.preload( task.config.new Task(tasksFactory, changeData.getId().toString())), @@ -583,10 +584,24 @@ return BranchNameKey.create(allUsers.get(), RefNames.refsUsers(acct)); } + public List<ChangeData> query(String query) throws StorageException, QueryParseException { + List<ChangeData> changeDataList = changesByNamesFactoryQuery.get(query); + if (changeDataList == null) { + changeDataList = + changeQueryProcessorProvider + .get() + .query(changeQueryBuilderProvider.get().parse(query)) + .entities(); + changesByNamesFactoryQuery.put(query, changeDataList); + } + return changeDataList; + } + public void initStatistics() { statistics = new Statistics(); definitionsBySubSection.initStatistics(); definitionsByBranchBySubSection.initStatistics(); + changesByNamesFactoryQuery.initStatistics(); } public Statistics getStatistics() { @@ -594,6 +609,7 @@ statistics.definitionsPerSubSectionCache = definitionsBySubSection.getStatistics(); statistics.definitionsByBranchBySubSectionCache = definitionsByBranchBySubSection.getStatistics(); + statistics.changesByNamesFactoryQueryCache = changesByNamesFactoryQuery.getStatistics(); } return statistics; }