opendev/gerrit
Code Issues Proposed changes

WalkSorter: Don't topo sort

Browse Source 
Topo sorting slurps all interesting commits when the walk starts, and
the nature of this operation means we aren't marking any commits as
uninteresting. This makes WalkSorter far too slow for large
repositories; the default sort will have to be good enough.

Change-Id: I7d5b7e47aae10963a9787defd0067b68d7147b85
This commit is contained in:
Dave Borowitz
2015-06-04 12:43:41 -07:00
parent c6132e6b81
commit 3dd0f4cad9
2 changed files with 11 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-server/src/main/java/com/google/gerrit/server/change/WalkSorter.java
View File

@@ -34,7 +34,6 @@ import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -124,10 +123,13 @@ class WalkSorter {
private List<PatchSetData> sortProject(Project.NameKey project,
Collection<ChangeData> in) throws OrmException, IOException {
// Use default sort; topo sorting is way too slow, as it slurps all
// interesting commits, and we don't mark anything uninteresting. If we
// really need to topo sort, we would need to identify the rootmost commits
// and mark their parents uninteresting, but that's nontrivial in itself.
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {
rw.setRetainBody(retainBody);
rw.sort(RevSort.TOPO);
Multimap<RevCommit, PatchSetData> byCommit = byCommit(rw, in);
if (byCommit.isEmpty()) {
return ImmutableList.of();

10
gerrit-server/src/test/java/com/google/gerrit/server/change/WalkSorterTest.java
View File

@@ -115,12 +115,16 @@ public class WalkSorterTest {
List<ChangeData> changes = ImmutableList.of(cd1, cd2, cd3, cd4);
WalkSorter sorter = new WalkSorter(repoManager);
assertSorted(sorter, changes, ImmutableList.of(
List<PatchSetData> expected = ImmutableList.of(
patchSetData(cd4, c4),
patchSetData(cd3, c3),
patchSetData(cd2, c2),
patchSetData(cd1, c1)));
patchSetData(cd1, c1));
for (List<ChangeData> list : permutations(changes)) {
// Not inOrder(); default sort can't distinguish between these.
assertThat(sorter.sort(list)).containsExactlyElementsIn(expected);
}
}
@Test