DfsFsck: run connectivity check pass exactly once

The simpler algorithm is to load all branch tips into an ObjectWalk
and run that walk exactly once.  This avoids redoing work related to
parsing and considering trees reused across side branches.

Move the connectivity check into its own helper method.  This moves it
left one level of identation, and makes it easier to fit the method's
logic with less line wrapping.

Add a "Counting objects..." progress monitor around this phase.  Its
what is used when a server receives a push and is also trying to
verify the client sent all required objects.

Change-Id: I4d53d75d0cdd1a13fff7d513a6ae0b2d14ea4090

1 files changed, 35 insertions, 19 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsFsck.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsFsck.java
index f90ba7d986..7468bf5c9d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsFsck.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsFsck.java
@@ -48,11 +48,13 @@ import java.util.List;
 
 import org.eclipse.jgit.errors.CorruptPackIndexException;
 import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.fsck.FsckError;
 import org.eclipse.jgit.internal.fsck.FsckError.CorruptIndex;
 import org.eclipse.jgit.internal.fsck.FsckPackParser;
 import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectChecker;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
@@ -94,6 +96,10 @@ public class DfsFsck {
 	 *             if encounters IO errors during the process.
 	 */
 	public FsckError check(ProgressMonitor pm) throws IOException {
+		if (pm == null) {
+			pm = NullProgressMonitor.INSTANCE;
+		}
+
 		FsckError errors = new FsckError();
 		try {
 			for (DfsPackFile pack : objdb.getPacks()) {
@@ -120,31 +126,41 @@ public class DfsFsck {
 				}
 			}
 
-			try (ObjectWalk ow = new ObjectWalk(ctx)) {
-				for (Ref r : repo.getAllRefs().values()) {
-					try {
-						RevObject tip = ow.parseAny(r.getObjectId());
-						if (r.getLeaf().getName().startsWith(Constants.R_HEADS)) {
-							// check if heads point to a commit object
-							if (tip.getType() != Constants.OBJ_COMMIT) {
-								errors.getNonCommitHeads()
-										.add(r.getLeaf().getName());
-							}
-						}
-						ow.markStart(tip);
-						ow.checkConnectivity();
-						ow.markUninteresting(tip);
-					} catch (MissingObjectException e) {
-						errors.getMissingObjects().add(e.getObjectId());
-					}
-				}
-			}
+			checkConnectivity(pm, errors);
 		} finally {
 			ctx.close();
 		}
 		return errors;
 	}
 
+	private void checkConnectivity(ProgressMonitor pm, FsckError errors)
+			throws IOException {
+		pm.beginTask(JGitText.get().countingObjects, ProgressMonitor.UNKNOWN);
+		try (ObjectWalk ow = new ObjectWalk(ctx)) {
+			for (Ref r : repo.getAllRefs().values()) {
+				RevObject tip;
+				try {
+					tip = ow.parseAny(r.getObjectId());
+					if (r.getLeaf().getName().startsWith(Constants.R_HEADS)
+							&& tip.getType() != Constants.OBJ_COMMIT) {
+						// heads should only point to a commit object
+						errors.getNonCommitHeads().add(r.getLeaf().getName());
+					}
+				} catch (MissingObjectException e) {
+					errors.getMissingObjects().add(e.getObjectId());
+					continue;
+				}
+				ow.markStart(tip);
+			}
+			try {
+				ow.checkConnectivity();
+			} catch (MissingObjectException e) {
+				errors.getMissingObjects().add(e.getObjectId());
+			}
+		}
+		pm.endTask();
+	}
+
 	/**
 	 * Use a customized object checker instead of the default one. Caller can
 	 * specify a skip list to ignore some errors.