From ca62b3447befebfdab8503807d6441839bceb47d Mon Sep 17 00:00:00 2001 From: Simon Sohrt Date: Mon, 4 Apr 2022 10:10:19 +0200 Subject: [PATCH] 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 Change-Id: I59c7a7c7f3766e97627764608bc8dc733804274c --- .../src/org/eclipse/jgit/revwalk/RevWalk.java | 13 +++- .../jgit/revwalk/RewriteGenerator.java | 68 +++++++++++++++---- .../eclipse/jgit/revwalk/StartGenerator.java | 6 -- .../eclipse/jgit/revwalk/TreeRevFilter.java | 3 + 4 files changed, 68 insertions(+), 22 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java index a50eaf1a8..a25948e50 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java @@ -143,8 +143,19 @@ public class RevWalk implements Iterable, AutoCloseable { */ static final int TOPO_QUEUED = 1 << 6; + /** + * Set on a RevCommit when a {@link TreeRevFilter} has been applied. + *

+ * 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); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java index 4565a4b5d..1adef07ad 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java @@ -24,14 +24,7 @@ * commit that matched the revision walker's filters. *

* 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]; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java index bfcea6ea8..a79901ca1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java +++ b/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); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java index 822fc5320..92d72268d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java +++ b/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;