aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn Pearce <spearce@spearce.org>2014-04-26 10:40:30 -0700
committerRobin Rosenberg <robin.rosenberg@dewire.com>2014-05-19 15:45:43 -0400
commit6d724dcd3355f09e3450e417cf173fcafaee9e08 (patch)
tree3960b2d01016111e9d443f5e151f0cc0e61f6bc2
parenta5ee6fe904d6eb75a0edf6b3e93d7bfd733b78fe (diff)
downloadjgit-6d724dcd3355f09e3450e417cf173fcafaee9e08.tar.gz
jgit-6d724dcd3355f09e3450e417cf173fcafaee9e08.zip
blame: Revert common subtree elimination "optimization"
This partially reverts 6de12836d72fe4cba9afc297c8988c6fff851fa9. Performing a TreeWalk over 2 trees to identify and skip unmodified subtrees to pass all blame onto an ancestor appears to be a micro optimization that works for a very limited number of files. In the general case the 2 tree walk is slowing down blame more than it helps to speed it up. I keep coming up with files in multiple repositories where 6de128 is making things worse, not better, and only one example where it actually improved performance, render_view_impl.cc in chromium as described in the commit message. Change-Id: Ic6d5fff22acb5ab6485614a07bdb388e8c336679
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java38
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java7
2 files changed, 3 insertions, 42 deletions
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java
index a54ef56697..8961537ce5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java
@@ -73,7 +73,6 @@ import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
-import org.eclipse.jgit.treewalk.filter.AndTreeFilter;
import org.eclipse.jgit.treewalk.filter.PathFilter;
import org.eclipse.jgit.treewalk.filter.TreeFilter;
@@ -601,16 +600,7 @@ public class BlameGenerator {
return split(n.getNextCandidate(0), n);
revPool.parseHeaders(parent);
- if (n.sourceCommit != null && n.recursivePath) {
- treeWalk.setFilter(AndTreeFilter.create(n.sourcePath, ID_DIFF));
- treeWalk.reset(n.sourceCommit.getTree(), parent.getTree());
- if (!treeWalk.next())
- return blameEntireRegionOnParent(n, parent);
- if (isFile(treeWalk.getRawMode(1))) {
- treeWalk.getObjectId(idBuf, 1);
- return splitBlameWithParent(n, parent);
- }
- } else if (find(parent, n.sourcePath)) {
+ if (find(parent, n.sourcePath)) {
if (idBuf.equals(n.sourceBlob))
return blameEntireRegionOnParent(n, parent);
return splitBlameWithParent(n, parent);
@@ -627,7 +617,7 @@ public class BlameGenerator {
// A 100% rename without any content change can also
// skip directly to the parent.
n.sourceCommit = parent;
- n.setSourcePath(PathFilter.create(r.getOldPath()));
+ n.sourcePath = PathFilter.create(r.getOldPath());
push(n);
return false;
}
@@ -720,7 +710,7 @@ public class BlameGenerator {
// have an exact content match. For performance reasons
// we choose to follow the one parent over trying to do
// possibly both parents.
- n.setSourcePath(PathFilter.create(r.getOldPath()));
+ n.sourcePath = PathFilter.create(r.getOldPath());
return blameEntireRegionOnParent(n, parent);
}
@@ -988,26 +978,4 @@ public class BlameGenerator {
return ent.getChangeType() == ChangeType.RENAME
|| ent.getChangeType() == ChangeType.COPY;
}
-
- private static final TreeFilter ID_DIFF = new TreeFilter() {
- @Override
- public boolean include(TreeWalk tw) {
- return !tw.idEqual(0, 1);
- }
-
- @Override
- public boolean shouldBeRecursive() {
- return false;
- }
-
- @Override
- public TreeFilter clone() {
- return this;
- }
-
- @Override
- public String toString() {
- return "ID_DIFF"; //$NON-NLS-1$
- }
- };
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java
index c7c34f0a3a..72d8abe1fc 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java
@@ -82,7 +82,6 @@ class Candidate {
/** Path of the candidate file in {@link #sourceCommit}. */
PathFilter sourcePath;
- boolean recursivePath;
/** Unique name of the candidate blob in {@link #sourceCommit}. */
ObjectId sourceBlob;
@@ -113,7 +112,6 @@ class Candidate {
Candidate(RevCommit commit, PathFilter path) {
sourceCommit = commit;
sourcePath = path;
- recursivePath = path.shouldBeRecursive();
}
void beginResult(RevWalk rw) throws MissingObjectException, IOException {
@@ -152,11 +150,6 @@ class Candidate {
return sourceCommit.getAuthorIdent();
}
- void setSourcePath(PathFilter path) {
- sourcePath = path;
- recursivePath = path.shouldBeRecursive();
- }
-
Candidate create(RevCommit commit, PathFilter path) {
return new Candidate(commit, path);
}