RewriteGenerator: Fully buffering of input is no longer necessary

Fully buffering by the previous generator of the input for the RewriteGenerator is no longer necessary.

Bug: 577948
Signed-off-by: Simon Sohrt <sohrt@his.de>
Change-Id: I59c7a7c7f3766e97627764608bc8dc733804274c

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java

@@ -143,8 +143,19 @@ public class RevWalk implements Iterable<RevCommit>, AutoCloseable {
 	 */
 	static final int TOPO_QUEUED = 1 << 6;
 
+	/**
+	 * Set on a RevCommit when a {@link TreeRevFilter} has been applied.
+	 * <p>
+	 * This flag is processed by the {@link RewriteGenerator} to check if a
+	 * {@link TreeRevFilter} has been applied.
+	 *
+	 * @see TreeRevFilter
+	 * @see RewriteGenerator
+	 */
+	static final int TREE_REV_FILTER_APPLIED = 1 << 7;
+
 	/** Number of flag bits we keep internal for our own use. See above flags. */
-	static final int RESERVED_FLAGS = 7;
+	static final int RESERVED_FLAGS = 8;
 
 	private static final int APP_FLAGS = -1 & ~((1 << RESERVED_FLAGS) - 1);
 

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java

@@ -24,14 +24,7 @@
  * commit that matched the revision walker's filters.
  * <p>
  * This generator is the second phase of a path limited revision walk and
- * assumes it is receiving RevCommits from {@link TreeRevFilter},
- * after they have been fully buffered by {@link AbstractRevQueue}. The full
- * buffering is necessary to allow the simple loop used within our own
- * {@link #rewrite(RevCommit)} to pull completely through a strand of
- * {@link RevWalk#REWRITE} colored commits and come up with a simplification
- * that makes the DAG dense. Not fully buffering the commits first would cause
- * this loop to abort early, due to commits not being parsed and colored
- * correctly.
+ * assumes it is receiving RevCommits from {@link TreeRevFilter}.
  *
  * @see TreeRevFilter
  */
@@ -43,9 +36,12 @@ class RewriteGenerator extends Generator {
 
 	private final Generator source;
 
+	private final FIFORevQueue pending;
+
 	RewriteGenerator(Generator s) {
 		super(s.firstParent);
 		source = s;
+		pending = new FIFORevQueue(s.firstParent);
 	}
 
 	@Override
@@ -62,10 +58,19 @@ int outputType() {
 	@Override
 	RevCommit next() throws MissingObjectException,
 			IncorrectObjectTypeException, IOException {
-		final RevCommit c = source.next();
+		RevCommit c = pending.next();
+
 		if (c == null) {
-			return null;
+			c = source.next();
+			if (c == null) {
+				// We are done: Both the source generator and our internal list
+				// are completely exhausted.
+				return null;
+			}
 		}
+
+		applyFilterToParents(c);
+
 		boolean rewrote = false;
 		final RevCommit[] pList = c.parents;
 		final int nParents = pList.length;
@@ -91,10 +96,41 @@ RevCommit next() throws MissingObjectException,
 		return c;
 	}
 
-	private RevCommit rewrite(RevCommit p) {
+	/**
+	 * Makes sure that the {@link TreeRevFilter} has been applied to all parents
+	 * of this commit by the previous {@link PendingGenerator}.
+	 *
+	 * @param c
+	 * @throws MissingObjectException
+	 * @throws IncorrectObjectTypeException
+	 * @throws IOException
+	 */
+	private void applyFilterToParents(RevCommit c)
+			throws MissingObjectException, IncorrectObjectTypeException,
+			IOException {
+		for (RevCommit parent : c.parents) {
+			while ((parent.flags & RevWalk.TREE_REV_FILTER_APPLIED) == 0) {
+
+				RevCommit n = source.next();
+
+				if (n != null) {
+					pending.add(n);
+				} else {
+					// Source generator is exhausted; filter has been applied to
+					// all commits
+					return;
+				}
+
+			}
+
+		}
+	}
+
+	private RevCommit rewrite(RevCommit p) throws MissingObjectException,
+			IncorrectObjectTypeException, IOException {
 		for (;;) {
-			final RevCommit[] pList = p.parents;
-			if (pList.length > 1) {
+
+			if (p.parents.length > 1) {
 				// This parent is a merge, so keep it.
 				//
 				return p;
@@ -114,14 +150,16 @@ private RevCommit rewrite(RevCommit p) {
 				return p;
 			}
 
-			if (pList.length == 0) {
+			if (p.parents.length == 0) {
 				// We can't go back any further, other than to
 				// just delete the parent entirely.
 				//
 				return null;
 			}
 
-			p = pList[0];
+			applyFilterToParents(p.parents[0]);
+			p = p.parents[0];
+
 		}
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java

@@ -125,12 +125,6 @@ RevCommit next() throws MissingObjectException,
 		}
 
 		if ((g.outputType() & NEEDS_REWRITE) != 0) {
-			// Correction for an upstream NEEDS_REWRITE is to buffer
-			// fully and then apply a rewrite generator that can
-			// pull through the rewrite chain and produce a dense
-			// output graph.
-			//
-			g = new FIFORevQueue(g);
 			g = new RewriteGenerator(g);
 		}
 

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java

@@ -41,6 +41,8 @@ public class TreeRevFilter extends RevFilter {
 
 	private static final int UNINTERESTING = RevWalk.UNINTERESTING;
 
+	private static final int FILTER_APPLIED = RevWalk.TREE_REV_FILTER_APPLIED;
+
 	private final int rewriteFlag;
 	private final TreeWalk pathFilter;
 
@@ -101,6 +103,7 @@ public RevFilter clone() {
 	public boolean include(RevWalk walker, RevCommit c)
 			throws StopWalkException, MissingObjectException,
 			IncorrectObjectTypeException, IOException {
+		c.flags |= FILTER_APPLIED;
 		// Reset the tree filter to scan this commit and parents.
 		//
 		RevCommit[] pList = c.parents;