AutoMerger: Support disabling writes to refs/cache-automerge/*

It turns out that when the persistent diff cache (and eventually the
blame cache) is present, we never have to actually read the
refs/cache-automerge/* refs. When we need the commit/tree data for the
purposes of populating the cache, we can always just read that back
from the ObjectInserter that created it. This also means we can do
batch work like an online RebuildNoteDb without ever having to write
refs/cache-automerge/* refs.

Leave the default behavior of writing out the refs as people currently
expect that behavior, but we could potentially revisit that in the
future.

Change-Id: I8bad4de10c8ef6ac6fd5a8b2cf79fc3e3ef6f830

Documentation/config-gerrit.txt

@@ -928,6 +928,21 @@ deleted or published.
 +
 Default is true.
 
+[[change.cacheAutomerge]]change.cacheAutomerge::
++
+When reviewing diff commits, the left-hand side shows the output of the
+result of JGit's automatic merge algorithm. This option controls whether
+this output is cached in the change repository, or if only the diff is
+cached in the persistent `diff` cache.
++
+If true, automerge results are stored in the repository under
+`refs/cache-automerge/*`; the results of diffing the change against its
+automerge base are stored in the diff cache. If false, no extra data is
+stored in the repository, only the diff cache. This can result in slight
+performance improvements by reducing the number of refs in the repo.
++
+Default is true.
+
 [[change.submitLabel]]change.submitLabel::
 +
 Label name for the submit button.

gerrit-server/src/main/java/com/google/gerrit/server/change/GetBlame.java

@@ -37,6 +37,7 @@ import com.google.inject.Inject;
 
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
@@ -85,7 +86,8 @@ public class GetBlame implements RestReadView<FileResource> {
 
     Project.NameKey project = resource.getRevision().getChange().getProject();
     try (Repository repository = repoManager.openRepository(project);
-        RevWalk revWalk = new RevWalk(repository)) {
+        ObjectInserter ins = repository.newObjectInserter();
+        RevWalk revWalk = new RevWalk(ins.newReader())) {
       String refName = resource.getRevision().getEdit().isPresent()
           ? resource.getRevision().getEdit().get().getRefName()
           : resource.getRevision().getPatchSet().getRefName();
@@ -111,8 +113,8 @@ public class GetBlame implements RestReadView<FileResource> {
         result = blame(parents[0], path, repository, revWalk);
 
       } else if (parents.length == 2) {
-        ObjectId automerge = autoMerger.merge(repository, revWalk, revCommit,
+        ObjectId automerge = autoMerger.merge(repository, revWalk, ins,
-            mergeStrategy);
+            revCommit, mergeStrategy);
         result = blame(automerge, path, repository, revWalk);
 
       } else {

gerrit-server/src/main/java/com/google/gerrit/server/index/change/AllChangesIndexer.java

@@ -267,7 +267,6 @@ public class AllChangesIndexer
     private final ProgressMonitor failed;
     private final PrintWriter verboseWriter;
     private final Repository repo;
-    private RevWalk walk;
 
     private ProjectIndexer(ChangeIndexer indexer,
         ThreeWayMergeStrategy mergeStrategy,
@@ -289,8 +288,8 @@ public class AllChangesIndexer
 
     @Override
     public Void call() throws Exception {
-      walk = new RevWalk(repo);
+      try (ObjectInserter ins = repo.newObjectInserter();
-      try {
+          RevWalk walk = new RevWalk(ins.newReader())) {
         // Walk only refs first to cover as many changes as we can without having
         // to mark every single change.
         for (Ref ref : repo.getRefDatabase().getRefs(Constants.R_HEADS).values()) {
@@ -303,26 +302,25 @@ public class AllChangesIndexer
         RevCommit bCommit;
         while ((bCommit = walk.next()) != null && !byId.isEmpty()) {
           if (byId.containsKey(bCommit)) {
-            getPathsAndIndex(bCommit);
+            getPathsAndIndex(walk, ins, bCommit);
             byId.removeAll(bCommit);
           }
         }
 
         for (ObjectId id : byId.keySet()) {
-          getPathsAndIndex(id);
+          getPathsAndIndex(walk, ins, id);
         }
-      } finally {
-        walk.close();
       }
       return null;
     }
 
-    private void getPathsAndIndex(ObjectId b) throws Exception {
+    private void getPathsAndIndex(RevWalk walk, ObjectInserter ins, ObjectId b)
+        throws Exception {
       List<ChangeData> cds = Lists.newArrayList(byId.get(b));
       try (DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
         RevCommit bCommit = walk.parseCommit(b);
         RevTree bTree = bCommit.getTree();
-        RevTree aTree = aFor(bCommit, walk);
+        RevTree aTree = aFor(bCommit, walk, ins);
         df.setRepository(repo);
         if (!cds.isEmpty()) {
           List<String> paths = (aTree != null)
@@ -364,7 +362,8 @@ public class AllChangesIndexer
       return ImmutableList.copyOf(paths);
     }
 
-    private RevTree aFor(RevCommit b, RevWalk walk) throws IOException {
+    private RevTree aFor(RevCommit b, RevWalk walk, ObjectInserter ins)
+        throws IOException {
       switch (b.getParentCount()) {
         case 0:
           return walk.parseTree(emptyTree());
@@ -373,7 +372,7 @@ public class AllChangesIndexer
           walk.parseBody(a);
           return walk.parseTree(a.getTree());
         case 2:
-          RevCommit am = autoMerger.merge(repo, walk, b, mergeStrategy);
+          RevCommit am = autoMerger.merge(repo, walk, ins, b, mergeStrategy);
           return am == null ? null : am.getTree();
         default:
           return null;

gerrit-server/src/main/java/com/google/gerrit/server/patch/AutoMerger.java

@@ -18,6 +18,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
 
 import org.eclipse.jgit.diff.Sequence;
@@ -25,6 +26,7 @@ import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.dircache.DirCacheBuilder;
 import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
@@ -52,9 +54,13 @@ public class AutoMerger {
   private static final Logger log = LoggerFactory.getLogger(AutoMerger.class);
 
   private final PersonIdent gerritIdent;
+  private final boolean save;
 
   @Inject
-  AutoMerger(@GerritPersonIdent PersonIdent gerritIdent) {
+  AutoMerger(
+      @GerritServerConfig Config cfg,
+      @GerritPersonIdent PersonIdent gerritIdent) {
+    save = cfg.getBoolean("change", null, "cacheAutomerge", true);
     this.gerritIdent = gerritIdent;
   }
 
@@ -64,8 +70,9 @@ public class AutoMerger {
    * @return auto-merge commit or {@code null} if an auto-merge commit
    *     couldn't be created. Headers of the returned RevCommit are parsed.
    */
