RevWalk: Avoid unnecessary re-parsing of commit bodies

If the RevFilter doesn't actually require the commit body,
we shouldn't reparse it if the body was disposed.  This happens
often inside of UploadPack during common ancestor negotation, the
RevWalk is reset and re-run over roughly the same commit space,
but the bodies are discarded because the commit message is not
relevant to the process.

Change-Id: I87b6b6a5fb269669867047698abf718d366bd002
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

13 files changed, 121 insertions, 2 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkFilterTest.java
index dd67a9833a..643ba26d9a 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkFilterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkFilterTest.java
@@ -312,5 +312,10 @@ public class RevWalkFilterTest extends RevWalkTestCase {
 				IncorrectObjectTypeException, IOException {
 			return true;
 		}
+
+		@Override
+		public boolean requiresCommitBody() {
+			return false;
+		}
 	}
 }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkResetTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkResetTest.java
index 27bd1a91ae..7cc5a32726 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkResetTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkResetTest.java
@@ -81,6 +81,10 @@ public class RevWalkResetTest extends RevWalkTestCase {
 				return this;
 			}
 
+			@Override
+			public boolean requiresCommitBody() {
+				return true;
+			}
 		};
 
 		// Do an initial run through the walk
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java
index f1624070df..f24c27873f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java
@@ -136,7 +136,8 @@ class PendingGenerator extends Generator {
 				if ((c.flags & UNINTERESTING) != 0)
 					produce = false;
 				else {
-					c.parseBody(walker);
+					if (filter.requiresCommitBody())
+						c.parseBody(walker);
 					produce = filter.include(walker, c);
 				}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteTreeFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteTreeFilter.java
index 41cfcf8691..dc7338aa8c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteTreeFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteTreeFilter.java
@@ -230,6 +230,11 @@ class RewriteTreeFilter extends RevFilter {
 		return false;
 	}
 
+	@Override
+	public boolean requiresCommitBody() {
+		return false;
+	}
+
 	private void updateFollowFilter(ObjectId[] trees)
 			throws MissingObjectException, IncorrectObjectTypeException,
 			CorruptObjectException, IOException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/AndRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/AndRevFilter.java
index d4790f870c..831051263a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/AndRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/AndRevFilter.java
@@ -120,9 +120,13 @@ public abstract class AndRevFilter extends RevFilter {
 
 		private final RevFilter b;
 
+		private final boolean requiresCommitBody;
+
 		Binary(final RevFilter one, final RevFilter two) {
 			a = one;
 			b = two;
+			requiresCommitBody = a.requiresCommitBody()
+					|| b.requiresCommitBody();
 		}
 
 		@Override
@@ -133,6 +137,11 @@ public abstract class AndRevFilter extends RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return requiresCommitBody;
+		}
+
+		@Override
 		public RevFilter clone() {
 			return new Binary(a.clone(), b.clone());
 		}
@@ -146,8 +155,15 @@ public abstract class AndRevFilter extends RevFilter {
 	private static class List extends AndRevFilter {
 		private final RevFilter[] subfilters;
 
+		private final boolean requiresCommitBody;
+
 		List(final RevFilter[] list) {
 			subfilters = list;
+
+			boolean rcb = false;
+			for (RevFilter filter : subfilters)
+				rcb |= filter.requiresCommitBody();
+			requiresCommitBody = rcb;
 		}
 
 		@Override
@@ -162,6 +178,11 @@ public abstract class AndRevFilter extends RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return requiresCommitBody;
+		}
+
+		@Override
 		public RevFilter clone() {
 			final RevFilter[] s = new RevFilter[subfilters.length];
 			for (int i = 0; i < s.length; i++)
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/CommitTimeRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/CommitTimeRevFilter.java
index 858bec4c00..14c03a4bcd 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/CommitTimeRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/CommitTimeRevFilter.java
@@ -134,6 +134,11 @@ public abstract class CommitTimeRevFilter extends RevFilter {
 		return this;
 	}
 
+	@Override
+	public boolean requiresCommitBody() {
+		return false;
+	}
+
 	private static class Before extends CommitTimeRevFilter {
 		Before(final long ts) {
 			super(ts);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/NotRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/NotRevFilter.java
index 117378c9fa..6d1a49cb58 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/NotRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/NotRevFilter.java
@@ -82,6 +82,11 @@ public class NotRevFilter extends RevFilter {
 	}
 
 	@Override
+	public boolean requiresCommitBody() {
+		return a.requiresCommitBody();
+	}
+
+	@Override
 	public RevFilter clone() {
 		return new NotRevFilter(a.clone());
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/OrRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/OrRevFilter.java
index 586c138fff..b35711f5aa 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/OrRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/OrRevFilter.java
@@ -118,9 +118,13 @@ public abstract class OrRevFilter extends RevFilter {
 
 		private final RevFilter b;
 
+		private final boolean requiresCommitBody;
+
 		Binary(final RevFilter one, final RevFilter two) {
 			a = one;
 			b = two;
+			requiresCommitBody = a.requiresCommitBody()
+					|| b.requiresCommitBody();
 		}
 
 		@Override
@@ -131,6 +135,11 @@ public abstract class OrRevFilter extends RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return requiresCommitBody;
+		}
+
+		@Override
 		public RevFilter clone() {
 			return new Binary(a.clone(), b.clone());
 		}
@@ -144,8 +153,15 @@ public abstract class OrRevFilter extends RevFilter {
 	private static class List extends OrRevFilter {
 		private final RevFilter[] subfilters;
 
+		private final boolean requiresCommitBody;
+
 		List(final RevFilter[] list) {
 			subfilters = list;
+
+			boolean rcb = false;
+			for (RevFilter filter : subfilters)
+				rcb |= filter.requiresCommitBody();
+			requiresCommitBody = rcb;
 		}
 
 		@Override
@@ -160,6 +176,11 @@ public abstract class OrRevFilter extends RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return requiresCommitBody;
+		}
+
+		@Override
 		public RevFilter clone() {
 			final RevFilter[] s = new RevFilter[subfilters.length];
 			for (int i = 0; i < s.length; i++)
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/PatternMatchRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/PatternMatchRevFilter.java
index c2a287958a..feb5dd1a13 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/PatternMatchRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/PatternMatchRevFilter.java
@@ -131,6 +131,11 @@ public abstract class PatternMatchRevFilter extends RevFilter {
 		return compiledPattern.reset(text(cmit)).matches();
 	}
 
+	@Override
+	public boolean requiresCommitBody() {
+		return true;
+	}
+
 	/**
 	 * Obtain the raw text to match against.
 	 *
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFilter.java
index 8b45a312e1..c3f71feafa 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFilter.java
@@ -108,6 +108,11 @@ public abstract class RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return false;
+		}
+
+		@Override
 		public String toString() {
 			return "ALL";
 		}
@@ -128,6 +133,11 @@ public abstract class RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return false;
+		}
+
+		@Override
 		public String toString() {
 			return "NONE";
 		}
@@ -148,6 +158,11 @@ public abstract class RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return false;
+		}
+
+		@Override
 		public String toString() {
 			return "NO_MERGES";
 		}
@@ -175,6 +190,11 @@ public abstract class RevFilter {
 		}
 
 		@Override
+		public boolean requiresCommitBody() {
+			return false;
+		}
+
+		@Override
 		public String toString() {
 			return "MERGE_BASE";
 		}
@@ -189,6 +209,12 @@ public abstract class RevFilter {
 		return NotRevFilter.create(this);
 	}
 
+	/** @return true if the filter needs the commit body to be parsed. */
+	public boolean requiresCommitBody() {
+		// Assume true to be backward compatible with prior behavior.
+		return true;
+	}
+
 	/**
 	 * Determine if the supplied commit should be included in results.
 	 *
@@ -196,7 +222,8 @@ public abstract class RevFilter {
 	 *            the active walker this filter is being invoked from within.
 	 * @param cmit
 	 *            the commit currently being tested. The commit has been parsed
-	 *            and its body is available for inspection.
+	 *            and its body is available for inspection only if the filter
+	 *            returns true from {@link #requiresCommitBody()}.
 	 * @return true to include this commit in the results; false to have this
 	 *         commit be omitted entirely from the results.
 	 * @throws StopWalkException
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFlagFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFlagFilter.java
index 2faf90ca8e..1fbf74617e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFlagFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/RevFlagFilter.java
@@ -144,6 +144,11 @@ public abstract class RevFlagFilter extends RevFilter {
 				IOException {
 			return c.hasAll(flags);
 		}
+
+		@Override
+		public boolean requiresCommitBody() {
+			return false;
+		}
 	}
 
 	private static class HasAny extends RevFlagFilter {
@@ -157,5 +162,10 @@ public abstract class RevFlagFilter extends RevFilter {
 				IOException {
 			return c.hasAny(flags);
 		}
+
+		@Override
+		public boolean requiresCommitBody() {
+			return false;
+		}
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/SubStringRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/SubStringRevFilter.java
index f8e34a9239..20d867fd58 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/SubStringRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/SubStringRevFilter.java
@@ -104,6 +104,11 @@ public abstract class SubStringRevFilter extends RevFilter {
 		return pattern.match(text(cmit)) >= 0;
 	}
 
+	@Override
+	public boolean requiresCommitBody() {
+		return true;
+	}
+
 	/**
 	 * Obtain the raw text to match against.
 	 *
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
index 2653bad552..4f3d1bb49d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
@@ -599,6 +599,11 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 				}
 				return !remoteKnowsIsCommon;
 			}
+
+			@Override
+			public boolean requiresCommitBody() {
+				return false;
+			}
 		});
 	}