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
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 f9c01b9..6339b03 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 @@ private void addObjectsToPack(PackWriter pw, DfsReader ctx, 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 45dbe75..618969f 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.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.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.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 @@ long findOffset(DfsReader ctx, AnyObjectId id) throws IOException { 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 @@ long getIndexedObjectSize(DfsReader ctx, AnyObjectId id) 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 @@ void representation(DfsObjectRepresentation r, final long pos, 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 @@ void representation(DfsObjectRepresentation r, final long pos, 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 62f6753..04288bc 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.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.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 DfsObjectToPack newObjectToPack(AnyObjectId objectId, int type) { 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 @@ private void trySelectRepresentation(PackWriter packer, 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 @@ private static boolean checkGarbagePacks(Iterable<ObjectToPack> objects) { 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,