Move PackWriter progress monitors onto the operations

Rather than taking the ProgressMonitor objects in our constructor and
carrying them around as instance fields, take them as arguments to the
actual time consuming operations we need to run.

Change-Id: I2b230d07e277de029b1061c807e67de5428cc1c4
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java

@@ -600,14 +600,15 @@ private static void assertHash(RevObject id, byte[] bin) {
 	public void packAndPrune() throws Exception {
 		if (db.getObjectDatabase() instanceof ObjectDirectory) {
 			ObjectDirectory odb = (ObjectDirectory) db.getObjectDatabase();
+			NullProgressMonitor m = NullProgressMonitor.INSTANCE;
 
 			final File pack, idx;
-			PackWriter pw = new PackWriter(db, NullProgressMonitor.INSTANCE);
+			PackWriter pw = new PackWriter(db);
 			try {
 				Set<ObjectId> all = new HashSet<ObjectId>();
 				for (Ref r : db.getAllRefs().values())
 					all.add(r.getObjectId());
-				pw.preparePack(all, Collections.<ObjectId> emptySet());
+				pw.preparePack(m, all, Collections.<ObjectId> emptySet());
 
 				final ObjectId name = pw.computeName();
 				OutputStream out;
@@ -615,7 +616,7 @@ public void packAndPrune() throws Exception {
 				pack = nameFor(odb, name, ".pack");
 				out = new BufferedOutputStream(new FileOutputStream(pack));
 				try {
-					pw.writePack(out);
+					pw.writePack(m, m, out);
 				} finally {
 					out.close();
 				}

org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java

@@ -134,7 +134,7 @@ public void testObjectMovedWithinPack()
 		// within the pack has been modified.
 		//
 		final RevObject o2 = writeBlob(eden, "o2");
-		final PackWriter pw = new PackWriter(eden, NullProgressMonitor.INSTANCE);
+		final PackWriter pw = new PackWriter(eden);
 		pw.addObject(o2);
 		pw.addObject(o1);
 		write(out1, pw);
@@ -199,7 +199,7 @@ private RevObject parse(final AnyObjectId id)
 
 	private File[] pack(final Repository src, final RevObject... list)
 			throws IOException {
-		final PackWriter pw = new PackWriter(src, NullProgressMonitor.INSTANCE);
+		final PackWriter pw = new PackWriter(src);
 		for (final RevObject o : list) {
 			pw.addObject(o);
 		}
@@ -216,11 +216,12 @@ private File[] pack(final Repository src, final RevObject... list)
 	private static void write(final File[] files, final PackWriter pw)
 			throws IOException {
 		final long begin = files[0].getParentFile().lastModified();
+		NullProgressMonitor m = NullProgressMonitor.INSTANCE;
 		OutputStream out;
 
 		out = new BufferedOutputStream(new FileOutputStream(files[0]));
 		try {
-			pw.writePack(out);
+			pw.writePack(m, m, out);
 		} finally {
 			out.close();
 		}

org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/PackWriterTest.java

@@ -59,6 +59,7 @@
 import java.util.List;
 
 import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.SampleDataRepositoryTestCase;
 import org.eclipse.jgit.lib.TextProgressMonitor;
@@ -95,7 +96,7 @@ public void setUp() throws Exception {
 		packBase = new File(trash, "tmp_pack");
 		packFile = new File(trash, "tmp_pack.pack");
 		indexFile = new File(trash, "tmp_pack.idx");
-		writer = new PackWriter(db, new TextProgressMonitor());
+		writer = new PackWriter(db);
 	}
 
 	/**
@@ -480,18 +481,20 @@ private void createVerifyOpenPack(final Collection<ObjectId> interestings,
 			final Collection<ObjectId> uninterestings, final boolean thin,
 			final boolean ignoreMissingUninteresting)
 			throws MissingObjectException, IOException {
+		NullProgressMonitor m = NullProgressMonitor.INSTANCE;
 		writer.setThin(thin);
 		writer.setIgnoreMissingUninteresting(ignoreMissingUninteresting);
-		writer.preparePack(interestings, uninterestings);
+		writer.preparePack(m, interestings, uninterestings);
-		writer.writePack(os);
+		writer.writePack(m, m, os);
 		writer.release();
 		verifyOpenPack(thin);
 	}
 
 	private void createVerifyOpenPack(final Iterator<RevObject> objectSource)
 			throws MissingObjectException, IOException {
+		NullProgressMonitor m = NullProgressMonitor.INSTANCE;
 		writer.preparePack(objectSource);
-		writer.writePack(os);
+		writer.writePack(m, m, os);
 		writer.release();
 		verifyOpenPack(false);
 	}

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BundleWriterTest.java

@@ -148,12 +148,12 @@ private byte[] makeBundle(final String name,
 			throws FileNotFoundException, IOException {
 		final BundleWriter bw;
 
-		bw = new BundleWriter(db, NullProgressMonitor.INSTANCE);
+		bw = new BundleWriter(db);
 		bw.include(name, ObjectId.fromString(anObjectToInclude));
 		if (assume != null)
 			bw.assume(assume);
 		final ByteArrayOutputStream out = new ByteArrayOutputStream();
-		bw.writeBundle(out);
+		bw.writeBundle(NullProgressMonitor.INSTANCE, out);
 		return out.toByteArray();
 	}
 

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

@@ -50,10 +50,13 @@
 import java.util.zip.CRC32;
 
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.util.NB;
 
 /** Custom output stream to support {@link PackWriter}. */
 public final class PackOutputStream extends OutputStream {
+	private final ProgressMonitor writeMonitor;
+
 	private final OutputStream out;
 
 	private final boolean ofsDelta;
@@ -68,7 +71,9 @@ public final class PackOutputStream extends OutputStream {
 
 	private byte[] copyBuffer;
 
-	PackOutputStream(final OutputStream out, final boolean ofsDelta) {
+	PackOutputStream(final ProgressMonitor writeMonitor,
+			final OutputStream out, final boolean ofsDelta) {
+		this.writeMonitor = writeMonitor;
 		this.out = out;
 		this.ofsDelta = ofsDelta;
 	}
@@ -168,6 +173,10 @@ public byte[] getCopyBuffer() {
 		return copyBuffer;
 	}
 
+	void endObject() {
+		writeMonitor.update(1);
+	}
+
 	/** @return total number of bytes written since stream start. */
 	long length() {
 		return count;

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

@@ -96,8 +96,8 @@
  * Typical usage consists of creating instance intended for some pack,
  * configuring options, preparing the list of objects by calling
  * {@link #preparePack(Iterator)} or
- * {@link #preparePack(Collection, Collection)}, and finally
+ * {@link #preparePack(ProgressMonitor, Collection, Collection)}, and finally
- * producing the stream with {@link #writePack(OutputStream)}.
+ * producing the stream with {@link #writePack(ProgressMonitor, ProgressMonitor, OutputStream)}.
  * </p>
  * <p>
  * Class provide set of configurable options and {@link ProgressMonitor}
@@ -116,7 +116,7 @@ public class PackWriter {
 	 * Title of {@link ProgressMonitor} task used during counting objects to
 	 * pack.
 	 *
-	 * @see #preparePack(Collection, Collection)
+	 * @see #preparePack(ProgressMonitor, Collection, Collection)
 	 */
 	public static final String COUNTING_OBJECTS_PROGRESS = JGitText.get().countingObjects;
 
@@ -124,7 +124,7 @@ public class PackWriter {
 	 * Title of {@link ProgressMonitor} task used during searching for objects
 	 * reuse or delta reuse.
 	 *
-	 * @see #writePack(OutputStream)
+	 * @see #writePack(ProgressMonitor, ProgressMonitor, OutputStream)
 	 */
 	public static final String SEARCHING_REUSE_PROGRESS = JGitText.get().compressingObjects;
 
@@ -132,7 +132,7 @@ public class PackWriter {
 	 * Title of {@link ProgressMonitor} task used during writing out pack
 	 * (objects)
 	 *
-	 * @see #writePack(OutputStream)
+	 * @see #writePack(ProgressMonitor, ProgressMonitor, OutputStream)
 	 */
 	public static final String WRITING_OBJECTS_PROGRESS = JGitText.get().writingObjects;
 
@@ -185,10 +185,6 @@ public class PackWriter {
 
 	private final Deflater deflater;
 
-	private ProgressMonitor initMonitor;
-
-	private ProgressMonitor writeMonitor;
-
 	private final ObjectReader reader;
 
 	/** {@link #reader} recast to the reuse interface, if it supports it. */
@@ -216,38 +212,12 @@ public class PackWriter {
 	 * Create writer for specified repository.
 	 * <p>
 	 * Objects for packing are specified in {@link #preparePack(Iterator)} or
-	 * {@link #preparePack(Collection, Collection)}.
+	 * {@link #preparePack(ProgressMonitor, Collection, Collection)}.
 	 *
 	 * @param repo
 	 *            repository where objects are stored.
-	 * @param monitor
-	 *            operations progress monitor, used within
-	 *            {@link #preparePack(Iterator)},
-	 *            {@link #preparePack(Collection, Collection)}
-	 *            , or {@link #writePack(OutputStream)}.
 	 */
-	public PackWriter(final Repository repo, final ProgressMonitor monitor) {
+	public PackWriter(final Repository repo) {
-		this(repo, monitor, monitor);
-	}
-
-	/**
-	 * Create writer for specified repository.
-	 * <p>
-	 * Objects for packing are specified in {@link #preparePack(Iterator)} or
-	 * {@link #preparePack(Collection, Collection)}.
-	 *
-	 * @param repo
-	 *            repository where objects are stored.
-	 * @param imonitor
-	 *            operations progress monitor, used within
-	 *            {@link #preparePack(Iterator)},
-	 *            {@link #preparePack(Collection, Collection)}
-	 * @param wmonitor
-	 *            operations progress monitor, used within
-	 *            {@link #writePack(OutputStream)}.
-	 */
-	public PackWriter(final Repository repo, final ProgressMonitor imonitor,
-			final ProgressMonitor wmonitor) {
 		this.db = repo;
 
 		reader = db.newObjectReader();
@@ -256,9 +226,6 @@ public PackWriter(final Repository repo, final ProgressMonitor imonitor,
 		else
 			reuseSupport = null;
 
-		initMonitor = imonitor == null ? NullProgressMonitor.INSTANCE : imonitor;
-		writeMonitor = wmonitor == null ? NullProgressMonitor.INSTANCE : wmonitor;
-
 		final CoreConfig coreConfig = db.getConfig().get(CoreConfig.KEY);
 		this.deflater = new Deflater(coreConfig.getCompression());
 		outputVersion = coreConfig.getPackIndexVersion();
@@ -283,7 +250,7 @@ public boolean isReuseDeltas() {
 	 * use it if possible. Normally, only deltas with base to another object
 	 * existing in set of objects to pack will be used. Exception is however
 	 * thin-pack (see
-	 * {@link #preparePack(Collection, Collection)} and
+	 * {@link #preparePack(ProgressMonitor, Collection, Collection)} and
 	 * {@link #preparePack(Iterator)}) where base object must exist on other
 	 * side machine.
 	 * <p>
@@ -411,7 +378,7 @@ public void setThin(final boolean packthin) {
 	/**
 	 * @return true to ignore objects that are uninteresting and also not found
 	 *         on local disk; false to throw a {@link MissingObjectException}
-	 *         out of {@link #preparePack(Collection, Collection)} if an
+	 *         out of {@link #preparePack(ProgressMonitor, Collection, Collection)} if an
 	 *         uninteresting object is not in the source repository. By default,
 	 *         true, permitting gracefully ignoring of uninteresting objects.
 	 */
@@ -504,6 +471,8 @@ public void preparePack(final Iterator<RevObject> objectsSource)
 	 * recency, path and delta-base first.
 	 * </p>
 	 *
+	 * @param countingMonitor
+	 *            progress during object enumeration.
 	 * @param interestingObjects
 	 *            collection of objects to be marked as interesting (start
 	 *            points of graph traversal).
@@ -513,13 +482,15 @@ public void preparePack(final Iterator<RevObject> objectsSource)
 	 * @throws IOException
 	 *             when some I/O problem occur during reading objects.
 	 */
-	public void preparePack(
+	public void preparePack(ProgressMonitor countingMonitor,
 			final Collection<? extends ObjectId> interestingObjects,
 			final Collection<? extends ObjectId> uninterestingObjects)
 			throws IOException {
+		if (countingMonitor == null)
+			countingMonitor = NullProgressMonitor.INSTANCE;
 		ObjectWalk walker = setUpWalker(interestingObjects,
 				uninterestingObjects);
-		findObjectsToPack(walker);
+		findObjectsToPack(countingMonitor, walker);
 	}
 
 	/**
@@ -553,7 +524,7 @@ public ObjectId computeName() {
 	 * Create an index file to match the pack file just written.
 	 * <p>
 	 * This method can only be invoked after {@link #preparePack(Iterator)} or
-	 * {@link #preparePack(Collection, Collection)} has been
+	 * {@link #preparePack(ProgressMonitor, Collection, Collection)} has been
 	 * invoked and completed successfully. Writing a corresponding index is an
 	 * optional feature that not all pack users may require.
 	 *
@@ -599,6 +570,10 @@ private List<ObjectToPack> sortByName() {
 	 * validated against existing checksum.
 	 * </p>
 	 *
+	 * @param compressMonitor
+	 *            progress monitor to report object compression work.
+	 * @param writeMonitor
+	 *            progress monitor to report the number of objects written.
 	 * @param packStream
 	 *            output stream of pack data. The stream should be buffered by
 	 *            the caller. The caller is responsible for closing the stream.
@@ -607,16 +582,23 @@ private List<ObjectToPack> sortByName() {
 	 *             the pack, or writing compressed object data to the output
 	 *             stream.
 	 */
-	public void writePack(OutputStream packStream) throws IOException {
+	public void writePack(ProgressMonitor compressMonitor,
-		if ((reuseDeltas || reuseObjects) && reuseSupport != null)
+			ProgressMonitor writeMonitor, OutputStream packStream)
-			searchForReuse();
+			throws IOException {
+		if (compressMonitor == null)
+			compressMonitor = NullProgressMonitor.INSTANCE;
+		if (writeMonitor == null)
+			writeMonitor = NullProgressMonitor.INSTANCE;
 
-		final PackOutputStream out = new PackOutputStream(packStream,
+		if ((reuseDeltas || reuseObjects) && reuseSupport != null)
-				isDeltaBaseAsOffset());
+			searchForReuse(compressMonitor);
+
+		final PackOutputStream out = new PackOutputStream(writeMonitor,
+				packStream, isDeltaBaseAsOffset());
 
 		writeMonitor.beginTask(WRITING_OBJECTS_PROGRESS, getObjectsNumber());
 		out.writeFileHeader(PACK_VERSION_GENERATED, getObjectsNumber());
-		writeObjects(out);
+		writeObjects(writeMonitor, out);
 		writeChecksum(out);
 
 		reader.release();
@@ -628,21 +610,23 @@ public void release() {
 		reader.release();
 	}
 
-	private void searchForReuse() throws IOException {
+	private void searchForReuse(ProgressMonitor compressMonitor)
-		initMonitor.beginTask(SEARCHING_REUSE_PROGRESS, getObjectsNumber());
+			throws IOException {
+		compressMonitor.beginTask(SEARCHING_REUSE_PROGRESS, getObjectsNumber());
 		for (List<ObjectToPack> list : objectsLists) {
 			for (ObjectToPack otp : list) {
-				if (initMonitor.isCancelled())
+				if (compressMonitor.isCancelled())
 					throw new IOException(
 							JGitText.get().packingCancelledDuringObjectsWriting);
 				reuseSupport.selectObjectRepresentation(this, otp);
-				initMonitor.update(1);
+				compressMonitor.update(1);
 			}
 		}
-		initMonitor.endTask();
+		compressMonitor.endTask();
 	}
 
-	private void writeObjects(PackOutputStream out) throws IOException {
+	private void writeObjects(ProgressMonitor writeMonitor, PackOutputStream out)
+			throws IOException {
 		for (List<ObjectToPack> list : objectsLists) {
 			for (ObjectToPack otp : list) {
 				if (writeMonitor.isCancelled())
@@ -669,8 +653,8 @@ private void writeObject(PackOutputStream out, final ObjectToPack otp)
 		while (otp.isReuseAsIs()) {
 			try {
 				reuseSupport.copyObjectAsIs(out, otp);
+				out.endObject();
 				otp.setCRC(out.getCRC32());
-				writeMonitor.update(1);
 				return;
 			} catch (StoredObjectRepresentationNotAvailableException gone) {
 				if (otp.getOffset() == out.length()) {
@@ -690,8 +674,8 @@ private void writeObject(PackOutputStream out, final ObjectToPack otp)
 		// If we reached here, reuse wasn't possible.
 		//
 		writeWholeObjectDeflate(out, otp);
+		out.endObject();
 		otp.setCRC(out.getCRC32());
-		writeMonitor.update(1);
 	}
 
 	private void writeBaseFirst(PackOutputStream out, final ObjectToPack otp)
@@ -781,22 +765,22 @@ private ObjectWalk setUpWalker(
 		return walker;
 	}
 
-	private void findObjectsToPack(final ObjectWalk walker)
+	private void findObjectsToPack(final ProgressMonitor countingMonitor,
-			throws MissingObjectException, IncorrectObjectTypeException,
+			final ObjectWalk walker) throws MissingObjectException,
-			IOException {
+			IncorrectObjectTypeException,			IOException {
-		initMonitor.beginTask(COUNTING_OBJECTS_PROGRESS,
+		countingMonitor.beginTask(COUNTING_OBJECTS_PROGRESS,
 				ProgressMonitor.UNKNOWN);
 		RevObject o;
 
 		while ((o = walker.next()) != null) {
 			addObject(o);
-			initMonitor.update(1);
+			countingMonitor.update(1);
 		}
 		while ((o = walker.nextObject()) != null) {
 			addObject(o);
-			initMonitor.update(1);
+			countingMonitor.update(1);
 		}
-		initMonitor.endTask();
+		countingMonitor.endTask();
 	}
 
 	/**

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

@@ -231,7 +231,7 @@ private void writePack(final Map<String, RemoteRefUpdate> refUpdates,
 		List<ObjectId> newObjects = new ArrayList<ObjectId>(refUpdates.size());
 
 		final long start;
-		final PackWriter writer = new PackWriter(local, monitor);
+		final PackWriter writer = new PackWriter(local);
 		try {
 
 			for (final Ref r : getRefs())
@@ -244,9 +244,9 @@ private void writePack(final Map<String, RemoteRefUpdate> refUpdates,
 
 			writer.setThin(thinPack);
 			writer.setDeltaBaseAsOffset(capableOfsDelta);
-			writer.preparePack(newObjects, remoteObjects);
+			writer.preparePack(monitor, newObjects, remoteObjects);
 			start = System.currentTimeMillis();
-			writer.writePack(out);
+			writer.writePack(monitor, monitor, out);
 		} finally {
 			writer.release();
 		}

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

@@ -92,11 +92,9 @@ public class BundleWriter {
 	 *
 	 * @param repo
 	 *            repository where objects are stored.
-	 * @param monitor
-	 *            operations progress monitor.
 	 */
-	public BundleWriter(final Repository repo, final ProgressMonitor monitor) {
+	public BundleWriter(final Repository repo) {
-		packWriter = new PackWriter(repo, monitor);
+		packWriter = new PackWriter(repo);
 		include = new TreeMap<String, ObjectId>();
 		assume = new HashSet<RevCommit>();
 	}
@@ -155,6 +153,8 @@ public void assume(final RevCommit c) {
 	 * <p>
 	 * This method can only be called once per BundleWriter instance.
 	 *
+	 * @param monitor
+	 *            progress monitor to report bundle writing status to.
 	 * @param os
 	 *            the stream the bundle is written to. The stream should be
 	 *            buffered by the caller. The caller is responsible for closing
@@ -164,7 +164,8 @@ public void assume(final RevCommit c) {
 	 *             the bundle, or writing compressed object data to the output
 	 *             stream.
 	 */
-	public void writeBundle(OutputStream os) throws IOException {
+	public void writeBundle(ProgressMonitor monitor, OutputStream os)
+			throws IOException {
 		try {
 			final HashSet<ObjectId> inc = new HashSet<ObjectId>();
 			final HashSet<ObjectId> exc = new HashSet<ObjectId>();
@@ -172,7 +173,7 @@ public void writeBundle(OutputStream os) throws IOException {
 			for (final RevCommit r : assume)
 				exc.add(r.getId());
 			packWriter.setThin(exc.size() > 0);
-			packWriter.preparePack(inc, exc);
+			packWriter.preparePack(monitor, inc, exc);
 
 			final Writer w = new OutputStreamWriter(os, Constants.CHARSET);
 			w.write(TransportBundle.V2_BUNDLE_SIGNATURE);
@@ -197,7 +198,7 @@ public void writeBundle(OutputStream os) throws IOException {
 
 			w.write('\n');
 			w.flush();
-			packWriter.writePack(os);
+			packWriter.writePack(monitor, monitor, os);
 		} finally {
 			packWriter.release();
 		}

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

@@ -568,11 +568,11 @@ private void sendPack() throws IOException {
 		}
 
 		final PackWriter pw;
-		pw = new PackWriter(db, pm, NullProgressMonitor.INSTANCE);
+		pw = new PackWriter(db);
 		try {
 			pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA));
 			pw.setThin(thin);
-			pw.preparePack(wantAll, commonBase);
+			pw.preparePack(pm, wantAll, commonBase);
 			if (options.contains(OPTION_INCLUDE_TAG)) {
 				for (final Ref r : refs.values()) {
 					final RevObject o;
@@ -588,7 +588,7 @@ private void sendPack() throws IOException {
 						pw.addObject(t);
 				}
 			}
-			pw.writePack(packOut);
+			pw.writePack(pm, NullProgressMonitor.INSTANCE, packOut);
 		} finally {
 			pw.release();
 		}

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

@@ -209,7 +209,7 @@ private void sendpack(final List<RemoteRefUpdate> updates,
 		String pathPack = null;
 		String pathIdx = null;
 
-		final PackWriter pw = new PackWriter(local, monitor);
+		final PackWriter pw = new PackWriter(local);
 		try {
 			final List<ObjectId> need = new ArrayList<ObjectId>();
 			final List<ObjectId> have = new ArrayList<ObjectId>();
@@ -220,7 +220,7 @@ private void sendpack(final List<RemoteRefUpdate> updates,
 				if (r.getPeeledObjectId() != null)
 					have.add(r.getPeeledObjectId());
 			}
-			pw.preparePack(need, have);
+			pw.preparePack(monitor, need, have);
 
 			// We don't have to continue further if the pack will
 			// be an empty pack, as the remote has all objects it
@@ -254,7 +254,7 @@ private void sendpack(final List<RemoteRefUpdate> updates,
 			OutputStream os = dest.writeFile(pathPack, monitor, wt + "..pack");
 			try {
 				os = new BufferedOutputStream(os);
-				pw.writePack(os);
+				pw.writePack(monitor, monitor, os);
 			} finally {
 				os.close();
 			}