From 5664fb3bfb63e4db49dc07d13ace419e810186c2 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sun, 6 Feb 2011 01:00:44 -0800 Subject: [PATCH] UploadPack: Donate parsed commits to PackWriter When UploadPack has computed the merge base between the client's have set and the want set, its already loaded and parsed all of the interesting commits that PackWriter needs to transmit to the client. Switching the RevWalk and its object pool over to be an ObjectWalk saves PackWriter from needing to re-parse these same commits from the ObjectDatabase, reducing the startup latency for the enumeration phase of packing. UploadPack doesn't want to use an ObjectWalk for the okToGiveUp() tests because its slower, during each commit popped it needs to cache the tree into the pendingObjects list, and during each reset() it discards a bunch of ObjectWalk specific state and reallocates some internal collections. ObjectWalk was never meant to be rapidly reset() like UploadPack does, so its perhaps somewhat cleaner to allow "upgrading" a RevWalk to an ObjectWalk. Bug: 301639 Change-Id: I97ef52a0b79d78229c272880aedb7f74d0f7532f Signed-off-by: Shawn O. Pearce --- .../src/org/eclipse/jgit/revwalk/RevWalk.java | 22 ++++++++++- .../eclipse/jgit/storage/pack/PackWriter.java | 38 +++++++++++++++++-- .../eclipse/jgit/transport/UploadPack.java | 23 +++++++---- 3 files changed, 71 insertions(+), 12 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 94eb62106..17cdb443c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java @@ -170,7 +170,7 @@ public class RevWalk implements Iterable { final MutableObjectId idBuffer; - private final ObjectIdSubclassMap objects; + private ObjectIdSubclassMap objects; private int freeFlags = APP_FLAGS; @@ -1272,6 +1272,26 @@ private boolean isNotStarted() { return pending instanceof StartGenerator; } + /** + * Create and return an {@link ObjectWalk} using the same objects. + *

+ * Prior to using this method, the caller must reset this RevWalk to clean + * any flags that were used during the last traversal. + *

+ * The returned ObjectWalk uses the same ObjectReader, internal object pool, + * and free RevFlags. Once the ObjectWalk is created, this RevWalk should + * not be used anymore. + * + * @return a new walk, using the exact same object pool. + */ + public ObjectWalk toObjectWalkWithSameObjects() { + ObjectWalk ow = new ObjectWalk(reader); + RevWalk rw = ow; + rw.objects = objects; + rw.freeFlags = freeFlags; + return ow; + } + /** * Construct a new unparsed commit for the given object. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java index eee08ed3e..2a11de4c5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java @@ -381,6 +381,38 @@ public void preparePack(final Iterator objectsSource) * * @param countingMonitor * progress during object enumeration. + * @param want + * collection of objects to be marked as interesting (start + * points of graph traversal). + * @param have + * collection of objects to be marked as uninteresting (end + * points of graph traversal). + * @throws IOException + * when some I/O problem occur during reading objects. + */ + public void preparePack(ProgressMonitor countingMonitor, + final Collection want, + final Collection have) throws IOException { + ObjectWalk ow = new ObjectWalk(reader); + preparePack(countingMonitor, ow, want, have); + } + + /** + * Prepare the list of objects to be written to the pack stream. + *

+ * Basing on these 2 sets, another set of objects to put in a pack file is + * created: this set consists of all objects reachable (ancestors) from + * interesting objects, except uninteresting objects and their ancestors. + * This method uses class {@link ObjectWalk} extensively to find out that + * appropriate set of output objects and their optimal order in output pack. + * Order is consistent with general git in-pack rules: sort by object type, + * recency, path and delta-base first. + *

+ * + * @param countingMonitor + * progress during object enumeration. + * @param walk + * ObjectWalk to perform enumeration. * @param interestingObjects * collection of objects to be marked as interesting (start * points of graph traversal). @@ -391,12 +423,13 @@ public void preparePack(final Iterator objectsSource) * when some I/O problem occur during reading objects. */ public void preparePack(ProgressMonitor countingMonitor, + final ObjectWalk walk, final Collection interestingObjects, final Collection uninterestingObjects) throws IOException { if (countingMonitor == null) countingMonitor = NullProgressMonitor.INSTANCE; - findObjectsToPack(countingMonitor, interestingObjects, + findObjectsToPack(countingMonitor, walk, interestingObjects, uninterestingObjects); } @@ -1042,7 +1075,7 @@ private void writeChecksum(PackOutputStream out) throws IOException { } private void findObjectsToPack(final ProgressMonitor countingMonitor, - final Collection want, + final ObjectWalk walker, final Collection want, Collection have) throws MissingObjectException, IOException, IncorrectObjectTypeException { @@ -1057,7 +1090,6 @@ private void findObjectsToPack(final ProgressMonitor countingMonitor, all.addAll(have); final Map tipToPack = new HashMap(); - final ObjectWalk walker = new ObjectWalk(reader); final RevFlag inCachedPack = walker.newFlag("inCachedPack"); final RevFlag include = walker.newFlag("include"); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 5133f5cde..e3ce59d07 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -49,7 +49,6 @@ import java.io.OutputStream; import java.text.MessageFormat; import java.util.ArrayList; -import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -65,6 +64,7 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.AsyncRevObjectQueue; +import org.eclipse.jgit.revwalk.ObjectWalk; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevFlag; import org.eclipse.jgit.revwalk.RevFlagSet; @@ -656,10 +656,6 @@ private void sendPack() throws IOException { } } - Collection want = wantAll; - if (want.isEmpty()) - want = wantIds; - PackConfig cfg = packConfig; if (cfg == null) cfg = new PackConfig(db); @@ -668,7 +664,18 @@ private void sendPack() throws IOException { pw.setUseCachedPacks(true); pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA)); pw.setThin(options.contains(OPTION_THIN_PACK)); - pw.preparePack(pm, want, commonBase); + + RevWalk rw = walk; + if (wantAll.isEmpty()) { + pw.preparePack(pm, wantIds, commonBase); + } else { + walk.reset(); + + ObjectWalk ow = walk.toObjectWalkWithSameObjects(); + pw.preparePack(pm, ow, wantAll, commonBase); + rw = ow; + } + if (options.contains(OPTION_INCLUDE_TAG)) { for (Ref ref : refs.values()) { ObjectId objectId = ref.getObjectId(); @@ -678,7 +685,7 @@ private void sendPack() throws IOException { if (wantIds.contains(objectId)) continue; } else { - RevObject obj = walk.lookupOrNull(objectId); + RevObject obj = rw.lookupOrNull(objectId); if (obj != null && obj.has(WANT)) continue; } @@ -692,7 +699,7 @@ private void sendPack() throws IOException { objectId = ref.getObjectId(); if (pw.willInclude(peeledId) && !pw.willInclude(objectId)) - pw.addObject(walk.parseAny(objectId)); + pw.addObject(rw.parseAny(objectId)); } }