opendev/gerrit
Code Issues Proposed changes

Optimized "IncludedIn" Algorithm

Browse Source 
The most expensive part of the "IncludedIn" calculation is the
processing of paths which do not contain the requested commit. In that
case the processor needs to look at all reachable commits starting from
the tip commit to the initial commit. The method RevWalk.isMergedIn()
walks over the whole parent graph again and again for each tag and each
branch. The amount of walks can be reduced by sorting the tags and
branches and start bottom up. This allows ignoring subgraphs where the
commit is not contained in subsequent iterations and stop graph
traversal when a tag or branch is reached where we already know that it
contains the commit.

Performance measurement on larger Git repositories, like Linux Kernel,
indicate that the runtime with the new algorithm is three to four times
faster.

To be able to expose the "IncludedIn" calculation as a REST service or
SSH command I have extracted the algorithm into an own class in the
gerrit-server package.

Change-Id: I56b32a77e02e47dd31ec6ce4adfe0a781e73a76c
Signed-off-by: Christian Grail <christian.grail@sap.com>
This commit is contained in:
Christian Grail
2013-06-27 02:33:17 -07:00
committed by Saša Živkov
parent ac3d66305c
commit 52b7f60f3d
3 changed files with 367 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/IncludedInDetailFactory.java
View File

@@ -20,6 +20,7 @@ import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -29,23 +30,15 @@ import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
/** Creates a {@link IncludedInDetail} of a {@link Change}. */
class IncludedInDetailFactory extends Handler<IncludedInDetail> {
private static final Logger log =
LoggerFactory.getLogger(IncludedInDetailFactory.class);
interface Factory {
IncludedInDetailFactory create(Change.Id id);
@@ -56,7 +49,6 @@ class IncludedInDetailFactory extends Handler<IncludedInDetail> {
private final GitRepositoryManager repoManager;
private final Change.Id changeId;
private IncludedInDetail detail;
private ChangeControl control;
@Inject
@@ -92,11 +84,7 @@ class IncludedInDetailFactory extends Handler<IncludedInDetail> {
throw new InvalidRevisionException();
}
detail = new IncludedInDetail();
detail.setBranches(includedIn(repo, rw, rev, Constants.R_HEADS));
detail.setTags(includedIn(repo, rw, rev, Constants.R_TAGS));
return detail;
return IncludedInResolver.resolve(repo, rw, rev);
} finally {
rw.release();
}
@@ -104,32 +92,4 @@ class IncludedInDetailFactory extends Handler<IncludedInDetail> {
repo.close();
}
}
private List<String> includedIn(final Repository repo, final RevWalk rw,
final RevCommit rev, final String namespace) throws IOException,
MissingObjectException, IncorrectObjectTypeException {
final List<String> result = new ArrayList<String>();
for (final Ref ref : repo.getRefDatabase().getRefs(namespace).values()) {
final RevCommit tip;
try {
tip = rw.parseCommit(ref.getObjectId());
} catch (IncorrectObjectTypeException notCommit) {
// Its OK for a tag reference to point to a blob or a tree, this
// is common in the Linux kernel or git.git repository.
//
continue;
} catch (MissingObjectException notHere) {
// Log the problem with this branch, but keep processing.
//
log.warn("Reference " + ref.getName() + " in " + repo.getDirectory()
+ " points to dangling object " + ref.getObjectId());
continue;
}
if (rw.isMergedInto(rev, tip)) {
result.add(ref.getName().substring(namespace.length()));
}
}
return result;
}
}
}

159
gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedInResolver.java Normal file
View File

