From 1a7b8a11dff95e451e2ba59ee60f224b9c13f9a8 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 24 Jul 2017 09:38:36 -0700 Subject: [PATCH] dfs: expose DfsReftable from DfsObjDatabase Reftable storage in DFS is related to pack storage. Reftables are stored in the same namespace, but with PackExt.REFTABLE. Include the set of DfsReftable instances in the PackList and export some helpers to access the tables. Change-Id: I6a4f5f953ed6b0ff80a7780f4c6cbcc5eda0da3e --- .../internal/storage/dfs/DfsObjDatabase.java | 132 +++++++++++++----- .../storage/dfs/DfsPackDescription.java | 58 +++++++- .../internal/storage/dfs/DfsPackFile.java | 6 - 3 files changed, 149 insertions(+), 47 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java index 76189c161..943982201 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java @@ -48,6 +48,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -61,7 +62,9 @@ import org.eclipse.jgit.lib.ObjectReader; /** Manages objects stored in {@link DfsPackFile} on a storage system. */ public abstract class DfsObjDatabase extends ObjectDatabase { - private static final PackList NO_PACKS = new PackList(new DfsPackFile[0]) { + private static final PackList NO_PACKS = new PackList( + new DfsPackFile[0], + new DfsReftable[0]) { @Override boolean dirty() { return true; @@ -191,6 +194,18 @@ public abstract class DfsObjDatabase extends ObjectDatabase { return getPackList().packs; } + /** + * Scan and list all available reftable files in the repository. + * + * @return list of available reftables. The returned array is shared with + * the implementation and must not be modified by the caller. + * @throws IOException + * the pack list cannot be initialized. + */ + public DfsReftable[] getReftables() throws IOException { + return getPackList().reftables; + } + /** * Scan and list all available pack files in the repository. * @@ -219,6 +234,16 @@ public abstract class DfsObjDatabase extends ObjectDatabase { return getCurrentPackList().packs; } + /** + * List currently known reftable files in the repository, without scanning. + * + * @return list of available reftables. The returned array is shared with + * the implementation and must not be modified by the caller. + */ + public DfsReftable[] getCurrentReftables() { + return getCurrentPackList().reftables; + } + /** * List currently known pack files in the repository, without scanning. * @@ -428,7 +453,7 @@ public abstract class DfsObjDatabase extends ObjectDatabase { DfsPackFile[] packs = new DfsPackFile[1 + o.packs.length]; packs[0] = newPack; System.arraycopy(o.packs, 0, packs, 1, o.packs.length); - n = new PackListImpl(packs); + n = new PackListImpl(packs, o.reftables); } while (!packList.compareAndSet(o, n)); } @@ -454,59 +479,93 @@ public abstract class DfsObjDatabase extends ObjectDatabase { private PackList scanPacksImpl(PackList old) throws IOException { DfsBlockCache cache = DfsBlockCache.getInstance(); - Map forReuse = reuseMap(old); + Map packs = packMap(old); + Map reftables = reftableMap(old); + List scanned = listPacks(); Collections.sort(scanned); - List list = new ArrayList<>(scanned.size()); + List newPacks = new ArrayList<>(scanned.size()); + List newReftables = new ArrayList<>(scanned.size()); boolean foundNew = false; for (DfsPackDescription dsc : scanned) { - DfsPackFile oldPack = forReuse.remove(dsc); + DfsPackFile oldPack = packs.remove(dsc); if (oldPack != null) { - list.add(oldPack); + newPacks.add(oldPack); } else if (dsc.hasFileExt(PackExt.PACK)) { - list.add(new DfsPackFile(cache, dsc)); + newPacks.add(new DfsPackFile(cache, dsc)); + foundNew = true; + } + + DfsReftable oldReftable = reftables.remove(dsc); + if (oldReftable != null) { + newReftables.add(oldReftable); + } else if (dsc.hasFileExt(PackExt.REFTABLE)) { + newReftables.add(new DfsReftable(cache, dsc)); foundNew = true; } } - for (DfsPackFile p : forReuse.values()) - p.close(); - if (list.isEmpty()) - return new PackListImpl(NO_PACKS.packs); + if (newPacks.isEmpty()) + return new PackListImpl(NO_PACKS.packs, NO_PACKS.reftables); if (!foundNew) { old.clearDirty(); return old; } - return new PackListImpl(list.toArray(new DfsPackFile[list.size()])); + Collections.sort(newReftables, reftableComparator()); + return new PackListImpl( + newPacks.toArray(new DfsPackFile[0]), + newReftables.toArray(new DfsReftable[0])); } - private static Map reuseMap(PackList old) { + private static Map packMap(PackList old) { Map forReuse = new HashMap<>(); for (DfsPackFile p : old.packs) { - if (p.invalid()) { - // The pack instance is corrupted, and cannot be safely used - // again. Do not include it in our reuse map. - // - p.close(); - continue; + if (!p.invalid()) { + forReuse.put(p.desc, p); } + } + return forReuse; + } - DfsPackFile prior = forReuse.put(p.getPackDescription(), p); - if (prior != null) { - // This should never occur. It should be impossible for us - // to have two pack files with the same name, as all of them - // came out of the same directory. If it does, we promised to - // close any PackFiles we did not reuse, so close the second, - // readers are likely to be actively using the first. - // - forReuse.put(prior.getPackDescription(), prior); - p.close(); + private static Map reftableMap(PackList old) { + Map forReuse = new HashMap<>(); + for (DfsReftable p : old.reftables) { + if (!p.invalid()) { + forReuse.put(p.desc, p); } } return forReuse; } + /** @return comparator to sort {@link DfsReftable} by priority. */ + protected Comparator reftableComparator() { + return (fa, fb) -> { + DfsPackDescription a = fa.getPackDescription(); + DfsPackDescription b = fb.getPackDescription(); + + // GC, COMPACT reftables first by higher category. + int c = category(b) - category(a); + if (c != 0) { + return c; + } + + // Lower maxUpdateIndex first. + c = Long.signum(a.getMaxUpdateIndex() - b.getMaxUpdateIndex()); + if (c != 0) { + return c; + } + + // Older reftable first. + return Long.signum(a.getLastModified() - b.getLastModified()); + }; + } + + static int category(DfsPackDescription d) { + PackSource s = d.getPackSource(); + return s != null ? s.category : 0; + } + /** Clears the cached list of packs, forcing them to be scanned again. */ protected void clearCache() { packList.set(NO_PACKS); @@ -514,12 +573,7 @@ public abstract class DfsObjDatabase extends ObjectDatabase { @Override public void close() { - // PackList packs = packList.get(); packList.set(NO_PACKS); - - // TODO Close packs if they aren't cached. - // for (DfsPackFile p : packs.packs) - // p.close(); } /** Snapshot of packs scanned in a single pass. */ @@ -527,10 +581,14 @@ public abstract class DfsObjDatabase extends ObjectDatabase { /** All known packs, sorted. */ public final DfsPackFile[] packs; + /** All known reftables, sorted. */ + public final DfsReftable[] reftables; + private long lastModified = -1; - PackList(DfsPackFile[] packs) { + PackList(DfsPackFile[] packs, DfsReftable[] reftables) { this.packs = packs; + this.reftables = reftables; } /** @return last modified time of all packs, in milliseconds. */ @@ -561,8 +619,8 @@ public abstract class DfsObjDatabase extends ObjectDatabase { private static final class PackListImpl extends PackList { private volatile boolean dirty; - PackListImpl(DfsPackFile[] packs) { - super(packs); + PackListImpl(DfsPackFile[] packs, DfsReftable[] reftables) { + super(packs, reftables); } @Override diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java index 58a006e45..e865e6b54 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java @@ -44,11 +44,13 @@ package org.eclipse.jgit.internal.storage.dfs; import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK; +import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE; import java.util.Arrays; import org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource; import org.eclipse.jgit.internal.storage.pack.PackExt; +import org.eclipse.jgit.internal.storage.reftable.ReftableWriter; import org.eclipse.jgit.storage.pack.PackStatistics; /** @@ -68,7 +70,11 @@ public class DfsPackDescription implements Comparable { private int[] blockSizeMap; private long objectCount; private long deltaCount; - private PackStatistics stats; + private long minUpdateIndex; + private long maxUpdateIndex; + + private PackStatistics packStats; + private ReftableWriter.Stats refStats; private int extensions; private int indexVersion; private long estimatedPackSize; @@ -170,6 +176,36 @@ public class DfsPackDescription implements Comparable { return this; } + /** @return minUpdateIndex for the reftable, if present. */ + public long getMinUpdateIndex() { + return minUpdateIndex; + } + + /** + * @param min + * minUpdateIndex for the reftable, or 0. + * @return {@code this} + */ + public DfsPackDescription setMinUpdateIndex(long min) { + minUpdateIndex = min; + return this; + } + + /** @return maxUpdateIndex for the reftable, if present. */ + public long getMaxUpdateIndex() { + return maxUpdateIndex; + } + + /** + * @param max + * maxUpdateIndex for the reftable, or 0. + * @return {@code this} + */ + public DfsPackDescription setMaxUpdateIndex(long max) { + maxUpdateIndex = max; + return this; + } + /** * @param ext * the file extension. @@ -281,24 +317,38 @@ public class DfsPackDescription implements Comparable { * is being committed to the repository. */ public PackStatistics getPackStats() { - return stats; + return packStats; } DfsPackDescription setPackStats(PackStatistics stats) { - this.stats = stats; + this.packStats = stats; setFileSize(PACK, stats.getTotalBytes()); setObjectCount(stats.getTotalObjects()); setDeltaCount(stats.getTotalDeltas()); return this; } + /** @return stats from the sibling reftable, if created. */ + public ReftableWriter.Stats getReftableStats() { + return refStats; + } + + void setReftableStats(ReftableWriter.Stats stats) { + this.refStats = stats; + setMinUpdateIndex(stats.minUpdateIndex()); + setMaxUpdateIndex(stats.maxUpdateIndex()); + setFileSize(REFTABLE, stats.totalBytes()); + setBlockSize(REFTABLE, stats.refBlockSize()); + } + /** * Discard the pack statistics, if it was populated. * * @return {@code this} */ public DfsPackDescription clearPackStats() { - stats = null; + packStats = null; + refStats = null; return this; } 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 81b9980e2..dfb41e204 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 @@ -385,12 +385,6 @@ public final class DfsPackFile extends BlockBasedFile { idx(ctx).resolve(matches, id, matchLimit); } - /** Release all memory used by this DfsPackFile instance. */ - public void close() { - index = null; - reverseIndex = null; - } - /** * Obtain the total number of objects available in this pack. This method * relies on pack index, giving number of effectively available objects.