]> source.dussan.org Git - jgit.git/commitdiff
Remove cached_packs support in favor of bitmaps 83/11183/1
authorShawn Pearce <spearce@spearce.org>
Thu, 14 Mar 2013 23:27:53 +0000 (16:27 -0700)
committerShawn Pearce <spearce@spearce.org>
Thu, 14 Mar 2013 23:36:57 +0000 (16:36 -0700)
The bitmap code in PackWriter knows exactly when to use a pack as
a "cached pack". It enables cached pack usage only when the pack
has a bitmap and its entire closure of objects needs to be sent.
This is a much simpler code path to maintain, and JGit actually
has a way to write the necessary index.

Change-Id: I2645d482f8733fdf0c4120cc59ba9aa4d4ba6881

12 files changed:
org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsCachedPack.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackDescription.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LocalCachedPack.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCursor.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/CachedPack.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/ObjectReuseAsIs.java
org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java

index e0ecc8018a0aaf2e911ade7ff21400f3f0b006a9..1f9cfe83ced565a5e532529da1de2bb7779e2897 100644 (file)
@@ -44,9 +44,7 @@
 package org.eclipse.jgit.storage.dfs;
 
 import java.io.IOException;
-import java.util.Set;
 
-import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.storage.pack.CachedPack;
 import org.eclipse.jgit.storage.pack.ObjectToPack;
 import org.eclipse.jgit.storage.pack.PackOutputStream;
@@ -65,11 +63,6 @@ public class DfsCachedPack extends CachedPack {
                return pack.getPackDescription();
        }
 
