DfsPackFile: Abstract the loading of pack indexes DfsPackFile assumes that the indexes are stored in file streams and their references need to be cached in DFS. This doesn't allow us to experiment other storage options, like key-value databases. In these experiments not all indexes are together in the same storage. Define an interface per index to load it, so implementors can focus on the specifics of their index. Put them together in the IndexFactory interface. The implementation of the IndexFactory chooses the right combination of storages. At the moment we do this only for primary and reverse indexes. Following changes can do the same for other indexes. Change-Id: Icf790d8ba64b34dbe984759f1c6d8ec702554149
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 cdd1061..845feab 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
@@ -107,6 +107,53 @@ public final class DfsPackFile extends BlockBasedFile { /** Lock for {@link #corruptObjects}. */ private final Object corruptObjectsLock = new Object(); + private final IndexFactory indexFactory; + + /** + * Returns the indexes for this pack. + * <p> + * We define indexes in different sub interfaces to allow implementing the + * indexes over different combinations of backends. + * <p> + * Implementations decide if/how to cache the indexes. The calling + * DfsPackFile will keep the reference to the index as long as it needs it. + */ + public interface IndexFactory { + /** + * Take care of loading the primary and reverse indexes for this pack. + */ + interface PackIndexes { + /** + * Load the primary index for the pack. + * + * @param ctx + * reader to find the raw bytes + * @return a primary index + * @throws IOException + * a problem finding/parsing the index + */ + PackIndex index(DfsReader ctx) throws IOException; + + /** + * Load the reverse index of the pack + * + * @param ctx + * reader to find the raw bytes + * @return the reverse index of the pack + * @throws IOException + * a problem finding/parsing the reverse index + */ + PackReverseIndex reverseIndex(DfsReader ctx) throws IOException; + } + + /** + * Returns a provider of the primary and reverse indexes of this pack + * + * @return an implementation of the {@link PackIndexes} interface + */ + PackIndexes getPackIndexes(); + } + /** * Construct a reader for an existing, packfile. * @@ -116,7 +163,8 @@ public final class DfsPackFile extends BlockBasedFile { * description of the pack within the DFS. */ DfsPackFile(DfsBlockCache cache, DfsPackDescription desc) { - this(cache, desc, DEFAULT_BITMAP_LOADER); + this(cache, desc, DEFAULT_BITMAP_LOADER, + new CachedStreamIndexFactory(cache, desc)); } /** @@ -128,9 +176,11 @@ public final class DfsPackFile extends BlockBasedFile { * description of the pack within the DFS * @param bitmapLoader * loader to get the bitmaps of this pack (if any) + * @param indexFactory + * an IndexFactory to get references to the indexes of this pack */ public DfsPackFile(DfsBlockCache cache, DfsPackDescription desc, - PackBitmapIndexLoader bitmapLoader) { + PackBitmapIndexLoader bitmapLoader, IndexFactory indexFactory) { super(cache, desc, PACK); int bs = desc.getBlockSize(PACK); @@ -142,6 +192,7 @@ public DfsPackFile(DfsBlockCache cache, DfsPackDescription desc, length = sz > 0 ? sz : -1; this.bitmapLoader = bitmapLoader; + this.indexFactory = indexFactory; } /** @@ -196,22 +247,7 @@ private PackIndex idx(DfsReader ctx) throws IOException { Repository.getGlobalListenerList() .dispatch(new BeforeDfsPackIndexLoadedEvent(this)); try { - DfsStreamKey idxKey = desc.getStreamKey(INDEX); - // Keep the value parsed in the loader, in case the Ref<> is - // nullified in ClockBlockCacheTable#reserveSpace - // before we read its value. - AtomicReference<PackIndex> loadedRef = new AtomicReference<>(null); - DfsBlockCache.Ref<PackIndex> cachedRef = cache.getOrLoadRef(idxKey, - REF_POSITION, () -> { - RefWithSize<PackIndex> idx = loadPackIndex(ctx); - loadedRef.set(idx.ref); - return new DfsBlockCache.Ref<>(idxKey, REF_POSITION, - idx.size, idx.ref); - }); - if (loadedRef.get() == null) { - ctx.stats.idxCacheHit++; - } - index = cachedRef.get() != null ? cachedRef.get() : loadedRef.get(); + index = indexFactory.getPackIndexes().index(ctx); if (index == null) { throw new IOException( "Couldn't get a reference to the primary index"); //$NON-NLS-1$ @@ -328,26 +364,7 @@ public PackReverseIndex getReverseIdx(DfsReader ctx) throws IOException { return reverseIndex; } - PackIndex idx = idx(ctx); - DfsStreamKey revKey = desc.getStreamKey(REVERSE_INDEX); - // Keep the value parsed in the loader, in case the Ref<> is - // nullified in ClockBlockCacheTable#reserveSpace - // before we read its value. - AtomicReference<PackReverseIndex> loadedRef = new AtomicReference<>( - null); - DfsBlockCache.Ref<PackReverseIndex> cachedRef = cache - .getOrLoadRef(revKey, REF_POSITION, () -> { - RefWithSize<PackReverseIndex> ridx = loadReverseIdx(ctx, - idx); - loadedRef.set(ridx.ref); - return new DfsBlockCache.Ref<>(revKey, REF_POSITION, - ridx.size, ridx.ref); - }); - if (loadedRef.get() == null) { - ctx.stats.ridxCacheHit++; - } - reverseIndex = cachedRef.get() != null ? cachedRef.get() - : loadedRef.get(); + reverseIndex = indexFactory.getPackIndexes().reverseIndex(ctx); if (reverseIndex == null) { throw new IOException( "Couldn't get a reference to the reverse index"); //$NON-NLS-1$ @@ -1227,38 +1244,6 @@ private void setCorrupt(long offset) { } } - private RefWithSize<PackIndex> loadPackIndex(DfsReader ctx) - throws IOException { - try { - ctx.stats.readIdx++; - long start = System.nanoTime(); - try (ReadableChannel rc = ctx.db.openFile(desc, INDEX)) { - PackIndex idx = PackIndex.read(alignTo8kBlocks(rc)); - ctx.stats.readIdxBytes += rc.position(); - return new RefWithSize<>(idx, idx.getObjectCount() * REC_SIZE); - } finally { - ctx.stats.readIdxMicros += elapsedMicros(start); - } - } catch (EOFException e) { - throw new IOException(MessageFormat.format( - DfsText.get().shortReadOfIndex, - desc.getFileName(INDEX)), e); - } catch (IOException e) { - throw new IOException(MessageFormat.format( - DfsText.get().cannotReadIndex, - desc.getFileName(INDEX)), e); - } - } - - private static RefWithSize<PackReverseIndex> loadReverseIdx(DfsReader ctx, - PackIndex idx) { - ctx.stats.readReverseIdx++; - long start = System.nanoTime(); - PackReverseIndex revidx = PackReverseIndexFactory.computeFromIndex(idx); - ctx.stats.readReverseIdxMicros += elapsedMicros(start); - return new RefWithSize<>(revidx, idx.getObjectCount() * 8); - } - private DfsBlockCache.Ref<PackObjectSizeIndex> loadObjectSizeIndex( DfsReader ctx, DfsStreamKey objectSizeIndexKey) throws IOException { ctx.stats.readObjectSizeIndex++; @@ -1457,6 +1442,134 @@ public LoadResult loadPackBitmapIndex(DfsReader ctx, DfsPackFile pack) } } + /** + * An index factory backed by Dfs streams and references cached in + * DfsBlockCache + */ + public static final class CachedStreamIndexFactory implements IndexFactory { + private final CachedStreamPackIndexes indexes; + + /** + * An index factory + * + * @param cache + * DFS block cache to use for the references + * @param desc + * This factory loads indexes for this package + */ + public CachedStreamIndexFactory(DfsBlockCache cache, + DfsPackDescription desc) { + this.indexes = new CachedStreamPackIndexes(cache, desc); + } + + @Override + public PackIndexes getPackIndexes() { + return indexes; + } + } + + /** + * Load primary and reverse index from Dfs streams and cache the references + * in DfsBlockCache. + */ + public static final class CachedStreamPackIndexes implements IndexFactory.PackIndexes { + private final DfsBlockCache cache; + + private final DfsPackDescription desc; + + /** + * An index factory + * + * @param cache + * DFS block cache to use for the references + * @param desc This factory loads indexes for this package + */ + public CachedStreamPackIndexes(DfsBlockCache cache, + DfsPackDescription desc) { + this.cache = cache; + this.desc = desc; + } + + @Override + public PackIndex index(DfsReader ctx) throws IOException { + DfsStreamKey idxKey = desc.getStreamKey(INDEX); + // Keep the value parsed in the loader, in case the Ref<> is + // nullified in ClockBlockCacheTable#reserveSpace + // before we read its value. + AtomicReference<PackIndex> loadedRef = new AtomicReference<>(null); + DfsBlockCache.Ref<PackIndex> cachedRef = cache.getOrLoadRef(idxKey, + REF_POSITION, () -> { + RefWithSize<PackIndex> idx = loadPackIndex(ctx, desc); + loadedRef.set(idx.ref); + return new DfsBlockCache.Ref<>(idxKey, REF_POSITION, + idx.size, idx.ref); + }); + if (loadedRef.get() == null) { + ctx.stats.idxCacheHit++; + } + return cachedRef.get() != null ? cachedRef.get() : loadedRef.get(); + } + + private static RefWithSize<PackIndex> loadPackIndex(DfsReader ctx, + DfsPackDescription desc) throws IOException { + try { + ctx.stats.readIdx++; + long start = System.nanoTime(); + try (ReadableChannel rc = ctx.db.openFile(desc, INDEX)) { + PackIndex idx = PackIndex.read(alignTo8kBlocks(rc)); + ctx.stats.readIdxBytes += rc.position(); + return new RefWithSize<>(idx, + idx.getObjectCount() * REC_SIZE); + } finally { + ctx.stats.readIdxMicros += elapsedMicros(start); + } + } catch (EOFException e) { + throw new IOException( + MessageFormat.format(DfsText.get().shortReadOfIndex, + desc.getFileName(INDEX)), + e); + } catch (IOException e) { + throw new IOException( + MessageFormat.format(DfsText.get().cannotReadIndex, + desc.getFileName(INDEX)), + e); + } + } + + @Override + public PackReverseIndex reverseIndex(DfsReader ctx) throws IOException { + PackIndex idx = index(ctx); + DfsStreamKey revKey = desc.getStreamKey(REVERSE_INDEX); + // Keep the value parsed in the loader, in case the Ref<> is + // nullified in ClockBlockCacheTable#reserveSpace + // before we read its value. + AtomicReference<PackReverseIndex> loadedRef = new AtomicReference<>( + null); + DfsBlockCache.Ref<PackReverseIndex> cachedRef = cache + .getOrLoadRef(revKey, REF_POSITION, () -> { + RefWithSize<PackReverseIndex> ridx = loadReverseIdx(ctx, + idx); + loadedRef.set(ridx.ref); + return new DfsBlockCache.Ref<>(revKey, REF_POSITION, + ridx.size, ridx.ref); + }); + if (loadedRef.get() == null) { + ctx.stats.ridxCacheHit++; + } + return cachedRef.get() != null ? cachedRef.get() : loadedRef.get(); + } + + private static RefWithSize<PackReverseIndex> loadReverseIdx( + DfsReader ctx, PackIndex idx) { + ctx.stats.readReverseIdx++; + long start = System.nanoTime(); + PackReverseIndex revidx = PackReverseIndexFactory + .computeFromIndex(idx); + ctx.stats.readReverseIdxMicros += elapsedMicros(start); + return new RefWithSize<>(revidx, idx.getObjectCount() * 8); + } + } + private static final class RefWithSize<V> { final V ref; final long size;