From 6d724dcd3355f09e3450e417cf173fcafaee9e08 Mon Sep 17 00:00:00 2001
From: Shawn Pearce <spearce@spearce.org>
Date: Sat, 26 Apr 2014 10:40:30 -0700
Subject: [PATCH] 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
---
 .../eclipse/jgit/blame/BlameGenerator.java    | 38 ++-----------------
 .../src/org/eclipse/jgit/blame/Candidate.java |  7 ----
 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 a54ef5669..8961537ce 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 c7c34f0a3..72d8abe1f 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);
 	}