{"thread":{"id":"19794","subject":"[JGIT PATCH 0/5] Fix major performance problems in UploadPack","startedAt":"2009-06-12T23:00:14Z","lastAt":"2009-06-12T23:00:19Z","messageCount":6,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"116192","messageId":"1244847619-7364-1-git-send-email-spearce@spearce.org","threadId":"19794","inReplyTo":null,"subject":"[JGIT PATCH 0/5] Fix major performance problems in UploadPack","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-12T23:00:14Z","receivedAt":"2009-06-12T23:00:14Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If the client is really far ahead of the server, JGit's UploadPack\ncan spend a massive amount of CPU time on the server just doing the\nsame work over and over again.  This series addresses the issue by\ntaking advantage of cached data more frequently.\n\nShawn O. Pearce (5):\n  Make RevTag getObject(), getName() final to prevent overrides\n  Allow exceptions to be created with integer type codes\n  Unify RevWalk parsing code to be more consistent across types\n  Change RevObject dispose() semantics to avoid reparses\n  UploadPack: Only recompute okToGiveUp() if bases changed\n\n .../org/spearce/jgit/revwalk/RevWalkTestCase.java  |    2 +-\n .../jgit/errors/IncorrectObjectTypeException.java  |   13 +++\n .../jgit/errors/MissingObjectException.java        |   12 +++\n .../src/org/spearce/jgit/lib/PackWriter.java       |    3 +-\n .../spearce/jgit/revwalk/BoundaryGenerator.java    |    2 +-\n .../spearce/jgit/revwalk/MergeBaseGenerator.java   |    2 +-\n .../src/org/spearce/jgit/revwalk/ObjectWalk.java   |    4 +-\n .../org/spearce/jgit/revwalk/PendingGenerator.java |    6 +-\n .../src/org/spearce/jgit/revwalk/RevBlob.java      |    5 -\n .../src/org/spearce/jgit/revwalk/RevCommit.java    |   29 ++++----\n .../src/org/spearce/jgit/revwalk/RevObject.java    |   34 ++++++--\n .../src/org/spearce/jgit/revwalk/RevTag.java       |   32 ++++----\n .../src/org/spearce/jgit/revwalk/RevTree.java      |    5 -\n .../src/org/spearce/jgit/revwalk/RevWalk.java      |   82 ++++++++++++++-----\n .../spearce/jgit/revwalk/RewriteTreeFilter.java    |    2 +-\n .../src/org/spearce/jgit/transport/UploadPack.java |   31 +++++---\n .../jgit/transport/WalkFetchConnection.java        |   15 +---\n 17 files changed, 179 insertions(+), 100 deletions(-)\n"},{"id":"116193","messageId":"1244847619-7364-2-git-send-email-spearce@spearce.org","threadId":"19794","inReplyTo":"1244847619-7364-1-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 1/5] Make RevTag getObject(), getName() final to prevent overrides","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-12T23:00:15Z","receivedAt":"2009-06-12T23:00:15Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"These methods exist solely to export the object headers from the\ntag buffer.  Overriding them may create confusion by trying to\nreplace the value with something other than what was parsed from\nthe tag headers.  Other methods like getShortMessage() and the\ngetTaggerIdent() are likewise already marked final.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/revwalk/RevTag.java       |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\nindex cace82d..83fd873 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\n@@ -189,7 +189,7 @@ public Tag asTag(final RevWalk walk) {\n \t * \n \t * @return object this tag refers to.\n \t */\n-\tpublic RevObject getObject() {\n+\tpublic final RevObject getObject() {\n \t\treturn object;\n \t}\n \n@@ -198,7 +198,7 @@ public RevObject getObject() {\n \t * \n \t * @return name of the tag, according to the tag header.\n \t */\n-\tpublic String getName() {\n+\tpublic final String getName() {\n \t\treturn name;\n \t}\n \n-- \n1.6.3.2.367.gf0de\n"},{"id":"116195","messageId":"1244847619-7364-3-git-send-email-spearce@spearce.org","threadId":"19794","inReplyTo":"1244847619-7364-2-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 2/5] Allow exceptions to be created with integer type codes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-12T23:00:16Z","receivedAt":"2009-06-12T23:00:16Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"This may be easier in some contexts where we have the type code\nwe expect handy, but not the type string constant.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../jgit/errors/IncorrectObjectTypeException.java  |   13 +++++++++++++\n .../jgit/errors/MissingObjectException.java        |   12 ++++++++++++\n 2 files changed, 25 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/IncorrectObjectTypeException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/IncorrectObjectTypeException.java\nindex a194f19..3af1f44 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/errors/IncorrectObjectTypeException.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/errors/IncorrectObjectTypeException.java\n@@ -40,6 +40,7 @@\n \n import java.io.IOException;\n \n+import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.ObjectId;\n \n /**\n@@ -62,4 +63,16 @@\n \tpublic IncorrectObjectTypeException(final ObjectId id, final String type) {\n \t\tsuper(\"Object \" + id.name() + \" is not a \" + type + \".\");\n \t}\n+\n+\t/**\n+\t * Construct and IncorrectObjectTypeException for the specified object id.\n+\t *\n+\t * Provide the type to make it easier to track down the problem.\n+\t *\n+\t * @param id SHA-1\n+\t * @param type object type\n+\t */\n+\tpublic IncorrectObjectTypeException(final ObjectId id, final int type) {\n+\t\tthis(id, Constants.typeString(type));\n+\t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/MissingObjectException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/MissingObjectException.java\nindex 37147c3..7b7b8f3 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/errors/MissingObjectException.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/errors/MissingObjectException.java\n@@ -40,6 +40,7 @@\n \n import java.io.IOException;\n \n+import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.ObjectId;\n \n /**\n@@ -58,4 +59,15 @@\n \tpublic MissingObjectException(final ObjectId id, final String type) {\n \t\tsuper(\"Missing \" + type + \" \" + id.name());\n \t}\n+\n+\t/**\n+\t * Construct a MissingObjectException for the specified object id.\n+\t * Expected type is reported to simplify tracking down the problem.\n+\t *\n+\t * @param id SHA-1\n+\t * @param type object type\n+\t */\n+\tpublic MissingObjectException(final ObjectId id, final int type) {\n+\t\tthis(id, Constants.typeString(type));\n+\t}\n }\n-- \n1.6.3.2.367.gf0de\n"},{"id":"116194","messageId":"1244847619-7364-4-git-send-email-spearce@spearce.org","threadId":"19794","inReplyTo":"1244847619-7364-3-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 3/5] Unify RevWalk parsing code to be more consistent across types","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-12T23:00:17Z","receivedAt":"2009-06-12T23:00:17Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"The parse() method now verifies the object actually exists, and\nnot just that we have a handle to it (e.g. in the case of a tree\nor a blob type).  This allowed us to push a chunk of the loading\ncode up into the RevObject base class, removing some duplication.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/revwalk/RevBlob.java      |    5 ----\n .../src/org/spearce/jgit/revwalk/RevCommit.java    |   11 +--------\n .../src/org/spearce/jgit/revwalk/RevObject.java    |   23 +++++++++++++++++--\n .../src/org/spearce/jgit/revwalk/RevTag.java       |    9 +-------\n .../src/org/spearce/jgit/revwalk/RevTree.java      |    5 ----\n .../src/org/spearce/jgit/revwalk/RevWalk.java      |   14 ++---------\n 6 files changed, 25 insertions(+), 42 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java\nindex cf241cf..70eeac0 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java\n@@ -53,11 +53,6 @@ protected RevBlob(final AnyObjectId id) {\n \t}\n \n \t@Override\n-\tvoid parse(final RevWalk walk) {\n-\t\tflags |= PARSED;\n-\t}\n-\t\n-\t@Override\n \tpublic final int getType() {\n \t\treturn Constants.OBJ_BLOB;\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java\nindex 2a59ec4..679718e 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java\n@@ -46,7 +46,6 @@\n import org.spearce.jgit.lib.Commit;\n import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.MutableObjectId;\n-import org.spearce.jgit.lib.ObjectLoader;\n import org.spearce.jgit.lib.PersonIdent;\n import org.spearce.jgit.util.RawParseUtils;\n \n@@ -54,8 +53,6 @@\n public class RevCommit extends RevObject {\n \tstatic final RevCommit[] NO_PARENTS = {};\n \n-\tprivate static final String TYPE_COMMIT = Constants.TYPE_COMMIT;\n-\n \tprivate RevTree tree;\n \n \tRevCommit[] parents;\n@@ -79,13 +76,7 @@ protected RevCommit(final AnyObjectId id) {\n \t@Override\n \tvoid parse(final RevWalk walk) throws MissingObjectException,\n \t\t\tIncorrectObjectTypeException, IOException {\n-\t\tfinal ObjectLoader ldr = walk.db.openObject(walk.curs, this);\n-\t\tif (ldr == null)\n-\t\t\tthrow new MissingObjectException(this, TYPE_COMMIT);\n-\t\tfinal byte[] data = ldr.getCachedBytes();\n-\t\tif (Constants.OBJ_COMMIT != ldr.getType())\n-\t\t\tthrow new IncorrectObjectTypeException(this, TYPE_COMMIT);\n-\t\tparseCanonical(walk, data);\n+\t\tparseCanonical(walk, loadCanonical(walk));\n \t}\n \n \tvoid parseCanonical(final RevWalk walk, final byte[] raw) {\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\nindex 1a13d0a..0e0386c 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n@@ -39,11 +39,13 @@\n \n import java.io.IOException;\n \n+import org.spearce.jgit.errors.CorruptObjectException;\n import org.spearce.jgit.errors.IncorrectObjectTypeException;\n import org.spearce.jgit.errors.MissingObjectException;\n import org.spearce.jgit.lib.AnyObjectId;\n import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.ObjectId;\n+import org.spearce.jgit.lib.ObjectLoader;\n \n /** Base object type accessed during revision walking. */\n public abstract class RevObject extends ObjectId {\n@@ -55,9 +57,24 @@ RevObject(final AnyObjectId name) {\n \t\tsuper(name);\n \t}\n \n-\tabstract void parse(RevWalk walk) throws MissingObjectException,\n-\t\t\tIncorrectObjectTypeException, IOException;\n-\t\n+\tvoid parse(final RevWalk walk) throws MissingObjectException,\n+\t\t\tIncorrectObjectTypeException, IOException {\n+\t\tloadCanonical(walk);\n+\t\tflags |= PARSED;\n+\t}\n+\n+\tfinal byte[] loadCanonical(final RevWalk walk) throws IOException,\n+\t\t\tMissingObjectException, IncorrectObjectTypeException,\n+\t\t\tCorruptObjectException {\n+\t\tfinal ObjectLoader ldr = walk.db.openObject(walk.curs, this);\n+\t\tif (ldr == null)\n+\t\t\tthrow new MissingObjectException(this, getType());\n+\t\tfinal byte[] data = ldr.getCachedBytes();\n+\t\tif (getType() != ldr.getType())\n+\t\t\tthrow new IncorrectObjectTypeException(this, getType());\n+\t\treturn data;\n+\t}\n+\n \t/**\n \t * Get Git object type. See {@link Constants}.\n \t * \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\nindex 83fd873..e876025 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\n@@ -45,7 +45,6 @@\n import org.spearce.jgit.errors.MissingObjectException;\n import org.spearce.jgit.lib.AnyObjectId;\n import org.spearce.jgit.lib.Constants;\n-import org.spearce.jgit.lib.ObjectLoader;\n import org.spearce.jgit.lib.PersonIdent;\n import org.spearce.jgit.lib.Tag;\n import org.spearce.jgit.util.MutableInteger;\n@@ -72,13 +71,7 @@ protected RevTag(final AnyObjectId id) {\n \t@Override\n \tvoid parse(final RevWalk walk) throws MissingObjectException,\n \t\t\tIncorrectObjectTypeException, IOException {\n-\t\tfinal ObjectLoader ldr = walk.db.openObject(walk.curs, this);\n-\t\tif (ldr == null)\n-\t\t\tthrow new MissingObjectException(this, Constants.TYPE_TAG);\n-\t\tfinal byte[] data = ldr.getCachedBytes();\n-\t\tif (Constants.OBJ_TAG != ldr.getType())\n-\t\t\tthrow new IncorrectObjectTypeException(this, Constants.TYPE_TAG);\n-\t\tparseCanonical(walk, data);\n+\t\tparseCanonical(walk, loadCanonical(walk));\n \t}\n \n \tvoid parseCanonical(final RevWalk walk, final byte[] rawTag)\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java\nindex 4d767e4..e9f5d91 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java\n@@ -53,11 +53,6 @@ protected RevTree(final AnyObjectId id) {\n \t}\n \n \t@Override\n-\tvoid parse(final RevWalk walk) {\n-\t\tflags |= PARSED;\n-\t}\n-\t\n-\t@Override\n \tpublic final int getType() {\n \t\treturn Constants.OBJ_TREE;\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\nindex f567a33..40bdb4e 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n@@ -667,15 +667,7 @@ else if (!(c instanceof RevTree))\n \t\t\t\t\tConstants.TYPE_TREE);\n \t\telse\n \t\t\tt = (RevTree) c;\n-\n-\t\tif ((t.flags & PARSED) != 0)\n-\t\t\treturn t;\n-\t\tfinal ObjectLoader ldr = db.openObject(curs, t);\n-\t\tif (ldr == null)\n-\t\t\tthrow new MissingObjectException(t, Constants.TYPE_TREE);\n-\t\tif (ldr.getType() != Constants.OBJ_TREE)\n-\t\t\tthrow new IncorrectObjectTypeException(t, Constants.TYPE_TREE);\n-\t\tt.flags |= PARSED;\n+\t\tparse(t);\n \t\treturn t;\n \t}\n \n@@ -731,8 +723,8 @@ public RevObject parseAny(final AnyObjectId id)\n \t\t\t\tthrow new IllegalArgumentException(\"Bad object type: \" + type);\n \t\t\t}\n \t\t\tobjects.add(r);\n-\t\t} else if ((r.flags & PARSED) == 0)\n-\t\t\tr.parse(this);\n+\t\t} else\n+\t\t\tparse(r);\n \t\treturn r;\n \t}\n \n-- \n1.6.3.2.367.gf0de\n"},{"id":"116196","messageId":"1244847619-7364-5-git-send-email-spearce@spearce.org","threadId":"19794","inReplyTo":"1244847619-7364-4-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 4/5] Change RevObject dispose() semantics to avoid reparses","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-12T23:00:18Z","receivedAt":"2009-06-12T23:00:18Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"UploadPack was suffering from performance problems because dispose()\nwas unsetting the PARSED flag, causing another visit to a commit to\nreparse the object from disk, even though we already had its parents\nin memory.  If a client was 50,000 commits ahead of the server,\nthen the client could send 50,000 have requests, each time the\nserver is executing a full reparse of its own commit graph trying\nto determine if the common set is sufficient yet.\n\nInstead of asking applications to dispose of commit buffers when the\ncommit is popped out of the RevWalk we now dispose of the buffer as\nsoon as the headers are parsed.  For applications that never need\nthe buffer but have to process a lot of commits in the pending queue,\nthis can reduce memory usage by a fair amount, as the pending queue\nno longer has to retain the buffers until the commits are returned\nto the application for buffer disposal.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../org/spearce/jgit/revwalk/RevWalkTestCase.java  |    2 +-\n .../src/org/spearce/jgit/lib/PackWriter.java       |    3 +-\n .../spearce/jgit/revwalk/BoundaryGenerator.java    |    2 +-\n .../spearce/jgit/revwalk/MergeBaseGenerator.java   |    2 +-\n .../src/org/spearce/jgit/revwalk/ObjectWalk.java   |    4 +-\n .../org/spearce/jgit/revwalk/PendingGenerator.java |    6 +-\n .../src/org/spearce/jgit/revwalk/RevCommit.java    |   18 ++++-\n .../src/org/spearce/jgit/revwalk/RevObject.java    |   13 ++--\n .../src/org/spearce/jgit/revwalk/RevTag.java       |   19 ++++-\n .../src/org/spearce/jgit/revwalk/RevWalk.java      |   72 ++++++++++++++++----\n .../spearce/jgit/revwalk/RewriteTreeFilter.java    |    2 +-\n .../src/org/spearce/jgit/transport/UploadPack.java |    4 +-\n .../jgit/transport/WalkFetchConnection.java        |   15 +---\n 13 files changed, 111 insertions(+), 51 deletions(-)\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java\nindex 0a1e7b8..befc3d5 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java\n@@ -160,7 +160,7 @@ protected RevTag tag(final String name, final RevObject dst)\n \t}\n \n \tprotected <T extends RevObject> T parse(final T t) throws Exception {\n-\t\trw.parse(t);\n+\t\trw.parseBody(t);\n \t\treturn t;\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\nindex a35f61d..e25fe8f 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n@@ -820,6 +820,7 @@ private ObjectWalk setUpWalker(\n \t\t\tthrows MissingObjectException, IOException,\n \t\t\tIncorrectObjectTypeException {\n \t\tfinal ObjectWalk walker = new ObjectWalk(db);\n+\t\twalker.setRetainBody(false);\n \t\twalker.sort(RevSort.TOPO);\n \t\twalker.sort(RevSort.COMMIT_TIME_DESC, true);\n \t\tif (thin)\n@@ -854,12 +855,10 @@ private void findObjectsToPack(final ObjectWalk walker)\n \n \t\twhile ((o = walker.next()) != null) {\n \t\t\taddObject(o);\n-\t\t\to.dispose();\n \t\t\tinitMonitor.update(1);\n \t\t}\n \t\twhile ((o = walker.nextObject()) != null) {\n \t\t\taddObject(o);\n-\t\t\to.dispose();\n \t\t\tinitMonitor.update(1);\n \t\t}\n \t\tinitMonitor.endTask();\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/BoundaryGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/BoundaryGenerator.java\nindex f167830..6e3d6cf 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/BoundaryGenerator.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/BoundaryGenerator.java\n@@ -115,7 +115,7 @@ RevCommit next() throws MissingObjectException,\n \t\t\t\tif ((c.flags & DUPLICATE) != 0)\n \t\t\t\t\tcontinue;\n \t\t\t\tif ((c.flags & PARSED) == 0)\n-\t\t\t\t\tc.parse(walk);\n+\t\t\t\t\tc.parseHeaders(walk);\n \t\t\t\tc.flags |= DUPLICATE;\n \t\t\t\tboundary.add(c);\n \t\t\t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/MergeBaseGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/MergeBaseGenerator.java\nindex 8694e4c..afe3dfe 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/MergeBaseGenerator.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/MergeBaseGenerator.java\n@@ -137,7 +137,7 @@ RevCommit next() throws MissingObjectException,\n \t\t\t\tif ((p.flags & IN_PENDING) != 0)\n \t\t\t\t\tcontinue;\n \t\t\t\tif ((p.flags & PARSED) == 0)\n-\t\t\t\t\tp.parse(walker);\n+\t\t\t\t\tp.parseHeaders(walker);\n \t\t\t\tp.flags |= IN_PENDING;\n \t\t\t\tpending.add(p);\n \t\t\t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java\nindex 484d104..b2329cf 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java\n@@ -134,7 +134,7 @@ public void markStart(RevObject o) throws MissingObjectException,\n \t\twhile (o instanceof RevTag) {\n \t\t\taddObject(o);\n \t\t\to = ((RevTag) o).getObject();\n-\t\t\tparse(o);\n+\t\t\tparseHeaders(o);\n \t\t}\n \n \t\tif (o instanceof RevCommit)\n@@ -186,7 +186,7 @@ public void markUninteresting(RevObject o) throws MissingObjectException,\n \t\t\tif (hasRevSort(RevSort.BOUNDARY))\n \t\t\t\taddObject(o);\n \t\t\to = ((RevTag) o).getObject();\n-\t\t\tparse(o);\n+\t\t\tparseHeaders(o);\n \t\t}\n \n \t\tif (o instanceof RevCommit)\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java\nindex 8e3d6bc..0b97f92 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java\n@@ -135,7 +135,7 @@ RevCommit next() throws MissingObjectException,\n \t\t\t\t\tif ((p.flags & SEEN) != 0)\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\tif ((p.flags & PARSED) == 0)\n-\t\t\t\t\t\tp.parse(walker);\n+\t\t\t\t\t\tp.parseHeaders(walker);\n \t\t\t\t\tp.flags |= SEEN;\n \t\t\t\t\tpending.add(p);\n \t\t\t\t}\n@@ -157,14 +157,14 @@ RevCommit next() throws MissingObjectException,\n \t\t\t\t\t\toverScan = OVER_SCAN;\n \t\t\t\t\t}\n \t\t\t\t\tif (canDispose)\n-\t\t\t\t\t\tc.dispose();\n+\t\t\t\t\t\tc.disposeBody();\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \n \t\t\t\tif (produce)\n \t\t\t\t\treturn last = c;\n \t\t\t\telse if (canDispose)\n-\t\t\t\t\tc.dispose();\n+\t\t\t\t\tc.disposeBody();\n \t\t\t}\n \t\t} catch (StopWalkException swe) {\n \t\t\twalker.curs.release();\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java\nindex 679718e..f211dfd 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java\n@@ -74,11 +74,21 @@ protected RevCommit(final AnyObjectId id) {\n \t}\n \n \t@Override\n-\tvoid parse(final RevWalk walk) throws MissingObjectException,\n+\tvoid parseHeaders(final RevWalk walk) throws MissingObjectException,\n \t\t\tIncorrectObjectTypeException, IOException {\n \t\tparseCanonical(walk, loadCanonical(walk));\n \t}\n \n+\t@Override\n+\tvoid parseBody(final RevWalk walk) throws MissingObjectException,\n+\t\t\tIncorrectObjectTypeException, IOException {\n+\t\tif (buffer == null) {\n+\t\t\tbuffer = loadCanonical(walk);\n+\t\t\tif ((flags & PARSED) == 0)\n+\t\t\t\tparseCanonical(walk, buffer);\n+\t\t}\n+\t}\n+\n \tvoid parseCanonical(final RevWalk walk, final byte[] raw) {\n \t\tfinal MutableObjectId idBuffer = walk.idBuffer;\n \t\tidBuffer.fromString(raw, 5);\n@@ -125,7 +135,8 @@ else if (nParents == 1) {\n \t\t\tcommitTime = RawParseUtils.parseBase10(raw, ptr, null);\n \t\t}\n \n-\t\tbuffer = raw;\n+\t\tif (walk.isRetainBody())\n+\t\t\tbuffer = raw;\n \t\tflags |= PARSED;\n \t}\n \t\n@@ -375,8 +386,7 @@ public void reset() {\n \t\tinDegree = 0;\n \t}\n \n-\tpublic void dispose() {\n-\t\tflags &= ~PARSED;\n+\tfinal void disposeBody() {\n \t\tbuffer = null;\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\nindex 0e0386c..024e9ec 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n@@ -57,12 +57,18 @@ RevObject(final AnyObjectId name) {\n \t\tsuper(name);\n \t}\n \n-\tvoid parse(final RevWalk walk) throws MissingObjectException,\n+\tvoid parseHeaders(final RevWalk walk) throws MissingObjectException,\n \t\t\tIncorrectObjectTypeException, IOException {\n \t\tloadCanonical(walk);\n \t\tflags |= PARSED;\n \t}\n \n+\tvoid parseBody(final RevWalk walk) throws MissingObjectException,\n+\t\t\tIncorrectObjectTypeException, IOException {\n+\t\tif ((flags & PARSED) == 0)\n+\t\t\tparseHeaders(walk);\n+\t}\n+\n \tfinal byte[] loadCanonical(final RevWalk walk) throws IOException,\n \t\t\tMissingObjectException, IncorrectObjectTypeException,\n \t\t\tCorruptObjectException {\n@@ -180,11 +186,6 @@ public final void remove(final RevFlagSet set) {\n \t\tflags &= ~set.mask;\n \t}\n \n-\t/** Release as much memory as possible from this object. */\n-\tpublic void dispose() {\n-\t\t// Nothing needs to be done for most objects.\n-\t}\n-\n \t@Override\n \tpublic String toString() {\n \t\tfinal StringBuilder s = new StringBuilder();\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\nindex e876025..2fab266 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java\n@@ -69,11 +69,21 @@ protected RevTag(final AnyObjectId id) {\n \t}\n \n \t@Override\n-\tvoid parse(final RevWalk walk) throws MissingObjectException,\n+\tvoid parseHeaders(final RevWalk walk) throws MissingObjectException,\n \t\t\tIncorrectObjectTypeException, IOException {\n \t\tparseCanonical(walk, loadCanonical(walk));\n \t}\n \n+\t@Override\n+\tvoid parseBody(final RevWalk walk) throws MissingObjectException,\n+\t\t\tIncorrectObjectTypeException, IOException {\n+\t\tif (buffer == null) {\n+\t\t\tbuffer = loadCanonical(walk);\n+\t\t\tif ((flags & PARSED) == 0)\n+\t\t\t\tparseCanonical(walk, buffer);\n+\t\t}\n+\t}\n+\n \tvoid parseCanonical(final RevWalk walk, final byte[] rawTag)\n \t\t\tthrows CorruptObjectException {\n \t\tfinal MutableInteger pos = new MutableInteger();\n@@ -87,7 +97,9 @@ void parseCanonical(final RevWalk walk, final byte[] rawTag)\n \t\tint p = pos.value += 4; // \"tag \"\n \t\tfinal int nameEnd = RawParseUtils.nextLF(rawTag, p) - 1;\n \t\tname = RawParseUtils.decode(Constants.CHARSET, rawTag, p, nameEnd);\n-\t\tbuffer = rawTag;\n+\n+\t\tif (walk.isRetainBody())\n+\t\t\tbuffer = rawTag;\n \t\tflags |= PARSED;\n \t}\n \n@@ -195,8 +207,7 @@ public final String getName() {\n \t\treturn name;\n \t}\n \n-\tpublic void dispose() {\n-\t\tflags &= ~PARSED;\n+\tfinal void disposeBody() {\n \t\tbuffer = null;\n \t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\nindex 40bdb4e..5589fb0 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n@@ -175,6 +175,8 @@\n \n \tprivate TreeFilter treeFilter;\n \n+\tprivate boolean retainBody;\n+\n \t/**\n \t * Create a new revision walker for a given repository.\n \t * \n@@ -192,6 +194,7 @@ public RevWalk(final Repository repo) {\n \t\tsorting = EnumSet.of(RevSort.NONE);\n \t\tfilter = RevFilter.ALL;\n \t\ttreeFilter = TreeFilter.ALL;\n+\t\tretainBody = true;\n \t}\n \n \t/**\n@@ -237,7 +240,7 @@ public void markStart(final RevCommit c) throws MissingObjectException,\n \t\tif ((c.flags & SEEN) != 0)\n \t\t\treturn;\n \t\tif ((c.flags & PARSED) == 0)\n-\t\t\tc.parse(this);\n+\t\t\tc.parseHeaders(this);\n \t\tc.flags |= SEEN;\n \t\troots.add(c);\n \t\tqueue.add(c);\n@@ -510,6 +513,32 @@ public void setTreeFilter(final TreeFilter newFilter) {\n \t}\n \n \t/**\n+\t * Should the body of a commit or tag be retained after parsing its headers?\n+\t * <p>\n+\t * Usually the body is always retained, but some application code might not\n+\t * care and would prefer to discard the body of a commit as early as\n+\t * possible, to reduce memory usage.\n+\t *\n+\t * @return true if the body should be retained; false it is discarded.\n+\t */\n+\tpublic boolean isRetainBody() {\n+\t\treturn retainBody;\n+\t}\n+\n+\t/**\n+\t * Set whether or not the body of a commit or tag is retained.\n+\t * <p>\n+\t * If a body of a commit or tag is not retained, the application must\n+\t * call {@link #parseBody(RevObject)} before the body can be safely\n+\t * accessed through the type specific access methods.\n+\t *\n+\t * @param retain true to retain bodies; false to discard them early.\n+\t */\n+\tpublic void setRetainBody(final boolean retain) {\n+\t\tretainBody = retain;\n+\t}\n+\n+\t/**\n \t * Locate a reference to a blob without loading it.\n \t * <p>\n \t * The blob may or may not exist in the repository. It is impossible to tell\n@@ -625,7 +654,7 @@ public RevCommit parseCommit(final AnyObjectId id)\n \t\tRevObject c = parseAny(id);\n \t\twhile (c instanceof RevTag) {\n \t\t\tc = ((RevTag) c).getObject();\n-\t\t\tparse(c);\n+\t\t\tparseHeaders(c);\n \t\t}\n \t\tif (!(c instanceof RevCommit))\n \t\t\tthrow new IncorrectObjectTypeException(id.toObjectId(),\n@@ -637,7 +666,7 @@ public RevCommit parseCommit(final AnyObjectId id)\n \t * Locate a reference to a tree.\n \t * <p>\n \t * This method only returns successfully if the tree object exists, is\n-\t * verified to be a tree, and was parsed without error.\n+\t * verified to be a tree.\n \t *\n \t * @param id\n \t *            name of the tree object, or a commit or annotated tag that may\n@@ -656,7 +685,7 @@ public RevTree parseTree(final AnyObjectId id)\n \t\tRevObject c = parseAny(id);\n \t\twhile (c instanceof RevTag) {\n \t\t\tc = ((RevTag) c).getObject();\n-\t\t\tparse(c);\n+\t\t\tparseHeaders(c);\n \t\t}\n \n \t\tfinal RevTree t;\n@@ -667,12 +696,12 @@ else if (!(c instanceof RevTree))\n \t\t\t\t\tConstants.TYPE_TREE);\n \t\telse\n \t\t\tt = (RevTree) c;\n-\t\tparse(t);\n+\t\tparseHeaders(t);\n \t\treturn t;\n \t}\n \n \t/**\n-\t * Locate a reference to any object and immediately parse its content.\n+\t * Locate a reference to any object and immediately parse its headers.\n \t * <p>\n \t * This method only returns successfully if the object exists and was parsed\n \t * without error. Parsing an object can be expensive as the type must be\n@@ -724,12 +753,12 @@ public RevObject parseAny(final AnyObjectId id)\n \t\t\t}\n \t\t\tobjects.add(r);\n \t\t} else\n-\t\t\tparse(r);\n+\t\t\tparseHeaders(r);\n \t\treturn r;\n \t}\n \n \t/**\n-\t * Ensure the object's content has been parsed.\n+\t * Ensure the object's critical headers have been parsed.\n \t * <p>\n \t * This method only returns successfully if the object exists and was parsed\n \t * without error.\n@@ -741,11 +770,28 @@ public RevObject parseAny(final AnyObjectId id)\n \t * @throws IOException\n \t *             a pack file or loose object could not be read.\n \t */\n-\tpublic void parse(final RevObject obj) throws MissingObjectException,\n-\t\t\tIOException {\n-\t\tif ((obj.flags & PARSED) != 0)\n-\t\t\treturn;\n-\t\tobj.parse(this);\n+\tpublic void parseHeaders(final RevObject obj)\n+\t\t\tthrows MissingObjectException, IOException {\n+\t\tif ((obj.flags & PARSED) == 0)\n+\t\t\tobj.parseHeaders(this);\n+\t}\n+\n+\t/**\n+\t * Ensure the object's fully body content is available.\n+\t * <p>\n+\t * This method only returns successfully if the object exists and was parsed\n+\t * without error.\n+\t * \n+\t * @param obj\n+\t *            the object the caller needs to be parsed.\n+\t * @throws MissingObjectException\n+\t *             the supplied does not exist.\n+\t * @throws IOException\n+\t *             a pack file or loose object could not be read.\n+\t */\n+\tpublic void parseBody(final RevObject obj)\n+\t\t\tthrows MissingObjectException, IOException {\n+\t\tobj.parseBody(this);\n \t}\n \n \t/**\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java\nindex a5edbf0..754be07 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java\n@@ -94,7 +94,7 @@ public boolean include(final RevWalk walker, final RevCommit c)\n \t\tfor (int i = 0; i < nParents; i++) {\n \t\t\tfinal RevCommit p = c.parents[i];\n \t\t\tif ((p.flags & PARSED) == 0)\n-\t\t\t\tp.parse(walker);\n+\t\t\t\tp.parseHeaders(walker);\n \t\t\ttrees[i] = p.getTree();\n \t\t}\n \t\ttrees[nParents] = c.getTree();\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\nindex 45d57b3..5127dad 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\n@@ -137,6 +137,7 @@\n \tpublic UploadPack(final Repository copyFrom) {\n \t\tdb = copyFrom;\n \t\twalk = new RevWalk(db);\n+\t\twalk.setRetainBody(false);\n \n \t\tADVERTISED = walk.newFlag(\"ADVERTISED\");\n \t\tWANT = walk.newFlag(\"WANT\");\n@@ -270,7 +271,7 @@ private void writeAdvertisedTag(final StringBuilder m, final char[] idtmp,\n \t\twhile (o instanceof RevTag) {\n \t\t\t// Fully unwrap here so later on we have these already parsed.\n \t\t\ttry {\n-\t\t\t\twalk.parse(((RevTag) o).getObject());\n+\t\t\t\twalk.parseHeaders(((RevTag) o).getObject());\n \t\t\t} catch (IOException err) {\n \t\t\t\treturn;\n \t\t\t}\n@@ -434,7 +435,6 @@ private boolean wantSatisfied(final RevCommit want) throws IOException {\n \t\t\t\t}\n \t\t\t\treturn true;\n \t\t\t}\n-\t\t\tc.dispose();\n \t\t}\n \t\treturn false;\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java\nindex 4d14305..6929c5f 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java\n@@ -193,6 +193,7 @@ WalkFetchConnection(final WalkTransport t, final WalkRemoteObjectDatabase w) {\n \t\tpackLocks = new ArrayList<PackLock>(4);\n \n \t\trevWalk = new RevWalk(local);\n+\t\trevWalk.setRetainBody(false);\n \t\ttreeWalk = new TreeWalk(local);\n \t\tCOMPLETE = revWalk.newFlag(\"COMPLETE\");\n \t\tIN_WORK_QUEUE = revWalk.newFlag(\"IN_WORK_QUEUE\");\n@@ -266,7 +267,7 @@ private void process(final ObjectId id) throws TransportException {\n \t\t\t\tobj = (RevObject) id;\n \t\t\t\tif (obj.has(COMPLETE))\n \t\t\t\t\treturn;\n-\t\t\t\trevWalk.parse(obj);\n+\t\t\t\trevWalk.parseHeaders(obj);\n \t\t\t} else {\n \t\t\t\tobj = revWalk.parseAny(id);\n \t\t\t\tif (obj.has(COMPLETE))\n@@ -276,12 +277,6 @@ private void process(final ObjectId id) throws TransportException {\n \t\t\tthrow new TransportException(\"Cannot read \" + id.name(), e);\n \t\t}\n \n-\t\t// We only care about traversal; disposing an object throws its\n-\t\t// message buffer (if any) away but retains the links so we can\n-\t\t// continue to navigate, but use less memory long-term.\n-\t\t//\n-\t\tobj.dispose();\n-\n \t\tswitch (obj.getType()) {\n \t\tcase Constants.OBJ_BLOB:\n \t\t\tprocessBlob(obj);\t\n@@ -694,9 +689,8 @@ private void markLocalRefsComplete(final Set<ObjectId> have) throws TransportExc\n \tprivate void markLocalObjComplete(RevObject obj) throws IOException {\n \t\twhile (obj.getType() == Constants.OBJ_TAG) {\n \t\t\tobj.add(COMPLETE);\n-\t\t\tobj.dispose();\n \t\t\tobj = ((RevTag) obj).getObject();\n-\t\t\trevWalk.parse(obj);\n+\t\t\trevWalk.parseHeaders(obj);\n \t\t}\n \n \t\tswitch (obj.getType()) {\n@@ -734,11 +728,10 @@ private void pushLocalCommit(final RevCommit p)\n \t\t\tthrows MissingObjectException, IOException {\n \t\tif (p.has(LOCALLY_SEEN))\n \t\t\treturn;\n-\t\trevWalk.parse(p);\n+\t\trevWalk.parseHeaders(p);\n \t\tp.add(LOCALLY_SEEN);\n \t\tp.add(COMPLETE);\n \t\tp.carry(COMPLETE);\n-\t\tp.dispose();\n \t\tlocalCommitQueue.add(p);\n \t}\n \n-- \n1.6.3.2.367.gf0de\n"},{"id":"116197","messageId":"1244847619-7364-6-git-send-email-spearce@spearce.org","threadId":"19794","inReplyTo":"1244847619-7364-5-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 5/5] UploadPack: Only recompute okToGiveUp() if bases changed","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-12T23:00:19Z","receivedAt":"2009-06-12T23:00:19Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If we get a commit we don't recognize, but we haven't added any new\ncommon bases to the commonBase set since the last time we ran the\nokToGiveUp() algorithm, then there is no point in running it again,\nas the result will not change.  Instead we cache the result in a\nBoolean and just return the cached result until another common base\ncan be found and added to the set.\n\nThis helps to fix a performance problem when the client is 50,000\ncommits ahead of the server, and keeps sending them in \"have\"\nlines, but the server doesn't recognize them.  On every \"have\"\nline we will not have updated the common base, but we also aren't\nyet satisfied that it is OK to give up negotiation.\n\nBetween the changes introduced by the parent commit and this commit,\nnegotiation in this case of 50,000 ahead now completes instantly,\ninstead of taking hours.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/transport/UploadPack.java |   27 ++++++++++++++------\n 1 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\nindex 5127dad..5c8df62 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\n@@ -112,6 +112,9 @@\n \t/** Objects on both sides, these don't have to be sent. */\n \tprivate final List<RevObject> commonBase = new ArrayList<RevObject>();\n \n+\t/** null if {@link #commonBase} should be examined again. */\n+\tprivate Boolean okToGiveUp;\n+\n \t/** Marked on objects we sent in our advertisement list. */\n \tprivate final RevFlag ADVERTISED;\n \n@@ -396,15 +399,26 @@ private boolean matchHave(final ObjectId id) {\n \t\t\to.add(PEER_HAS);\n \t\t\tif (o instanceof RevCommit)\n \t\t\t\t((RevCommit) o).carry(PEER_HAS);\n-\t\t\tif (!o.has(COMMON)) {\n-\t\t\t\to.add(COMMON);\n-\t\t\t\tcommonBase.add(o);\n-\t\t\t}\n+\t\t\taddCommonBase(o);\n \t\t}\n \t\treturn true;\n \t}\n \n+\tprivate void addCommonBase(final RevObject o) {\n+\t\tif (!o.has(COMMON)) {\n+\t\t\to.add(COMMON);\n+\t\t\tcommonBase.add(o);\n+\t\t\tokToGiveUp = null;\n+\t\t}\n+\t}\n+\n \tprivate boolean okToGiveUp() throws PackProtocolException {\n+\t\tif (okToGiveUp == null)\n+\t\t\tokToGiveUp = Boolean.valueOf(okToGiveUpImp());\n+\t\treturn okToGiveUp.booleanValue();\n+\t}\n+\n+\tprivate boolean okToGiveUpImp() throws PackProtocolException {\n \t\tif (commonBase.isEmpty())\n \t\t\treturn false;\n \n@@ -429,10 +443,7 @@ private boolean wantSatisfied(final RevCommit want) throws IOException {\n \t\t\tif (c == null)\n \t\t\t\tbreak;\n \t\t\tif (c.has(PEER_HAS)) {\n-\t\t\t\tif (!c.has(COMMON)) {\n-\t\t\t\t\tc.add(COMMON);\n-\t\t\t\t\tcommonBase.add(c);\n-\t\t\t\t}\n+\t\t\t\taddCommonBase(c);\n \t\t\t\treturn true;\n \t\t\t}\n \t\t}\n-- \n1.6.3.2.367.gf0de\n"}]}