ChangeUtil: Don't scan all refs to get next patch set ID
The nextPatchSetId variant that takes a Map<String, Ref> was intended
for use in ReceiveCommits, which happens to have such a map readily
available. When it's not already available, scanning all refs is
wasteful, since we only care about refs under a given change. Split the
method that takes a Map into two different, slightly more descriptively
named methods. This should significantly improve performance on large
repositories of REST API handlers like Rebase that create new patch
sets.
The type Map<String, ObjectId> was chosen for its similarity to the
RefCache interface, which reports ObjectIds instead of Refs. A similar
interface will be used in the restricted repository view that we expose
to BatchUpdate.Contexts.
Change-Id: I6ffcc3099658afea1cf8ac5bcec44b92e263a915
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
index 10ae60c..cfd1d5d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
@@ -16,6 +16,7 @@
import static java.util.Comparator.comparingInt;
+import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
import com.google.common.io.BaseEncoding;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -24,8 +25,8 @@
import java.security.SecureRandom;
import java.util.Map;
import java.util.Random;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
@Singleton
@@ -47,7 +48,16 @@
return UUID_ENCODING.encode(buf, 0, 4) + '_' + UUID_ENCODING.encode(buf, 4, 4);
}
- public static PatchSet.Id nextPatchSetId(Map<String, Ref> allRefs, PatchSet.Id id) {
+ /**
+ * Get the next patch set ID from a previously-read map of all refs.
+ *
+ * @param allRefs map of full ref name to ref, in the same format returned by {@link
+ * org.eclipse.jgit.lib.RefDatabase#getRefs(String)} when passing {@code ""}.
+ * @param id previous patch set ID.
+ * @return next unused patch set ID for the same change, skipping any IDs whose corresponding ref
+ * names appear in the {@code allRefs} map.
+ */
+ public static PatchSet.Id nextPatchSetIdFromAllRefsMap(Map<String, Ref> allRefs, PatchSet.Id id) {
PatchSet.Id next = nextPatchSetId(id);
while (allRefs.containsKey(next.toRefName())) {
next = nextPatchSetId(next);
@@ -55,12 +65,55 @@
return next;
}
+ /**
+ * Get the next patch set ID from a previously-read map of refs below the change prefix.
+ *
+ * @param changeRefs map of ref suffix to SHA-1, where the keys are ref names with the {@code
+ * refs/changes/CD/ABCD/} prefix stripped. All refs should be under {@code id}'s change ref
+ * prefix. The keys match the format returned by {@link
+ * org.eclipse.jgit.lib.RefDatabase#getRefs(String)} when passing the appropriate {@code
+ * refs/changes/CD/ABCD}.
+ * @param id previous patch set ID.
+ * @return next unused patch set ID for the same change, skipping any IDs whose corresponding ref
+ * names appear in the {@code changeRefs} map.
+ */
+ public static PatchSet.Id nextPatchSetIdFromChangeRefsMap(
+ Map<String, ObjectId> changeRefs, PatchSet.Id id) {
+ int prefixLen = id.getParentKey().toRefPrefix().length();
+ PatchSet.Id next = nextPatchSetId(id);
+ while (changeRefs.containsKey(next.toRefName().substring(prefixLen))) {
+ next = nextPatchSetId(next);
+ }
+ return next;
+ }
+
+ /**
+ * Get the next patch set ID just looking at a single previous patch set ID.
+ *
+ * <p>This patch set ID may or may not be available in the database; callers that want a
+ * previously-unused ID should use {@link #nextPatchSetIdFromAllRefsMap} or {@link
+ * #nextPatchSetIdFromChangeRefsMap}.
+ *
+ * @param id previous patch set ID.
+ * @return next patch set ID for the same change, incrementing by 1.
+ */
public static PatchSet.Id nextPatchSetId(PatchSet.Id id) {
return new PatchSet.Id(id.getParentKey(), id.get() + 1);
}
+ /**
+ * Get the next patch set ID from scanning refs in the repo.
+ *
+ * @param git repository to scan for patch set refs.
+ * @param id previous patch set ID.
+ * @return next unused patch set ID for the same change, skipping any IDs whose corresponding ref
+ * names appear in the repository.
+ */
public static PatchSet.Id nextPatchSetId(Repository git, PatchSet.Id id) throws IOException {
- return nextPatchSetId(git.getRefDatabase().getRefs(RefDatabase.ALL), id);
+ return nextPatchSetIdFromChangeRefsMap(
+ Maps.transformValues(
+ git.getRefDatabase().getRefs(id.getParentKey().toRefPrefix()), Ref::getObjectId),
+ id);
}
public static String cropSubject(String subject) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 189df45..ae00b2e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -2507,7 +2507,8 @@
private void newPatchSet() throws IOException {
RevCommit newCommit = rp.getRevWalk().parseCommit(newCommitId);
- psId = ChangeUtil.nextPatchSetId(allRefs, notes.getChange().currentPatchSetId());
+ psId =
+ ChangeUtil.nextPatchSetIdFromAllRefsMap(allRefs, notes.getChange().currentPatchSetId());
info = patchSetInfoFactory.get(rp.getRevWalk(), newCommit, psId);
cmd = new ReceiveCommand(ObjectId.zeroId(), newCommitId, psId.toRefName());
}