@@ -0,0 +1,159 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import com.google.gerrit.common.data.IncludedInDetail;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
/**
* Resolve in which tags and branches a commit is included.
*/
public class IncludedInResolver {
private static final Logger log = LoggerFactory
.getLogger(IncludedInResolver.class);
public static IncludedInDetail resolve(final Repository repo,
final RevWalk rw, final RevCommit commit) throws IOException {
Set<Ref> tags =
new HashSet<Ref>(repo.getRefDatabase().getRefs(Constants.R_TAGS)
.values());
Set<Ref> branches =
new HashSet<Ref>(repo.getRefDatabase().getRefs(Constants.R_HEADS)
.values());
Set<Ref> allTagsAndBranches = new HashSet<Ref>();
allTagsAndBranches.addAll(tags);
allTagsAndBranches.addAll(branches);
Set<Ref> allMatchingTagsAndBranches =
includedIn(repo, rw, commit, allTagsAndBranches);
IncludedInDetail detail = new IncludedInDetail();
detail
.setBranches(getMatchingRefNames(allMatchingTagsAndBranches, branches));
detail.setTags(getMatchingRefNames(allMatchingTagsAndBranches, tags));
return detail;
}
/**
* Resolves which tip refs include the target commit.
*/
private static Set<Ref> includedIn(final Repository repo, final RevWalk rw,
final RevCommit target, final Set<Ref> tipRefs) throws IOException,
MissingObjectException, IncorrectObjectTypeException {
Set<Ref> result = new HashSet<Ref>();
Map<RevCommit, Set<Ref>> tipsAndCommits = parseCommits(repo, rw, tipRefs);
List<RevCommit> tips = new ArrayList<RevCommit>(tipsAndCommits.keySet());
Collections.sort(tips, new Comparator<RevCommit>() {
@Override
public int compare(RevCommit c1, RevCommit c2) {
return c1.getCommitTime() - c2.getCommitTime();
}
});
Set<RevCommit> targetReachableFrom = new HashSet<RevCommit>();
targetReachableFrom.add(target);
for (RevCommit tip : tips) {
boolean commitFound = false;
rw.resetRetain(RevFlag.UNINTERESTING);
rw.markStart(tip);
for (RevCommit commit : rw) {
if (targetReachableFrom.contains(commit)) {
commitFound = true;
targetReachableFrom.add(tip);
result.addAll(tipsAndCommits.get(tip));
break;
}
}
if (!commitFound) {
rw.markUninteresting(tip);
}
}
return result;
}
/**
* Returns the short names of refs which are as well in the matchingRefs list
* as well as in the allRef list.
*/
private static List<String> getMatchingRefNames(Set<Ref> matchingRefs,
Set<Ref> allRefs) {
List<String> refNames = new ArrayList<String>();
for (Ref matchingRef : matchingRefs) {
if (allRefs.contains(matchingRef)) {
refNames.add(Repository.shortenRefName(matchingRef.getName()));
}
}
return refNames;
}
/**
* Parse commit of ref and store the relation between ref and commit.
*/
private static Map<RevCommit, Set<Ref>> parseCommits(final Repository repo,
final RevWalk rw, final Set<Ref> refs) throws IOException {
Map<RevCommit, Set<Ref>> result = new HashMap<RevCommit, Set<Ref>>();
for (Ref ref : refs) {
final RevCommit commit;
try {
commit = rw.parseCommit(ref.getObjectId());
} catch (IncorrectObjectTypeException notCommit) {
// Its OK for a tag reference to point to a blob or a tree, this
// is common in the Linux kernel or git.git repository.
//
continue;
} catch (MissingObjectException notHere) {
// Log the problem with this branch, but keep processing.
//
log.warn("Reference " + ref.getName() + " in " + repo.getDirectory()
+ " points to dangling object " + ref.getObjectId());
continue;
}
Set<Ref> relatedRefs = result.get(commit);
if (relatedRefs == null) {
relatedRefs = new HashSet<Ref>();
result.put(commit, relatedRefs);
}
relatedRefs.add(ref);
}
return result;
}
}

205
gerrit-server/src/test/java/com/google/gerrit/server/change/IncludedInResolverTest.java Normal file
View File

