Remove DFS locality ordering during packing

PackWriter generally chooses the order for objects when it builds the
object lists.  This ordering already depends on history information to
guide placing more recent objects first and historical objects last.

Allow PackWriter to make the basic ordering decisions, instead of
trying to override them.  The old approach of sorting the list caused
DfsReader to override any ordering change PackWriter might have tried
to make when repacking a repository.

This now better matches with WindowCursor's implementation, where
PackWriter solely determines the object ordering.

Change-Id: Ic17ab5631ec539f0758b962966c3a1823735b814

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectRepresentation.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectRepresentation.java
index 45bf7f3b4eb8758b0b1c3fc227ec05da01875a63..7edae88110bbf15cc53cea9a9d65fe25ed2dc956 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectRepresentation.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectRepresentation.java
@@ -50,15 +50,13 @@ import org.eclipse.jgit.lib.ObjectId;
 
 class DfsObjectRepresentation extends StoredObjectRepresentation {
        final DfsPackFile pack;
-       final int packIndex;
        int format;
        long offset;
        long length;
        ObjectId baseId;
 
-       DfsObjectRepresentation(DfsPackFile pack, int packIndex) {
+       DfsObjectRepresentation(DfsPackFile pack) {
                this.pack = pack;
-               this.packIndex = packIndex;
        }
 
        @Override

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectToPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectToPack.java
index 469f9a77329fbfd96f795c0a5e1c466a213ed2c6..ff270bf9744aa6bc30e1d80f7601ae496002623f 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectToPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjectToPack.java
@@ -54,9 +54,6 @@ class DfsObjectToPack extends ObjectToPack {
        /** Pack to reuse compressed data from, otherwise null. */
        DfsPackFile pack;
 
-       /** Position of the pack in the reader's pack list. */
-       int packIndex;
-
        /** Offset of the object's header in {@link #pack}. */
        long offset;
 
@@ -85,7 +82,6 @@ class DfsObjectToPack extends ObjectToPack {
        public void select(StoredObjectRepresentation ref) {
                DfsObjectRepresentation ptr = (DfsObjectRepresentation) ref;
                this.pack = ptr.pack;
-               this.packIndex = ptr.packIndex;
                this.offset = ptr.offset;
                this.length = ptr.length;
        }

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 fff6f301d1e43d5d5c67c82feb2bf4f3dea4f7ca..26fc03583454ed736d382923cce7cd13adf11833 100644 (file)
--- 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
@@ -46,8 +46,6 @@ package org.eclipse.jgit.internal.storage.dfs;
 
 import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
 import static org.eclipse.jgit.lib.Constants.OBJECT_ID_LENGTH;
-import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
-import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
 
 import java.io.IOException;
 import java.security.MessageDigest;
@@ -454,17 +452,13 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
        public void selectObjectRepresentation(PackWriter packer,
                        ProgressMonitor monitor, Iterable<ObjectToPack> objects)
                        throws IOException, MissingObjectException {
-               DfsPackFile[] packList = db.getPacks();
-               for (int packIndex = 0; packIndex < packList.length; packIndex++) {
-                       DfsPackFile pack = packList[packIndex];
+               for (DfsPackFile pack : db.getPacks()) {
                        List<DfsObjectToPack> tmp = findAllFromPack(pack, objects);
                        if (tmp.isEmpty())
                                continue;
                        Collections.sort(tmp, OFFSET_SORT);
                        PackReverseIndex rev = pack.getReverseIdx(this);
-                       DfsObjectRepresentation rep = new DfsObjectRepresentation(
-                                       pack,
-                                       packIndex);
+                       DfsObjectRepresentation rep = new DfsObjectRepresentation(pack);
                        for (DfsObjectToPack otp : tmp) {
                                pack.representation(rep, otp.getOffset(), this, rev);
                                otp.setOffset(0);
@@ -498,41 +492,8 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
                src.pack.copyAsIs(out, src, validate, this);
        }
 
-       private static final Comparator<ObjectToPack> WRITE_SORT = new Comparator<ObjectToPack>() {
-               public int compare(ObjectToPack o1, ObjectToPack o2) {
-                       DfsObjectToPack a = (DfsObjectToPack) o1;
-                       DfsObjectToPack b = (DfsObjectToPack) o2;
-                       int cmp = a.packIndex - b.packIndex;
-                       if (cmp == 0)
-                               cmp = Long.signum(a.offset - b.offset);
-                       return cmp;
-               }
-       };
-
        public void writeObjects(PackOutputStream out, List<ObjectToPack> list)
                        throws IOException {
-               if (list.isEmpty())
-                       return;
-
-               // Sorting objects by order in the current packs is usually
-               // worthwhile. Most packs are going to be OFS_DELTA style,
-               // where the base must appear before the deltas. If both base
-               // and delta are to be reused, this ensures the base writes in
-               // the output first without the recursive write-base-first logic
-               // used by PackWriter to ensure OFS_DELTA can be used.
-               //
-               // Sorting by pack also ensures newer objects go first, which
-               // typically matches the desired order.
-               //
-               // Only do this sorting for OBJ_TREE and OBJ_BLOB. Commits
-               // are very likely to already be sorted in a good order in the
-               // incoming list, and if they aren't, JGit's PackWriter has fixed
-               // the order to be more optimal for readers, so honor that.
-               switch (list.get(0).getType()) {
-               case OBJ_TREE:
-               case OBJ_BLOB:
-                       Collections.sort(list, WRITE_SORT);
-               }
                for (ObjectToPack otp : list)
                        out.writeObject(otp);
        }