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 <spearce@spearce.org>

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

@@ -170,7 +170,7 @@ public class RevWalk implements Iterable<RevCommit> {
 
 	final MutableObjectId idBuffer;
 
-	private final ObjectIdSubclassMap<RevObject> objects;
+	private ObjectIdSubclassMap<RevObject> objects;
 
 	private int freeFlags = APP_FLAGS;
 
@@ -1272,6 +1272,26 @@ public class RevWalk implements Iterable<RevCommit> {
 		return pending instanceof StartGenerator;
 	}
 
+	/**
+	 * Create and return an {@link ObjectWalk} using the same objects.
+	 * <p>
+	 * Prior to using this method, the caller must reset this RevWalk to clean
+	 * any flags that were used during the last traversal.
+	 * <p>
+	 * 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.
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java

@@ -381,6 +381,38 @@ public class PackWriter {
 	 *
 	 * @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<? extends ObjectId> want,
+			final Collection<? extends ObjectId> 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.
+	 * <p>
+	 * 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.
+	 * </p>
+	 *
+	 * @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 class PackWriter {
 	 *             when some I/O problem occur during reading objects.
 	 */
 	public void preparePack(ProgressMonitor countingMonitor,
+			final ObjectWalk walk,
 			final Collection<? extends ObjectId> interestingObjects,
 			final Collection<? extends ObjectId> uninterestingObjects)
 			throws IOException {
 		if (countingMonitor == null)
 			countingMonitor = NullProgressMonitor.INSTANCE;
-		findObjectsToPack(countingMonitor, interestingObjects,
+		findObjectsToPack(countingMonitor, walk, interestingObjects,
 				uninterestingObjects);
 	}
 
@@ -1042,7 +1075,7 @@ public class PackWriter {
 	}
 
 	private void findObjectsToPack(final ProgressMonitor countingMonitor,
-			final Collection<? extends ObjectId> want,
+			final ObjectWalk walker, final Collection<? extends ObjectId> want,
 			Collection<? extends ObjectId> have)
 			throws MissingObjectException, IOException,
 			IncorrectObjectTypeException {
@@ -1057,7 +1090,6 @@ public class PackWriter {
 		all.addAll(have);
 
 		final Map<ObjectId, CachedPack> tipToPack = new HashMap<ObjectId, CachedPack>();
-		final ObjectWalk walker = new ObjectWalk(reader);
 		final RevFlag inCachedPack = walker.newFlag("inCachedPack");
 		final RevFlag include = walker.newFlag("include");
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@@ -49,7 +49,6 @@ import java.io.InputStream;
 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.ProgressMonitor;
 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 @@ public class UploadPack {
 			}
 		}
 
-		Collection<? extends ObjectId> want = wantAll;
-		if (want.isEmpty())
-			want = wantIds;
-
 		PackConfig cfg = packConfig;
 		if (cfg == null)
 			cfg = new PackConfig(db);
@@ -668,7 +664,18 @@ public class UploadPack {
 			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 @@ public class UploadPack {
 						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 @@ public class UploadPack {
 
 					objectId = ref.getObjectId();
 					if (pw.willInclude(peeledId) && !pw.willInclude(objectId))
-						pw.addObject(walk.parseAny(objectId));
+						pw.addObject(rw.parseAny(objectId));
 				}
 			}