@@ -0,0 +1,205 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import com.google.gerrit.common.data.IncludedInDetail;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.MergeCommand.FastForwardMode;
import org.eclipse.jgit.junit.RepositoryTestCase;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevTag;
import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public class IncludedInResolverTest extends RepositoryTestCase {
// Branch names
private static final String BRANCH_MASTER = "master";
private static final String BRANCH_1_0 = "rel-1.0";
private static final String BRANCH_1_3 = "rel-1.3";
private static final String BRANCH_2_0 = "rel-2.0";
private static final String BRANCH_2_5 = "rel-2.5";
// Tag names
private static final String TAG_1_0 = "1.0";
private static final String TAG_1_0_1 = "1.0.1";
private static final String TAG_1_3 = "1.3";
private static final String TAG_2_0_1 = "2.0.1";
private static final String TAG_2_0 = "2.0";
private static final String TAG_2_5 = "2.5";
private static final String TAG_2_5_ANNOTATED = "2.5-annotated";
private static final String TAG_2_5_ANNOTATED_TWICE = "2.5-annotated_twice";
// Commits
private RevCommit commit_initial;
private RevCommit commit_v1_3;
private RevCommit commit_v2_5;
private List<String> expTags = new ArrayList<String>();
private List<String> expBranches = new ArrayList<String>();
private RevWalk revWalk;
@Before
public void setUp() throws Exception {
super.setUp();
/*- The following graph will be created.
o tag 2.5, 2.5_annotated, 2.5_annotated_twice
|\
| o tag 2.0.1
| o tag 2.0
o | tag 1.3
|/
o c3
| o tag 1.0.1
|/
o tag 1.0
o c2
o c1
*/
Git git = new Git(db);
revWalk = new RevWalk(db);
// Version 1.0
commit_initial = git.commit().setMessage("c1").call();
git.commit().setMessage("c2").call();
RevCommit commit_v1_0 = git.commit().setMessage("version 1.0").call();
git.tag().setName(TAG_1_0).setObjectId(commit_v1_0).call();
RevCommit c3 = git.commit().setMessage("c3").call();
// Version 1.01
createAndCheckoutBranch(commit_v1_0, BRANCH_1_0);
RevCommit commit_v1_0_1 =
git.commit().setMessage("verREFS_HEADS_RELsion 1.0.1").call();
git.tag().setName(TAG_1_0_1).setObjectId(commit_v1_0_1).call();
// Version 1.3
createAndCheckoutBranch(c3, BRANCH_1_3);
commit_v1_3 = git.commit().setMessage("version 1.3").call();
git.tag().setName(TAG_1_3).setObjectId(commit_v1_3).call();
// Version 2.0
createAndCheckoutBranch(c3, BRANCH_2_0);
RevCommit commit_v2_0 = git.commit().setMessage("version 2.0").call();
git.tag().setName(TAG_2_0).setObjectId(commit_v2_0).call();
RevCommit commit_v2_0_1 = git.commit().setMessage("version 2.0.1").call();
git.tag().setName(TAG_2_0_1).setObjectId(commit_v2_0_1).call();
// Version 2.5
createAndCheckoutBranch(commit_v1_3, BRANCH_2_5);
git.merge().include(commit_v2_0_1).setCommit(false)
.setFastForward(FastForwardMode.NO_FF).call();
commit_v2_5 = git.commit().setMessage("version 2.5").call();
git.tag().setName(TAG_2_5).setObjectId(commit_v2_5).setAnnotated(false)
.call();
Ref ref_tag_2_5_annotated =
git.tag().setName(TAG_2_5_ANNOTATED).setObjectId(commit_v2_5)
.setAnnotated(true).call();
RevTag tag_2_5_annotated =
revWalk.parseTag(ref_tag_2_5_annotated.getObjectId());
git.tag().setName(TAG_2_5_ANNOTATED_TWICE).setObjectId(tag_2_5_annotated)
.setAnnotated(true).call();
}
@After
public void tearDown() throws Exception {
revWalk.release();
super.tearDown();
}
@Test
public void resolveLatestCommit() throws Exception {
// Check tip commit
IncludedInDetail detail = resolve(commit_v2_5);
// Check that only tags and branches which refer the tip are returned
expTags.add(TAG_2_5);
expTags.add(TAG_2_5_ANNOTATED);
expTags.add(TAG_2_5_ANNOTATED_TWICE);
assertEquals(expTags, detail.getTags());
expBranches.add(BRANCH_2_5);
assertEquals(expBranches, detail.getBranches());
}
@Test
public void resolveFirstCommit() throws Exception {
// Check first commit
IncludedInDetail detail = resolve(commit_initial);
// Check whether all tags and branches are returned
expTags.add(TAG_1_0);
expTags.add(TAG_1_0_1);
expTags.add(TAG_1_3);
expTags.add(TAG_2_0);
expTags.add(TAG_2_0_1);
expTags.add(TAG_2_5);
expTags.add(TAG_2_5_ANNOTATED);
expTags.add(TAG_2_5_ANNOTATED_TWICE);
assertEquals(expTags, detail.getTags());
expBranches.add(BRANCH_MASTER);
expBranches.add(BRANCH_1_0);
expBranches.add(BRANCH_1_3);
expBranches.add(BRANCH_2_0);
expBranches.add(BRANCH_2_5);
assertEquals(expBranches, detail.getBranches());
}
@Test
public void resolveBetwixtCommit() throws Exception {
// Check a commit somewhere in the middle
IncludedInDetail detail = resolve(commit_v1_3);
// Check whether all succeeding tags and branches are returned
expTags.add(TAG_1_3);
expTags.add(TAG_2_5);
expTags.add(TAG_2_5_ANNOTATED);
expTags.add(TAG_2_5_ANNOTATED_TWICE);
assertEquals(expTags, detail.getTags());
expBranches.add(BRANCH_1_3);
expBranches.add(BRANCH_2_5);
assertEquals(expBranches, detail.getBranches());
}
private IncludedInDetail resolve(RevCommit commit) throws Exception {
return IncludedInResolver.resolve(db, revWalk, commit);
}
private void assertEquals(List<String> list1, List<String> list2) {
Collections.sort(list1);
Collections.sort(list2);
Assert.assertEquals(list1, list2);
}
private void createAndCheckoutBranch(ObjectId objectId, String branchName)
throws IOException {
String fullBranchName = "refs/heads/" + branchName;
super.createBranch(objectId, fullBranchName);
super.checkoutBranch(fullBranchName);
}
}