ListProjects: Refactor to avoid excessive heap usage

The JVM heap explosion during project list was introduced during the
migration to the permission backend in change I0ba5491fc.

Avoid pre-computing all the projects permissions and visibility checks
when rendering the list, keeping the navigation through an iterator
(fixed memory occupation) instead of pre-loading everything into a
collection (variable memory occupation).

Bug: Issue 10326
Change-Id: I7f0b7efcd57f096d2643723130bd543718b5dcc5

3 files changed, 37 insertions, 25 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
index 3f07d54dd3..a8547649bb 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
@@ -35,6 +35,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCacheImpl;
 import com.google.gerrit.server.project.Util;
 import com.google.inject.Inject;
 import java.util.List;
@@ -92,6 +93,7 @@ public class ListProjectsIT extends AbstractDaemonTest {
 
   @Test
   public void listProjectsWithLimit() throws Exception {
+    ProjectCacheImpl projectCacheImpl = (ProjectCacheImpl) projectCache;
     for (int i = 0; i < 5; i++) {
       createProject("someProject" + i);
     }
@@ -99,9 +101,12 @@ public class ListProjectsIT extends AbstractDaemonTest {
     String p = name("");
     // 5, plus p which was automatically created.
     int n = 6;
+    projectCacheImpl.evictAllByName();
     for (int i = 1; i <= n + 2; i++) {
       assertThatNameList(gApi.projects().list().withPrefix(p).withLimit(i).get())
           .hasSize(Math.min(i, n));
+      assertThat(projectCacheImpl.sizeAllByName())
+          .isAtMost((long) (i + 2)); // 2 = AllProjects + AllUsers
     }
   }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
index 887dfe3360..9ed04b0c43 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
@@ -16,12 +16,10 @@ package com.google.gerrit.server.project;
 
 import static com.google.gerrit.extensions.client.ProjectState.HIDDEN;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.stream.Collectors.toList;
 
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.errors.NoSuchGroupException;
@@ -59,19 +57,19 @@ import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Set;
+import java.util.Objects;
 import java.util.SortedMap;
 import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
@@ -339,7 +337,8 @@ public class ListProjects implements RestReadView<TopLevelResource> {
     PermissionBackend.WithUser perm = permissionBackend.user(currentUser);
     final TreeMap<Project.NameKey, ProjectNode> treeMap = new TreeMap<>();
     try {
-      for (Project.NameKey projectName : filter(perm)) {
+      Iterable<Project.NameKey> projectNames = filter(perm)::iterator;
+      for (Project.NameKey projectName : projectNames) {
         final ProjectState e = projectCache.get(projectName);
         if (e == null || (!all && e.getProject().getState() == HIDDEN)) {
           // If we can't get it from the cache, pretend its not present.
@@ -478,31 +477,28 @@ public class ListProjects implements RestReadView<TopLevelResource> {
     }
   }
 
-  private Collection<Project.NameKey> filter(PermissionBackend.WithUser perm)
-      throws BadRequestException, PermissionBackendException {
-    Collection<Project.NameKey> matches = Lists.newArrayList(scan());
+  private Stream<Project.NameKey> filter(PermissionBackend.WithUser perm)
+      throws BadRequestException {
+    Stream<Project.NameKey> matches = StreamSupport.stream(scan().spliterator(), false);
     if (type == FilterType.PARENT_CANDIDATES) {
-      matches = parentsOf(matches);
+      matches =
+          matches.map(projectCache::get).map(this::parentOf).filter(Objects::nonNull).sorted();
     }
-    return perm.filter(ProjectPermission.ACCESS, matches).stream().sorted().collect(toList());
+    return matches.filter(p -> perm.project(p).testOrFalse(ProjectPermission.ACCESS));
   }
 
-  private Collection<Project.NameKey> parentsOf(Collection<Project.NameKey> matches) {
-    Set<Project.NameKey> parents = new HashSet<>();
-    for (Project.NameKey p : matches) {
-      ProjectState ps = projectCache.get(p);
-      if (ps != null) {
-        Project.NameKey parent = ps.getProject().getParent();
-        if (parent != null) {
-          if (projectCache.get(parent) != null) {
-            parents.add(parent);
-          } else {
-            log.warn("parent project {} of project {} not found", parent.get(), ps.getName());
-          }
-        }
+  private Project.NameKey parentOf(ProjectState ps) {
+    if (ps == null) {
+      return null;
+    }
+    Project.NameKey parent = ps.getProject().getParent();
+    if (parent != null) {
+      if (projectCache.get(parent) != null) {
+        return parent;
       }
+      log.warn("parent project {} of project {} not found", parent.get(), ps.getName());
     }
-    return parents;
+    return null;
   }
 
   private boolean isParentAccessible(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 3daf9c83e5..fff19ff5c8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -16,6 +16,7 @@ package com.google.gerrit.server.project;
 
 import static java.util.stream.Collectors.toSet;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Throwables;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
@@ -337,4 +338,14 @@ public class ProjectCacheImpl implements ProjectCache {
       return mgr.list();
     }
   }
+
+  @VisibleForTesting
+  public void evictAllByName() {
+    byName.invalidateAll();
+  }
+
+  @VisibleForTesting
+  public long sizeAllByName() {
+    return byName.size();
+  }
 }