DfsReader/PackFile: Move represention to the packfile

The reader iterates the packs looking for the best representation of
an object. It does so using the primary and reverse indexes of the
pack. This direct use of the indexes prevent the pack to be more
clever and use the multipack index in these operations.

Move finding objects in the pack and the representation to the pack
itself. Now the pack can decide how to implement it (e.g. using the
multipack index).

Change-Id: Ief0f2384ab395557c2533990f7b3c532a88d7ac9

3 files changed, 92 insertions, 39 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java
index f9c01b9d6e..6339b0326a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java
@@ -405,7 +405,7 @@ public class DfsPackCompactor {
 				pw.addObject(obj);
 				obj.add(added);
 
-				src.representation(rep, id.offset, ctx, rev);
+				src.fillRepresentation(rep, id.offset, ctx, rev);
 				if (rep.getFormat() != PACK_DELTA)
 					continue;
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
index 45dbe75107..618969f62e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
@@ -27,6 +27,9 @@ import java.io.InputStream;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.text.MessageFormat;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -49,6 +52,7 @@ import org.eclipse.jgit.internal.storage.file.PackObjectSizeIndexLoader;
 import org.eclipse.jgit.internal.storage.file.PackReverseIndex;
 import org.eclipse.jgit.internal.storage.file.PackReverseIndexFactory;
 import org.eclipse.jgit.internal.storage.pack.BinaryDelta;
+import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
 import org.eclipse.jgit.internal.storage.pack.PackOutputStream;
 import org.eclipse.jgit.internal.storage.pack.StoredObjectRepresentation;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
@@ -59,6 +63,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.util.BlockList;
 import org.eclipse.jgit.util.LongList;
 
 /**
@@ -71,6 +76,10 @@ public final class DfsPackFile extends BlockBasedFile {
 
 	private static final long REF_POSITION = 0;
 
+	private static final Comparator<DfsObjectToPack> OFFSET_SORT = (
+			DfsObjectToPack a,
+			DfsObjectToPack b) -> Long.signum(a.getOffset() - b.getOffset());
+
 	/**
 	 * Loader for the default file-based {@link PackBitmapIndex} implementation.
 	 */
@@ -455,6 +464,40 @@ public final class DfsPackFile extends BlockBasedFile {
 		return idx(ctx).findOffset(id);
 	}
 
+	/**
+	 * Return objects in the list available in this pack, sorted in (pack,
+	 * offset) order.
+	 *
+	 * @param ctx
+	 *            a reader
+	 * @param objects
+	 *            objects we are looking for
+	 * @param skipFound
+	 *            ignore objects already found.
+	 * @return list of objects with pack and offset set.
+	 * @throws IOException
+	 *             an error occurred
+	 */
+	List<DfsObjectToPack> findAllFromPack(DfsReader ctx,
+			Iterable<ObjectToPack> objects, boolean skipFound)
+			throws IOException {
+		List<DfsObjectToPack> tmp = new BlockList<>();
+		for (ObjectToPack obj : objects) {
+			DfsObjectToPack otp = (DfsObjectToPack) obj;
+			if (skipFound && otp.isFound()) {
+				continue;
+			}
+			long p = idx(ctx).findOffset(otp);
+			if (p <= 0 || isCorrupt(p)) {
+				continue;
+			}
+			otp.setOffset(p);
+			tmp.add(otp);
+		}
+		Collections.sort(tmp, OFFSET_SORT);
+		return tmp;
+	}
+
 	void resolve(DfsReader ctx, Set<ObjectId> matches, AbbreviatedObjectId id,
 			int matchLimit) throws IOException {
 		idx(ctx).resolve(matches, id, matchLimit);
@@ -1155,12 +1198,47 @@ public final class DfsPackFile extends BlockBasedFile {
 		return sizeIdx.getSize(idxPosition);
 	}
 
-	void representation(DfsObjectRepresentation r, final long pos,
+	/**
+	 * Populates the representation object with the details of how the object at
+	 * "pos" is stored in this pack (e.g. whole or deltified, its packed
+	 * length).
+	 *
+	 * @param r
+	 *            represention object to carry data
+	 * @param offset
+	 *            offset in this pack of the object
+	 * @param ctx
+	 *            a reader
+	 * @throws IOException
+	 *             an error reading the object from disk
+	 */
+	void fillRepresentation(DfsObjectRepresentation r, long offset,
+			DfsReader ctx) throws IOException {
+		fillRepresentation(r, offset, ctx, getReverseIdx(ctx));
+	}
+
+	/**
+	 * Populates the representation object with the details of how the object at
+	 * "pos" is stored in this pack (e.g. whole or deltified, its packed
+	 * length).
+	 *
+	 * @param r
+	 *            represention object to carry data
+	 * @param offset
+	 *            offset in this pack of the object
+	 * @param ctx
+	 *            a reader
+	 * @param rev
+	 *            reverse index of this pack
+	 * @throws IOException
+	 *             an error reading the object from disk
+	 */
+	void fillRepresentation(DfsObjectRepresentation r, long offset,
 			DfsReader ctx, PackReverseIndex rev)
 			throws IOException {
-		r.offset = pos;
+		r.offset = offset;
 		final byte[] ib = ctx.tempId;
-		readFully(pos, ib, 0, 20, ctx);
+		readFully(offset, ib, 0, 20, ctx);
 		int c = ib[0] & 0xff;
 		int p = 1;
 		final int typeCode = (c >> 4) & 7;
@@ -1168,7 +1246,7 @@ public final class DfsPackFile extends BlockBasedFile {
 			c = ib[p++] & 0xff;
 		}
 
-		long len = rev.findNextOffset(pos, length - 20) - pos;
+		long len = rev.findNextOffset(offset, length - 20) - offset;
 		switch (typeCode) {
 		case Constants.OBJ_COMMIT:
 		case Constants.OBJ_TREE:
@@ -1189,13 +1267,13 @@ public final class DfsPackFile extends BlockBasedFile {
 				ofs += (c & 127);
 			}
 			r.format = StoredObjectRepresentation.PACK_DELTA;
-			r.baseId = rev.findObject(pos - ofs);
+			r.baseId = rev.findObject(offset - ofs);
 			r.length = len - p;
 			return;
 		}
 
 		case Constants.OBJ_REF_DELTA: {
-			readFully(pos + p, ib, 0, 20, ctx);
+			readFully(offset + p, ib, 0, 20, ctx);
 			r.format = StoredObjectRepresentation.PACK_DELTA;
 			r.baseId = ObjectId.fromRaw(ib);
 			r.length = len - p - 20;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java
index 62f6753e5d..04288bc6c6 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java
@@ -38,8 +38,6 @@ import org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource;
 import org.eclipse.jgit.internal.storage.dfs.DfsReader.PackLoadListener.DfsBlockData;
 import org.eclipse.jgit.internal.storage.file.BitmapIndexImpl;
 import org.eclipse.jgit.internal.storage.file.PackBitmapIndex;
-import org.eclipse.jgit.internal.storage.file.PackIndex;
-import org.eclipse.jgit.internal.storage.file.PackReverseIndex;
 import org.eclipse.jgit.internal.storage.pack.CachedPack;
 import org.eclipse.jgit.internal.storage.pack.ObjectReuseAsIs;
 import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
@@ -58,7 +56,6 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ProgressMonitor;
-import org.eclipse.jgit.util.BlockList;
 
 /**
  * Reader to access repository content through.
@@ -619,10 +616,6 @@ public class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 		return new DfsObjectToPack(objectId, type);
 	}
 
-	private static final Comparator<DfsObjectToPack> OFFSET_SORT = (
-			DfsObjectToPack a,
-			DfsObjectToPack b) -> Long.signum(a.getOffset() - b.getOffset());
-
 	@Override
 	public void selectObjectRepresentation(PackWriter packer,
 			ProgressMonitor monitor, Iterable<ObjectToPack> objects)
@@ -642,16 +635,15 @@ public class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 			ProgressMonitor monitor, Iterable<ObjectToPack> objects,
 			List<DfsPackFile> packs, boolean skipFound) throws IOException {
 		for (DfsPackFile pack : packs) {
-			List<DfsObjectToPack> tmp = findAllFromPack(pack, objects, skipFound);
-			if (tmp.isEmpty())
+			List<DfsObjectToPack> inPack = pack.findAllFromPack(this, objects, skipFound);
+			if (inPack.isEmpty())
 				continue;
-			Collections.sort(tmp, OFFSET_SORT);
-			PackReverseIndex rev = pack.getReverseIdx(this);
 			DfsObjectRepresentation rep = new DfsObjectRepresentation(pack);
-			for (DfsObjectToPack otp : tmp) {
-				pack.representation(rep, otp.getOffset(), this, rev);
+			for (DfsObjectToPack otp : inPack) {
+				// Populate rep.{offset,length} from the pack
+				pack.fillRepresentation(rep, otp.getOffset(), this);
 				otp.setOffset(0);
-				packer.select(otp, rep);
+				packer.select(otp, rep); // Set otp.offset from rep
 				if (!otp.isFound()) {
 					otp.setFound();
 					monitor.update(1);
@@ -698,24 +690,7 @@ public class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 		return false;
 	}
 
-	private List<DfsObjectToPack> findAllFromPack(DfsPackFile pack,
-			Iterable<ObjectToPack> objects, boolean skipFound)
-					throws IOException {
-		List<DfsObjectToPack> tmp = new BlockList<>();
-		PackIndex idx = pack.getPackIndex(this);
-		for (ObjectToPack obj : objects) {
-			DfsObjectToPack otp = (DfsObjectToPack) obj;
-			if (skipFound && otp.isFound()) {
-				continue;
-			}
-			long p = idx.findOffset(otp);
-			if (0 < p && !pack.isCorrupt(p)) {
-				otp.setOffset(p);
-				tmp.add(otp);
-			}
-		}
-		return tmp;
-	}
+
 
 	@Override
 	public void copyObjectAsIs(PackOutputStream out, ObjectToPack otp,