From 24c1c530db67d56cfad9713ac59354d067421231 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 7 Feb 2011 16:30:49 -0800 Subject: [PATCH] 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 --- .../jgit/revwalk/RevWalkFilterTest.java | 5 ++++ .../jgit/revwalk/RevWalkResetTest.java | 4 +++ .../jgit/revwalk/PendingGenerator.java | 3 +- .../jgit/revwalk/RewriteTreeFilter.java | 5 ++++ .../jgit/revwalk/filter/AndRevFilter.java | 21 ++++++++++++++ .../revwalk/filter/CommitTimeRevFilter.java | 5 ++++ .../jgit/revwalk/filter/NotRevFilter.java | 5 ++++ .../jgit/revwalk/filter/OrRevFilter.java | 21 ++++++++++++++ .../revwalk/filter/PatternMatchRevFilter.java | 5 ++++ .../jgit/revwalk/filter/RevFilter.java | 29 ++++++++++++++++++- .../jgit/revwalk/filter/RevFlagFilter.java | 10 +++++++ .../revwalk/filter/SubStringRevFilter.java | 5 ++++ .../transport/BasePackFetchConnection.java | 5 ++++ 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 dd67a9833..643ba26d9 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 27bd1a91a..7cc5a3272 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 f1624070d..f24c27873 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 41cfcf869..dc7338aa8 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 d4790f870..831051263 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 @@ -132,6 +136,11 @@ public abstract class AndRevFilter extends RevFilter { return a.include(walker, c) && b.include(walker, c); } + @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 @@ -161,6 +177,11 @@ public abstract class AndRevFilter extends RevFilter { return true; } + @Override + public boolean requiresCommitBody() { + return requiresCommitBody; + } + @Override public RevFilter clone() { final RevFilter[] s = new RevFilter[subfilters.length]; 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 858bec4c0..14c03a4bc 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 117378c9f..6d1a49cb5 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 @@ -81,6 +81,11 @@ public class NotRevFilter extends RevFilter { return !a.include(walker, c); } + @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 586c138ff..b35711f5a 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 @@ -130,6 +134,11 @@ public abstract class OrRevFilter extends RevFilter { return a.include(walker, c) || b.include(walker, c); } + @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 @@ -159,6 +175,11 @@ public abstract class OrRevFilter extends RevFilter { return false; } + @Override + public boolean requiresCommitBody() { + return requiresCommitBody; + } + @Override public RevFilter clone() { final RevFilter[] s = new RevFilter[subfilters.length]; 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 c2a287958..feb5dd1a1 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 8b45a312e..c3f71feaf 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 @@ -107,6 +107,11 @@ public abstract class RevFilter { return this; } + @Override + public boolean requiresCommitBody() { + return false; + } + @Override public String toString() { return "ALL"; @@ -127,6 +132,11 @@ public abstract class RevFilter { return this; } + @Override + public boolean requiresCommitBody() { + return false; + } + @Override public String toString() { return "NONE"; @@ -147,6 +157,11 @@ public abstract class RevFilter { return this; } + @Override + public boolean requiresCommitBody() { + return false; + } + @Override public String toString() { return "NO_MERGES"; @@ -174,6 +189,11 @@ public abstract class RevFilter { return this; } + @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 2faf90ca8..1fbf74617 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 f8e34a923..20d867fd5 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 2653bad55..4f3d1bb49 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; + } }); }