Merge changes from topic 'fsck'

* changes:
  DfsFsck: reduce memory usage during verifyIndex
  DfsFsck: refactor pack verify into its own method
  DfsFsck: run connectivity check pass exactly once

2 files changed, 71 insertions, 51 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/fsck/FsckPackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/fsck/FsckPackParser.java
index e6ec6814b3..184bf416e0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/fsck/FsckPackParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/fsck/FsckPackParser.java
@@ -49,7 +49,6 @@ import java.nio.channels.Channels;
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import java.util.zip.CRC32;
 
@@ -65,6 +64,7 @@ import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.ObjectChecker;
 import org.eclipse.jgit.lib.ObjectDatabase;
+import org.eclipse.jgit.lib.ObjectIdOwnerMap;
 import org.eclipse.jgit.transport.PackParser;
 import org.eclipse.jgit.transport.PackedObjectInfo;
 
@@ -265,18 +265,18 @@ public class FsckPackParser extends PackParser {
 	/**
 	 * Verify the existing index file with all objects from the pack.
 	 *
-	 * @param entries
-	 *            all the entries that are expected in the index file
 	 * @param idx
 	 *            index file associate with the pack
 	 * @throws CorruptPackIndexException
 	 *             when the index file is corrupt.
 	 */
-	public void verifyIndex(List<PackedObjectInfo> entries, PackIndex idx)
+	public void verifyIndex(PackIndex idx)
 			throws CorruptPackIndexException {
-		Set<String> all = new HashSet<>();
-		for (PackedObjectInfo entry : entries) {
-			all.add(entry.getName());
+		ObjectIdOwnerMap<ObjFromPack> inPack = new ObjectIdOwnerMap<>();
+		for (int i = 0; i < getObjectCount(); i++) {
+			PackedObjectInfo entry = getObject(i);
+			inPack.add(new ObjFromPack(entry));
+
 			long offset = idx.findOffset(entry);
 			if (offset == -1) {
 				throw new CorruptPackIndexException(
@@ -305,7 +305,7 @@ public class FsckPackParser extends PackParser {
 		}
 
 		for (MutableEntry entry : idx) {
-			if (!all.contains(entry.name())) {
+			if (!inPack.contains(entry.toObjectId())) {
 				throw new CorruptPackIndexException(MessageFormat.format(
 						JGitText.get().unknownObjectInIndex, entry.name()),
 						ErrorType.UNKNOWN_OBJ);
@@ -323,4 +323,10 @@ public class FsckPackParser extends PackParser {
 	public void overwriteObjectCount(long expectedObjectCount) {
 		this.expectedObjectCount = expectedObjectCount;
 	}
+
+	static class ObjFromPack extends ObjectIdOwnerMap.Entry {
+		ObjFromPack(AnyObjectId id) {
+			super(id);
+		}
+	}
 }
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..75eade2273 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
@@ -43,31 +43,30 @@
 
 package org.eclipse.jgit.internal.storage.dfs;
 
+import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
+import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
+
+import java.io.FileNotFoundException;
 import java.io.IOException;
-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;
 import org.eclipse.jgit.revwalk.ObjectWalk;
 import org.eclipse.jgit.revwalk.RevObject;
-import org.eclipse.jgit.transport.PackedObjectInfo;
 
 /** Verify the validity and connectivity of a DFS repository. */
 public class DfsFsck {
 	private final DfsRepository repo;
-
 	private final DfsObjDatabase objdb;
-
-	private final DfsReader ctx;
-
 	private ObjectChecker objChecker = new ObjectChecker();
 
 	/**
@@ -79,10 +78,8 @@ public class DfsFsck {
 	public DfsFsck(DfsRepository repository) {
 		repo = repository;
 		objdb = repo.getObjectDatabase();
-		ctx = objdb.newReader();
 	}
 
-
 	/**
 	 * Verify the integrity and connectivity of all objects in the object
 	 * database.
@@ -94,55 +91,72 @@ 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 {
+		checkPacks(pm, errors);
+		checkConnectivity(pm, errors);
+		return errors;
+	}
+
+	private void checkPacks(ProgressMonitor pm, FsckError errors)
+			throws IOException, FileNotFoundException {
+		try (DfsReader ctx = objdb.newReader()) {
 			for (DfsPackFile pack : objdb.getPacks()) {
 				DfsPackDescription packDesc = pack.getPackDescription();
-				try (ReadableChannel channel = repo.getObjectDatabase()
-						.openFile(packDesc, PackExt.PACK)) {
-					List<PackedObjectInfo> objectsInPack;
-					FsckPackParser parser = new FsckPackParser(
-							repo.getObjectDatabase(), channel);
-					parser.setObjectChecker(objChecker);
-					parser.overwriteObjectCount(packDesc.getObjectCount());
-					parser.parse(pm);
-					errors.getCorruptObjects()
-							.addAll(parser.getCorruptObjects());
-					objectsInPack = parser.getSortedObjectList(null);
-					parser.verifyIndex(objectsInPack, pack.getPackIndex(ctx));
+				try (ReadableChannel rc = objdb.openFile(packDesc, PACK)) {
+					verifyPack(pm, errors, ctx, pack, rc);
 				} catch (MissingObjectException e) {
 					errors.getMissingObjects().add(e.getObjectId());
 				} catch (CorruptPackIndexException e) {
 					errors.getCorruptIndices().add(new CorruptIndex(
-							pack.getPackDescription()
-									.getFileName(PackExt.INDEX),
+							pack.getPackDescription().getFileName(INDEX),
 							e.getErrorType()));
 				}
 			}
+		}
+	}
+
+	private void verifyPack(ProgressMonitor pm, FsckError errors, DfsReader ctx,
+			DfsPackFile pack, ReadableChannel ch)
+					throws IOException, CorruptPackIndexException {
+		FsckPackParser fpp = new FsckPackParser(objdb, ch);
+		fpp.setObjectChecker(objChecker);
+		fpp.overwriteObjectCount(pack.getPackDescription().getObjectCount());
+		fpp.parse(pm);
+		errors.getCorruptObjects().addAll(fpp.getCorruptObjects());
+
+		fpp.verifyIndex(pack.getPackIndex(ctx));
+	}
 
-			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());
+	private void checkConnectivity(ProgressMonitor pm, FsckError errors)
+			throws IOException {
+		pm.beginTask(JGitText.get().countingObjects, ProgressMonitor.UNKNOWN);
+		try (ObjectWalk ow = new ObjectWalk(repo)) {
+			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());
 			}
-		} finally {
-			ctx.close();
 		}
-		return errors;
+		pm.endTask();
 	}
 
 	/**