-  public RevCommit merge(Repository repo, RevWalk rw, RevCommit merge,
+  public RevCommit merge(Repository repo, RevWalk rw, final ObjectInserter ins,
-      ThreeWayMergeStrategy mergeStrategy) throws IOException {
+      RevCommit merge, ThreeWayMergeStrategy mergeStrategy)
+      throws IOException {
     rw.parseHeaders(merge);
     String hash = merge.name();
     String refName = RefNames.REFS_CACHE_AUTOMERGE
@@ -78,11 +85,10 @@ public class AutoMerger {
       if (obj instanceof RevCommit) {
         return (RevCommit) obj;
       }
-      return commit(repo, rw, refName, obj, merge);
+      return commit(repo, rw, ins, refName, obj, merge);
     }
 
     ResolveMerger m = (ResolveMerger) mergeStrategy.newMerger(repo, true);
-    try (ObjectInserter ins = repo.newObjectInserter()) {
     DirCache dc = DirCache.newInCore();
     m.setDirCache(dc);
     m.setObjectInserter(new ObjectInserter.Filter() {
@@ -194,12 +200,11 @@ public class AutoMerger {
     }
     ins.flush();
 
-      return commit(repo, rw, refName, treeId, merge);
+    return commit(repo, rw, ins, refName, treeId, merge);
-    }
   }
 
-  private RevCommit commit(Repository repo, RevWalk rw, String refName,
+  private RevCommit commit(Repository repo, RevWalk rw, ObjectInserter ins,
-      ObjectId tree, RevCommit merge) throws IOException {
+      String refName, ObjectId tree, RevCommit merge) throws IOException {
     rw.parseHeaders(merge);
     // For maximum stability, choose a single ident using the committer time of
     // the input commit, using the server name and timezone.
@@ -216,16 +221,15 @@ public class AutoMerger {
       cb.addParentId(p);
     }
     ObjectId commitId;
-    try (ObjectInserter ins = repo.newObjectInserter()) {
     commitId = ins.insert(cb);
+    if (save) {
       ins.flush();
-    }
 
       RefUpdate ru = repo.updateRef(refName);
       ru.setNewObjectId(commitId);
       ru.disableRefLog();
       ru.forceUpdate();
-
+    }
     return rw.parseCommit(commitId);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java

@@ -136,11 +136,12 @@ public class PatchListLoader implements Callable<PatchList> {
   private PatchList readPatchList(final PatchListKey key, final Repository repo)
       throws IOException, PatchListNotAvailableException {
     final RawTextComparator cmp = comparatorFor(key.getWhitespace());
-    try (ObjectReader reader = repo.newObjectReader();
+    try (ObjectInserter ins = repo.newObjectInserter();
+        ObjectReader reader = ins.newReader();
         RevWalk rw = new RevWalk(reader);
         DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
       final RevCommit b = rw.parseCommit(key.getNewId());
-      final RevObject a = aFor(key, repo, rw, b);
+      final RevObject a = aFor(key, repo, rw, ins, b);
 
       if (a == null) {
         // TODO(sop) Remove this case.
@@ -317,8 +318,8 @@ public class PatchListLoader implements Callable<PatchList> {
     }
   }
 
-  private RevObject aFor(final PatchListKey key,
+  private RevObject aFor(PatchListKey key,
-      final Repository repo, final RevWalk rw, final RevCommit b)
+      Repository repo, RevWalk rw, ObjectInserter ins, RevCommit b)
       throws IOException {
     if (key.getOldId() != null) {
       return rw.parseAny(key.getOldId());
@@ -333,7 +334,7 @@ public class PatchListLoader implements Callable<PatchList> {
         return r;
       }
       case 2:
-        return autoMerger.merge(repo, rw, b, mergeStrategy);
+        return autoMerger.merge(repo, rw, ins, b, mergeStrategy);
       default:
         // TODO(sop) handle an octopus merge.
         return null;