-       @Override
-       public Set<ObjectId> getTips() {
-               return getPackDescription().getTips();
-       }
-
        @Override
        public long getObjectCount() throws IOException {
                return getPackDescription().getObjectCount();
index 8f175a8227878297d79a179fc028893778b679ea..7f8a78abe56a188fbaec716741b9ea2345bc1ac1 100644 (file)
@@ -275,7 +275,7 @@ public class DfsGarbageCollector {
                try {
                        pw.preparePack(pm, allHeads, Collections.<ObjectId> emptySet());
                        if (0 < pw.getObjectCount())
-                               writePack(GC, pw, pm).setTips(allHeads);
+                               writePack(GC, pw, pm);
                } finally {
                        pw.release();
                }
index c4a531e7be1ae4cb089d384b1f40d218173aab6d..cd9766b5643755ebb1bae85f7d36950844779623 100644 (file)
@@ -47,9 +47,7 @@ import static org.eclipse.jgit.storage.pack.PackExt.PACK;
 
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 
-import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.storage.dfs.DfsObjDatabase.PackSource;
 import org.eclipse.jgit.storage.pack.PackExt;
 import org.eclipse.jgit.storage.pack.PackWriter;
@@ -77,8 +75,6 @@ public class DfsPackDescription implements Comparable<DfsPackDescription> {
 
        private long deltaCount;
 
-       private Set<ObjectId> tips;
-
        private PackWriter.Statistics stats;
 
        private int extensions;
@@ -223,21 +219,6 @@ public class DfsPackDescription implements Comparable<DfsPackDescription> {
                return this;
        }
 
-       /** @return the tips that created this pack, if known. */
-       public Set<ObjectId> getTips() {
-               return tips;
-       }
-
-       /**
-        * @param tips
-        *            the tips of the pack, null if it has no known tips.
-        * @return {@code this}
-        */
-       public DfsPackDescription setTips(Set<ObjectId> tips) {
-               this.tips = tips;
-               return this;
-       }
-
        /**
         * @return statistics from PackWriter, if the pack was built with it.
         *         Generally this is only available for packs created by
index 401c483d4db604f331211f888906f8fb66772040..9409ec117aa0452c9a4d733b924e6bf4c9c7280d 100644 (file)
@@ -593,21 +593,6 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
                }
        }
 
-       public Collection<CachedPack> getCachedPacks() throws IOException {
-               DfsPackFile[] packList = db.getPacks();
-               List<CachedPack> cached = new ArrayList<CachedPack>(packList.length);
-               for (DfsPackFile pack : packList) {
-                       DfsPackDescription desc = pack.getPackDescription();
-                       if (canBeCachedPack(desc))
-                               cached.add(new DfsCachedPack(pack));
-               }
-               return cached;
-       }
-
-       private static boolean canBeCachedPack(DfsPackDescription desc) {
-               return desc.getTips() != null && !desc.getTips().isEmpty();
-       }
-
        public void copyPackAsIs(PackOutputStream out, CachedPack pack,
                        boolean validate) throws IOException {
                try {
index 939b9cf3c1bf7a8ba2da6b9bd7dfd7e8de6c25ab..fc55be9b110cb89f04426af1db3a648f29ade312 100644 (file)
@@ -57,7 +57,6 @@ import org.eclipse.jgit.lib.ObjectDatabase;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdOwnerMap;
 import org.eclipse.jgit.lib.ObjectLoader;
-import org.eclipse.jgit.storage.pack.CachedPack;
 import org.eclipse.jgit.storage.pack.ObjectToPack;
 import org.eclipse.jgit.storage.pack.PackWriter;
 import org.eclipse.jgit.util.FS;
@@ -146,11 +145,6 @@ class CachedObjectDirectory extends FileObjectDatabase {
                return wrapped.getShallowCommits();
        }
 
-       @Override
-       Collection<? extends CachedPack> getCachedPacks() throws IOException {
-               return wrapped.getCachedPacks();
-       }
-
        @Override
        AlternateHandle[] myAlternates() {
                if (alts == null) {
index ed0577f804b3bdf1a8eb0136d53e604c764f5b1d..da0465cd2a629e1fbee4bfe15d356ccd05c07deb 100644 (file)
@@ -55,7 +55,6 @@ import org.eclipse.jgit.lib.ObjectDatabase;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.storage.pack.CachedPack;
 import org.eclipse.jgit.storage.pack.ObjectToPack;
 import org.eclipse.jgit.storage.pack.PackWriter;
 import org.eclipse.jgit.util.FS;
@@ -262,9 +261,6 @@ abstract class FileObjectDatabase extends ObjectDatabase {
 
        abstract File getDirectory();
 
-       abstract Collection<? extends CachedPack> getCachedPacks()
-                       throws IOException;
-
        abstract AlternateHandle[] myAlternates();
 
        abstract boolean tryAgain1();
@@ -301,11 +297,6 @@ abstract class FileObjectDatabase extends ObjectDatabase {
                        this.db = db;
                }
 
-               @SuppressWarnings("unchecked")
-               Collection<CachedPack> getCachedPacks() throws IOException {
-                       return (Collection<CachedPack>) db.getCachedPacks();
-               }
-
                void close() {
                        db.close();
                }
index c0e47eea12ffea58f4120f8022a213482faa7ae6..9534b961600ffcadb6eb3d46ddeb50297cf26019 100644 (file)
@@ -46,11 +46,8 @@ package org.eclipse.jgit.storage.file;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 
-import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.storage.pack.CachedPack;
 import org.eclipse.jgit.storage.pack.ObjectToPack;
 import org.eclipse.jgit.storage.pack.PackOutputStream;
@@ -59,36 +56,21 @@ import org.eclipse.jgit.storage.pack.StoredObjectRepresentation;
 class LocalCachedPack extends CachedPack {
        private final ObjectDirectory odb;
 
-       private final Set<ObjectId> tips;
-
        private final String[] packNames;
 
        private PackFile[] packs;
 
-       LocalCachedPack(ObjectDirectory odb, Set<ObjectId> tips,
-                       List<String> packNames) {
+       LocalCachedPack(ObjectDirectory odb, List<String> packNames) {
                this.odb = odb;
-
-               if (tips.size() == 1)
-                       this.tips = Collections.singleton(tips.iterator().next());
-               else
-                       this.tips = Collections.unmodifiableSet(tips);
-
                this.packNames = packNames.toArray(new String[packNames.size()]);
        }
 
        LocalCachedPack(List<PackFile> packs) {
                odb = null;
-               tips = null;
                packNames = null;
                this.packs = packs.toArray(new PackFile[packs.size()]);
        }
 
-       @Override
-       public Set<ObjectId> getTips() {
-               return tips;
-       }
-
        @Override
        public long getObjectCount() throws IOException {
                long cnt = 0;
index 99ceeb74bf0f46a74f78de42b493b6fa506b6381..2f1aadaf4084c658dc62c800c27a1ca804aa32fc 100644 (file)
@@ -75,14 +75,11 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.RepositoryCache;
 import org.eclipse.jgit.lib.RepositoryCache.FileKey;
-import org.eclipse.jgit.storage.pack.CachedPack;
 import org.eclipse.jgit.storage.pack.ObjectToPack;
 import org.eclipse.jgit.storage.pack.PackExt;
 import org.eclipse.jgit.storage.pack.PackWriter;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.FileUtils;
-import org.eclipse.jgit.util.IO;
-import org.eclipse.jgit.util.RawParseUtils;
 
 /**
  * Traditional file system based {@link ObjectDatabase}.
@@ -119,12 +116,8 @@ public class ObjectDirectory extends FileObjectDatabase {
 
        private final File alternatesFile;
 
-       private final File cachedPacksFile;
-
        private final AtomicReference<PackList> packList;
 
-       private final AtomicReference<CachedPackList> cachedPacks;
-
        private final FS fs;
 
        private final AtomicReference<AlternateHandle[]> alternates;
@@ -162,9 +155,7 @@ public class ObjectDirectory extends FileObjectDatabase {
                infoDirectory = new File(objects, "info"); //$NON-NLS-1$
                packDirectory = new File(objects, "pack"); //$NON-NLS-1$
                alternatesFile = new File(infoDirectory, "alternates"); //$NON-NLS-1$
-               cachedPacksFile = new File(infoDirectory, "cached-packs"); //$NON-NLS-1$
                packList = new AtomicReference<PackList>(NO_PACKS);
-               cachedPacks = new AtomicReference<CachedPackList>();
                unpackedObjectCache = new UnpackedObjectCache();
                this.fs = fs;
                this.shallowFile = shallowFile;
@@ -250,83 +241,6 @@ public class ObjectDirectory extends FileObjectDatabase {
                return Collections.unmodifiableCollection(Arrays.asList(packs));
        }
 
-       @Override
-       Collection<? extends CachedPack> getCachedPacks() throws IOException {
-               CachedPackList list = cachedPacks.get();
-               if (list == null || list.snapshot.isModified(cachedPacksFile))
-                       list = scanCachedPacks(list);
-
-               Collection<CachedPack> result = list.getCachedPacks();
-               boolean resultIsCopy = false;
-
-               for (AlternateHandle h : myAlternates()) {
-                       Collection<CachedPack> altPacks = h.getCachedPacks();
-                       if (altPacks.isEmpty())
-                               continue;
-
-                       if (result.isEmpty()) {
-                               result = altPacks;
-                               continue;
-                       }
-
-                       if (!resultIsCopy) {
-                               result = new ArrayList<CachedPack>(result);
-                               resultIsCopy = true;
-                       }
-                       result.addAll(altPacks);
-               }
-               return result;
-       }
-
-       private CachedPackList scanCachedPacks(CachedPackList old)
-                       throws IOException {
-               FileSnapshot s = FileSnapshot.save(cachedPacksFile);
-               byte[] buf;
-               try {
-                       buf = IO.readFully(cachedPacksFile);
-               } catch (FileNotFoundException e) {
-                       buf = new byte[0];
-               }
-
-               if (old != null && old.snapshot.equals(s)
-                               && Arrays.equals(old.raw, buf)) {
-                       old.snapshot.setClean(s);
-                       return old;
-               }
-
-               ArrayList<LocalCachedPack> list = new ArrayList<LocalCachedPack>(4);
-               Set<ObjectId> tips = new HashSet<ObjectId>();
-               int ptr = 0;
-               while (ptr < buf.length) {
-                       if (buf[ptr] == '#' || buf[ptr] == '\n') {
-                               ptr = RawParseUtils.nextLF(buf, ptr);
-                               continue;
-                       }
-
-                       if (buf[ptr] == '+') {
-                               tips.add(ObjectId.fromString(buf, ptr + 2));
-                               ptr = RawParseUtils.nextLF(buf, ptr + 2);
-                               continue;
-                       }
-
-                       List<String> names = new ArrayList<String>(4);
-                       while (ptr < buf.length && buf[ptr] == 'P') {
-                               int end = RawParseUtils.nextLF(buf, ptr);
-                               if (buf[end - 1] == '\n')
-                                       end--;
-                               names.add(RawParseUtils.decode(buf, ptr + 2, end));
-                               ptr = RawParseUtils.nextLF(buf, end);
-                       }
-
-                       if (!tips.isEmpty() && !names.isEmpty()) {
-                               list.add(new LocalCachedPack(this, tips, names));
-                               tips = new HashSet<ObjectId>();
-                       }
-               }
-               list.trimToSize();
-               return new CachedPackList(s, Collections.unmodifiableList(list), buf);
-       }
-
        /**
         * Add a single existing pack to the list of available pack files.
         *
@@ -893,26 +807,6 @@ public class ObjectDirectory extends FileObjectDatabase {
                }
        }
 
-       private static final class CachedPackList {
-               final FileSnapshot snapshot;
-
-               final Collection<LocalCachedPack> packs;
-
-               final byte[] raw;
-
-               CachedPackList(FileSnapshot sn, List<LocalCachedPack> list, byte[] buf) {
-                       snapshot = sn;
-                       packs = list;
-                       raw = buf;
-               }
-
-               @SuppressWarnings("unchecked")
-               Collection<CachedPack> getCachedPacks() {
-                       Collection p = packs;
-                       return p;
-               }
-       }
-
        @Override
        public ObjectDatabase newCachedDatabase() {
                return newCachedFileObjectDatabase();
index d9906dec9e9ad8746861e02a9ec801aedee3d037..3f746a4c6865d7a0a35e71fc2ea61888ab9e1f8c 100644 (file)
@@ -196,11 +196,6 @@ final class WindowCursor extends ObjectReader implements ObjectReuseAsIs {
                        out.writeObject(otp);
        }
 
-       @SuppressWarnings("unchecked")
-       public Collection<CachedPack> getCachedPacks() throws IOException {
-               return (Collection<CachedPack>) db.getCachedPacks();
-       }
-
        /**
         * Copy bytes from the window to a caller supplied buffer.
         *
index 7199c5b7e83ff08420b04e9ed07667351b7e5cde..3b57dad47a7c460fcf392dab67137b2cf2880eff 100644 (file)
 package org.eclipse.jgit.storage.pack;
 
 import java.io.IOException;
-import java.util.Set;
-
-import org.eclipse.jgit.lib.ObjectId;
 
 /** Describes a pack file {@link ObjectReuseAsIs} can append onto a stream. */
 public abstract class CachedPack {
-       /**
-        * Objects that start this pack.
-        * <p>
-        * All objects reachable from the tips are contained within this pack. If
-        * {@link PackWriter} is going to include everything reachable from all of
-        * these objects, this cached pack is eligible to be appended directly onto
-        * the output pack stream.
-        *
-        * @return the tip objects that describe this pack.
-        */
-       public abstract Set<ObjectId> getTips();
-
        /**
         * Get the number of objects in this pack.
         *
index 8816ca495b0a84454f5a1ae48ab91ed72c279723..3b55259c9e35b91132891dd1f02324420dc7611d 100644 (file)
@@ -89,7 +89,7 @@ public interface ObjectReuseAsIs {
         * the writer can select the most suitable representation to reuse into the
         * output stream.
         * <p>
-        * If the implementation returns CachedPack from {@link #getCachedPacks()},
+        * If the implementation returns CachedPack from {@link #getCachedPacksAndUpdate(BitmapBuilder)}
         * it must consider the representation of any object that is stored in any
         * of the offered CachedPacks. PackWriter relies on this behavior to prune
         * duplicate objects out of the pack stream when it selects a CachedPack and
@@ -200,20 +200,6 @@ public interface ObjectReuseAsIs {
                        boolean validate) throws IOException,
                        StoredObjectRepresentationNotAvailableException;
 
-       /**
-        * Obtain the available cached packs.
-        * <p>
-        * A cached pack has known starting points and may be sent entirely as-is,
-        * with almost no effort on the sender's part.
-        *
-        * @return the available cached packs.
-        * @throws IOException
-        *             the cached packs cannot be listed from the repository.
-        *             Callers may choose to ignore this and continue as-if there
-        *             were no cached packs.
-        */
-       public Collection<CachedPack> getCachedPacks() throws IOException;
-
        /**
         * Append an entire pack's contents onto the output stream.
         * <p>
index 56c542cb6220fded839695ec26d9b1876144e22e..b4342d06cea16dde5cb1a381f2ae6c23493236ff 100644 (file)
@@ -57,10 +57,8 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
@@ -101,7 +99,6 @@ import org.eclipse.jgit.revwalk.DepthWalk;
 import org.eclipse.jgit.revwalk.ObjectWalk;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevFlag;
-import org.eclipse.jgit.revwalk.RevFlagSet;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevSort;
 import org.eclipse.jgit.revwalk.RevTag;
@@ -1387,7 +1384,8 @@ public class PackWriter {
                }
        }
 
-       private void runTasks(ExecutorService pool, ThreadSafeProgressMonitor pm,
+       private static void runTasks(ExecutorService pool,
+                       ThreadSafeProgressMonitor pm,
                        List<DeltaTask> tasks, List<Throwable> errors) throws IOException {
                List<Future<?>> futures = new ArrayList<Future<?>>(tasks.size());
                for (DeltaTask task : tasks)
@@ -1631,46 +1629,14 @@ public class PackWriter {
                all.addAll(want);
                all.addAll(have);
 
-               final Map<ObjectId, CachedPack> tipToPack = new HashMap<ObjectId, CachedPack>();
-               final RevFlag inCachedPack = walker.newFlag("inCachedPack"); //$NON-NLS-1$
                final RevFlag include = walker.newFlag("include"); //$NON-NLS-1$
                final RevFlag added = walker.newFlag("added"); //$NON-NLS-1$
 
-               final RevFlagSet keepOnRestart = new RevFlagSet();
-               keepOnRestart.add(inCachedPack);
-
                walker.carry(include);
 
                int haveEst = have.size();
                if (have.isEmpty()) {
                        walker.sort(RevSort.COMMIT_TIME_DESC);
-                       if (useCachedPacks && reuseSupport != null) {
-                               Set<ObjectId> need = new HashSet<ObjectId>(want);
-                               List<CachedPack> shortCircuit = new LinkedList<CachedPack>();
-
-                               for (CachedPack pack : reuseSupport.getCachedPacks()) {
-                                       if (need.containsAll(pack.getTips())) {
-                                               need.removeAll(pack.getTips());
-                                               shortCircuit.add(pack);
-                                       }
-
-                                       for (ObjectId id : pack.getTips()) {
-                                               tipToPack.put(id, pack);
-                                               all.add(id);
-                                       }
-                               }
-
-                               if (need.isEmpty() && !shortCircuit.isEmpty()) {
-                                       cachedPacks.addAll(shortCircuit);
-                                       for (CachedPack pack : shortCircuit)
-                                               countingMonitor.update((int) pack.getObjectCount());
-                                       endPhase(countingMonitor);
-                                       stats.timeCounting = System.currentTimeMillis() - countingStart;
-                                       return;
-                               }
-
-                               haveEst += tipToPack.size();
-                       }
                } else {
                        walker.sort(RevSort.TOPO);
                        if (thin)
@@ -1688,10 +1654,6 @@ public class PackWriter {
                                        RevObject o = q.next();
                                        if (o == null)
                                                break;
-
-                                       if (tipToPack.containsKey(o))
-                                               o.add(inCachedPack);
-
                                        if (have.contains(o))
                                                haveObjs.add(o);
                                        if (want.contains(o)) {
@@ -1747,20 +1709,6 @@ public class PackWriter {
                while ((c = walker.next()) != null) {
                        if (exclude(c))
                                continue;
-                       if (c.has(inCachedPack)) {
-                               CachedPack pack = tipToPack.get(c);
-                               if (includesAllTips(pack, include, walker)) {
-                                       useCachedPack(walker, keepOnRestart, //
-                                                       wantObjs, haveObjs, pack);
-                                       commits = new BlockList<RevCommit>();
-
-                                       endPhase(countingMonitor);
-                                       beginPhase(PackingPhase.COUNTING, countingMonitor,
-                                                       ProgressMonitor.UNKNOWN);
-                                       continue;
-                               }
-                       }
-
                        if (c.has(RevFlag.UNINTERESTING)) {
                                if (baseTrees.size() <= maxBases)
                                        baseTrees.add(c.getTree());
@@ -1893,34 +1841,6 @@ public class PackWriter {
                        list.remove(list.size() - 1);
        }
 
-       private void useCachedPack(ObjectWalk walker, RevFlagSet keepOnRestart,
-                       List<RevObject> wantObj, List<RevObject> baseObj, CachedPack pack)
-                       throws MissingObjectException, IncorrectObjectTypeException,
-                       IOException {
-               cachedPacks.add(pack);
-               for (ObjectId id : pack.getTips())
-                       baseObj.add(walker.lookupOrNull(id));
-
-               setThin(true);
-               walker.resetRetain(keepOnRestart);
-               walker.sort(RevSort.TOPO);
-               walker.sort(RevSort.BOUNDARY, true);
-
-               for (RevObject id : wantObj)
-                       walker.markStart(id);
-               for (RevObject id : baseObj)
-                       walker.markUninteresting(id);
-       }
-
-       private static boolean includesAllTips(CachedPack pack, RevFlag include,
-                       ObjectWalk walker) {
-               for (ObjectId id : pack.getTips()) {
-                       if (!walker.lookupOrNull(id).has(include))
-                               return false;
-               }
-               return true;
-       }
-
        /**
         * Include one object to the output file.
         * <p>