Remove cached Change subNodes on Node completion
Previously when a TaskTree Node's subNodes required a reload, they were
cached until the Node was refreshed which was the next time the Nodes
was used, which can be many changes later, or never. On Node refresh, it
would finally dump any Change subNodes, which are unreusable, that it
had. Since these Change subNodes would not be re-used, they just
consumed extra memory which could be significant if they had a full
subtree such as would be the case when walking each change's ancestors.
Avoid caching these Change subNodes by clearing them as soon as the node
processing is complete.
Before this change, a task.config which walks all dependencies for a
change would not complete after an hour with a 58000m heap when run with
status:open --no-limit. At the time of attempting to run the query, it
is visible that lots of CPU is being spent using top, more than a 1000%,
likely for java GC. After this change it completes in around 30mins
(5800m) heap, and 50mins (48000m) heap!
Change-Id: I88c3862e886e016b5731c8074da810da20480e47
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 6df4b9d..9775643 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -219,9 +219,17 @@
@Override
protected List<Node> loadSubNodes() throws ConfigInvalidException, IOException, OrmException {
- cachedNodes = new SubNodeAdder().getSubNodes();
+ List<Node> nodes = new SubNodeAdder().getSubNodes();
properties.expansionComplete();
- return cachedNodes;
+ if (!properties.isSubNodeReloadRequired()) {
+ cachedNodes = nodes;
+ } else {
+ cachedNodeByTask.clear();
+ nodes.stream()
+ .filter(n -> n != null && !n.isChange())
+ .forEach(n -> cachedNodeByTask.put(n.task.key(), n));
+ }
+ return nodes;
}
/* The task needs to be refreshed before a node is used, however
@@ -240,14 +248,6 @@
isDuplicate |= duplicateKeys.contains(task.duplicateKey);
duplicateKeys.add(task.duplicateKey);
}
-
- if (cachedNodes != null && properties.isSubNodeReloadRequired()) {
- cachedNodeByTask.clear();
- cachedNodes.stream()
- .filter(n -> n != null && !n.isChange())
- .forEach(n -> cachedNodeByTask.put(n.task.key(), n));
- cachedNodes = null;
- }
}
@Override
