From: Shawn Pearce Date: Thu, 14 Mar 2013 23:27:53 +0000 (-0700) Subject: Remove cached_packs support in favor of bitmaps X-Git-Tag: v3.0.0.201305080800-m7~95^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=3760e4319b02ce79ff1eeae021fd88faebf739d5;p=jgit.git Remove cached_packs support in favor of bitmaps 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 --- diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsCachedPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsCachedPack.java index e0ecc8018a..1f9cfe83ce 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsCachedPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsCachedPack.java @@ -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 getTips() { - return getPackDescription().getTips(); - } - @Override public long getObjectCount() throws IOException { return getPackDescription().getObjectCount(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java index 8f175a8227..7f8a78abe5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java @@ -275,7 +275,7 @@ public class DfsGarbageCollector { try { pw.preparePack(pm, allHeads, Collections. emptySet()); if (0 < pw.getObjectCount()) - writePack(GC, pw, pm).setTips(allHeads); + writePack(GC, pw, pm); } finally { pw.release(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackDescription.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackDescription.java index c4a531e7be..cd9766b564 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackDescription.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackDescription.java @@ -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 { private long deltaCount; - private Set tips; - private PackWriter.Statistics stats; private int extensions; @@ -223,21 +219,6 @@ public class DfsPackDescription implements Comparable { return this; } - /** @return the tips that created this pack, if known. */ - public Set getTips() { - return tips; - } - - /** - * @param tips - * the tips of the pack, null if it has no known tips. - * @return {@code this} - */ - public DfsPackDescription setTips(Set 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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java index 401c483d4d..9409ec117a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java @@ -593,21 +593,6 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs { } } - public Collection getCachedPacks() throws IOException { - DfsPackFile[] packList = db.getPacks(); - List cached = new ArrayList(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 { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java index 939b9cf3c1..fc55be9b11 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java @@ -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 getCachedPacks() throws IOException { - return wrapped.getCachedPacks(); - } - @Override AlternateHandle[] myAlternates() { if (alts == null) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java index ed0577f804..da0465cd2a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java @@ -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 getCachedPacks() - throws IOException; - abstract AlternateHandle[] myAlternates(); abstract boolean tryAgain1(); @@ -301,11 +297,6 @@ abstract class FileObjectDatabase extends ObjectDatabase { this.db = db; } - @SuppressWarnings("unchecked") - Collection getCachedPacks() throws IOException { - return (Collection) db.getCachedPacks(); - } - void close() { db.close(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LocalCachedPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LocalCachedPack.java index c0e47eea12..9534b96160 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LocalCachedPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LocalCachedPack.java @@ -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 tips; - private final String[] packNames; private PackFile[] packs; - LocalCachedPack(ObjectDirectory odb, Set tips, - List packNames) { + LocalCachedPack(ObjectDirectory odb, List 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 packs) { odb = null; - tips = null; packNames = null; this.packs = packs.toArray(new PackFile[packs.size()]); } - @Override - public Set getTips() { - return tips; - } - @Override public long getObjectCount() throws IOException { long cnt = 0; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java index 99ceeb74bf..2f1aadaf40 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java @@ -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; - private final AtomicReference cachedPacks; - private final FS fs; private final AtomicReference 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(NO_PACKS); - cachedPacks = new AtomicReference(); 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 getCachedPacks() throws IOException { - CachedPackList list = cachedPacks.get(); - if (list == null || list.snapshot.isModified(cachedPacksFile)) - list = scanCachedPacks(list); - - Collection result = list.getCachedPacks(); - boolean resultIsCopy = false; - - for (AlternateHandle h : myAlternates()) { - Collection altPacks = h.getCachedPacks(); - if (altPacks.isEmpty()) - continue; - - if (result.isEmpty()) { - result = altPacks; - continue; - } - - if (!resultIsCopy) { - result = new ArrayList(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 list = new ArrayList(4); - Set tips = new HashSet(); - 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 names = new ArrayList(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(); - } - } - 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 packs; - - final byte[] raw; - - CachedPackList(FileSnapshot sn, List list, byte[] buf) { - snapshot = sn; - packs = list; - raw = buf; - } - - @SuppressWarnings("unchecked") - Collection getCachedPacks() { - Collection p = packs; - return p; - } - } - @Override public ObjectDatabase newCachedDatabase() { return newCachedFileObjectDatabase(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCursor.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCursor.java index d9906dec9e..3f746a4c68 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCursor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCursor.java @@ -196,11 +196,6 @@ final class WindowCursor extends ObjectReader implements ObjectReuseAsIs { out.writeObject(otp); } - @SuppressWarnings("unchecked") - public Collection getCachedPacks() throws IOException { - return (Collection) db.getCachedPacks(); - } - /** * Copy bytes from the window to a caller supplied buffer. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/CachedPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/CachedPack.java index 7199c5b7e8..3b57dad47a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/CachedPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/CachedPack.java @@ -44,24 +44,9 @@ 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. - *

- * 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 getTips(); - /** * Get the number of objects in this pack. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/ObjectReuseAsIs.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/ObjectReuseAsIs.java index 8816ca495b..3b55259c9e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/ObjectReuseAsIs.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/ObjectReuseAsIs.java @@ -89,7 +89,7 @@ public interface ObjectReuseAsIs { * the writer can select the most suitable representation to reuse into the * output stream. *

- * 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. - *

- * 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 getCachedPacks() throws IOException; - /** * Append an entire pack's contents onto the output stream. *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java index 56c542cb62..b4342d06ce 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java @@ -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 tasks, List errors) throws IOException { List> futures = new ArrayList>(tasks.size()); for (DeltaTask task : tasks) @@ -1631,46 +1629,14 @@ public class PackWriter { all.addAll(want); all.addAll(have); - final Map tipToPack = new HashMap(); - 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 need = new HashSet(want); - List shortCircuit = new LinkedList(); - - 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(); - - 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 wantObj, List 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. *