opendev/gerrit
Code Issues Proposed changes

Improve performance of ReceiveCommits by reducing RevWalk load

Browse Source 
JGit RevWalk does not perform well when a large number of objects are
added to the start set by markStart or markUninteresting. Avoid
putting existing refs/changes/ or refs/tags/ into the RevWalk and
instead use only the refs/heads namespace and the name of the branch
used in the refs/for/ push line.

Catch existing changes by looking for their exact commit SHA-1, rather
than complete ancestory. This should have roughly the same outcome for
anyone pushing a new commit on top of an existing open change, but
with lower computional cost at the server.

Change-Id: Ie2bb9176799528f6422292f3f889e3d28cbf5135
This commit is contained in:
Shawn O. Pearce
2012-07-29 21:57:15 -07:00
parent c545c09012
commit 2fbea6c5da
1 changed files with 17 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.git;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK;
@@ -28,6 +29,7 @@ import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.CheckedFuture;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
@@ -1116,12 +1118,22 @@ public class ReceiveCommits {
walk.sort(RevSort.TOPO);
walk.sort(RevSort.REVERSE, true);
try {
Set<ObjectId> existing = Sets.newHashSet();
walk.markStart(walk.parseCommit(newChange.getNewId()));
for (ObjectId id : existingObjects()) {
try {
walk.markUninteresting(walk.parseCommit(id));
} catch (IOException e) {
for (Ref ref : repo.getAllRefs().values()) {
if (ref.getObjectId() == null) {
continue;
} else if (ref.getName().startsWith("refs/changes/")) {
existing.add(ref.getObjectId());
} else if (ref.getName().startsWith(R_HEADS)
|| ref.getName().equals(destBranchCtl.getRefName())) {
try {
walk.markUninteresting(walk.parseCommit(ref.getObjectId()));
} catch (IOException e) {
log.warn(String.format("Invalid ref %s in %s",
ref.getName(), project.getName()), e);
continue;
}
}
}
@@ -1131,7 +1143,7 @@ public class ReceiveCommits {
if (c == null) {
break;
}
if (replaceByCommit.containsKey(c)) {
if (existing.contains(c) || replaceByCommit.containsKey(c)) {
// This commit was already scheduled to replace an existing PatchSet.
//
continue;