Browse Source

blame: Revert common subtree elimination "optimization"

This partially reverts 6de12836d7.

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
stable-3.4
Shawn Pearce 11 years ago committed by Robin Rosenberg
parent
commit
6d724dcd33
  1. 38
      org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java
  2. 7
      org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java

38
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.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk; 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.PathFilter;
import org.eclipse.jgit.treewalk.filter.TreeFilter; import org.eclipse.jgit.treewalk.filter.TreeFilter;
@ -601,16 +600,7 @@ public class BlameGenerator {
return split(n.getNextCandidate(0), n); return split(n.getNextCandidate(0), n);
revPool.parseHeaders(parent); revPool.parseHeaders(parent);
if (n.sourceCommit != null && n.recursivePath) { if (find(parent, n.sourcePath)) {
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 (idBuf.equals(n.sourceBlob)) if (idBuf.equals(n.sourceBlob))
return blameEntireRegionOnParent(n, parent); return blameEntireRegionOnParent(n, parent);
return splitBlameWithParent(n, parent); return splitBlameWithParent(n, parent);
@ -627,7 +617,7 @@ public class BlameGenerator {
// A 100% rename without any content change can also // A 100% rename without any content change can also
// skip directly to the parent. // skip directly to the parent.
n.sourceCommit = parent; n.sourceCommit = parent;
n.setSourcePath(PathFilter.create(r.getOldPath())); n.sourcePath = PathFilter.create(r.getOldPath());
push(n); push(n);
return false; return false;
} }
@ -720,7 +710,7 @@ public class BlameGenerator {
// have an exact content match. For performance reasons // have an exact content match. For performance reasons
// we choose to follow the one parent over trying to do // we choose to follow the one parent over trying to do
// possibly both parents. // possibly both parents.
n.setSourcePath(PathFilter.create(r.getOldPath())); n.sourcePath = PathFilter.create(r.getOldPath());
return blameEntireRegionOnParent(n, parent); return blameEntireRegionOnParent(n, parent);
} }
@ -988,26 +978,4 @@ public class BlameGenerator {
return ent.getChangeType() == ChangeType.RENAME return ent.getChangeType() == ChangeType.RENAME
|| ent.getChangeType() == ChangeType.COPY; || 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$
}
};
} }

7
org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java

@ -82,7 +82,6 @@ class Candidate {
/** Path of the candidate file in {@link #sourceCommit}. */ /** Path of the candidate file in {@link #sourceCommit}. */
PathFilter sourcePath; PathFilter sourcePath;
boolean recursivePath;
/** Unique name of the candidate blob in {@link #sourceCommit}. */ /** Unique name of the candidate blob in {@link #sourceCommit}. */
ObjectId sourceBlob; ObjectId sourceBlob;
@ -113,7 +112,6 @@ class Candidate {
Candidate(RevCommit commit, PathFilter path) { Candidate(RevCommit commit, PathFilter path) {
sourceCommit = commit; sourceCommit = commit;
sourcePath = path; sourcePath = path;
recursivePath = path.shouldBeRecursive();
} }
void beginResult(RevWalk rw) throws MissingObjectException, IOException { void beginResult(RevWalk rw) throws MissingObjectException, IOException {
@ -152,11 +150,6 @@ class Candidate {
return sourceCommit.getAuthorIdent(); return sourceCommit.getAuthorIdent();
} }
void setSourcePath(PathFilter path) {
sourcePath = path;
recursivePath = path.shouldBeRecursive();
}
Candidate create(RevCommit commit, PathFilter path) { Candidate create(RevCommit commit, PathFilter path) {
return new Candidate(commit, path); return new Candidate(commit, path);
} }

Loading…
Cancel
Save