From da7671fcd5593431c9a31c9e005565217b44fe10 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 18 Jul 2017 21:58:30 -0700 Subject: [PATCH] dfs: Fix caching of index, bitmap index, reverse index When 07f98a8b71 ("Derive DfsStreamKey from DfsPackDescription") stopped caching DfsPackFile in the DfsBlockCache, the DfsPackFile began to always load the idx, bitmap, or compute reverse index, as the cache handles were no longer populated by prior requests. Rework caching to lookup the objects from the DfsBlockCache if the local DfsPackFile handle is invalid. This allows the DfsPackFile to be more of a flyweight instance across requests. Change-Id: Ic7b42ce2d90692cccea36deb30c2c76ccc81638b --- .../internal/storage/dfs/DfsBlockCache.java | 13 ++++ .../internal/storage/dfs/DfsPackFile.java | 76 +++++++++++-------- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java index 92b9866a9..e1773eb26 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java @@ -390,6 +390,10 @@ void put(DfsBlock v) { put(v.stream, v.start, v.size(), v); } + Ref putRef(DfsStreamKey key, long size, T v) { + return put(key, 0, (int) Math.min(size, Integer.MAX_VALUE), v); + } + Ref put(DfsStreamKey key, long pos, int size, T v) { int slot = slot(key, pos); HashEntry e1 = table.get(slot); @@ -444,6 +448,15 @@ private T scan(HashEntry n, DfsStreamKey key, long position) { return r != null ? r.get() : null; } + Ref getRef(DfsStreamKey key) { + Ref r = scanRef(table.get(slot(key, 0)), key, 0); + if (r != null) + statHit.incrementAndGet(); + else + statMiss.incrementAndGet(); + return r; + } + @SuppressWarnings("unchecked") private Ref scanRef(HashEntry n, DfsStreamKey key, long position) { for (; n != null; n = n.next) { 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 5620df42e..b58016fd5 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 @@ -144,8 +144,8 @@ public boolean isIndexLoaded() { void setPackIndex(PackIndex idx) { long objCnt = idx.getObjectCount(); int recSize = Constants.OBJECT_ID_LENGTH + 8; - int sz = (int) Math.min(objCnt * recSize, Integer.MAX_VALUE); - index = cache.put(desc.getStreamKey(INDEX), 0, sz, idx); + long sz = objCnt * recSize; + index = cache.putRef(desc.getStreamKey(INDEX), sz, idx); } /** @@ -184,6 +184,16 @@ private PackIndex idx(DfsReader ctx) throws IOException { return idx; } + DfsStreamKey idxKey = desc.getStreamKey(INDEX); + idxref = cache.getRef(idxKey); + if (idxref != null) { + PackIndex idx = idxref.get(); + if (idx != null) { + index = idxref; + return idx; + } + } + PackIndex idx; try { ctx.stats.readIdx++; @@ -205,18 +215,14 @@ else if (bs <= 0) } } catch (EOFException e) { invalid = true; - IOException e2 = new IOException(MessageFormat.format( + throw new IOException(MessageFormat.format( DfsText.get().shortReadOfIndex, - desc.getFileName(INDEX))); - e2.initCause(e); - throw e2; + desc.getFileName(INDEX)), e); } catch (IOException e) { invalid = true; - IOException e2 = new IOException(MessageFormat.format( + throw new IOException(MessageFormat.format( DfsText.get().cannotReadIndex, - desc.getFileName(INDEX))); - e2.initCause(e); - throw e2; + desc.getFileName(INDEX)), e); } setPackIndex(idx); @@ -229,8 +235,9 @@ final boolean isGarbage() { } PackBitmapIndex getBitmapIndex(DfsReader ctx) throws IOException { - if (invalid || isGarbage()) + if (invalid || isGarbage() || !desc.hasFileExt(BITMAP_INDEX)) return null; + DfsBlockCache.Ref idxref = bitmapIndex; if (idxref != null) { PackBitmapIndex idx = idxref.get(); @@ -238,9 +245,6 @@ PackBitmapIndex getBitmapIndex(DfsReader ctx) throws IOException { return idx; } - if (!desc.hasFileExt(BITMAP_INDEX)) - return null; - synchronized (initLock) { idxref = bitmapIndex; if (idxref != null) { @@ -249,6 +253,16 @@ PackBitmapIndex getBitmapIndex(DfsReader ctx) throws IOException { return idx; } + DfsStreamKey bitmapKey = desc.getStreamKey(BITMAP_INDEX); + idxref = cache.getRef(bitmapKey); + if (idxref != null) { + PackBitmapIndex idx = idxref.get(); + if (idx != null) { + bitmapIndex = idxref; + return idx; + } + } + long size; PackBitmapIndex idx; try { @@ -273,22 +287,16 @@ else if (bs <= 0) ctx.stats.readIdxMicros += elapsedMicros(start); } } catch (EOFException e) { - IOException e2 = new IOException(MessageFormat.format( + throw new IOException(MessageFormat.format( DfsText.get().shortReadOfIndex, - desc.getFileName(BITMAP_INDEX))); - e2.initCause(e); - throw e2; + desc.getFileName(BITMAP_INDEX)), e); } catch (IOException e) { - IOException e2 = new IOException(MessageFormat.format( + throw new IOException(MessageFormat.format( DfsText.get().cannotReadIndex, - desc.getFileName(BITMAP_INDEX))); - e2.initCause(e); - throw e2; + desc.getFileName(BITMAP_INDEX)), e); } - bitmapIndex = cache.put( - desc.getStreamKey(BITMAP_INDEX), - 0, (int) Math.min(size, Integer.MAX_VALUE), idx); + bitmapIndex = cache.putRef(bitmapKey, size, idx); return idx; } } @@ -309,13 +317,21 @@ PackReverseIndex getReverseIdx(DfsReader ctx) throws IOException { return revidx; } + DfsStreamKey revKey = + new DfsStreamKey.ForReverseIndex(desc.getStreamKey(INDEX)); + revref = cache.getRef(revKey); + if (revref != null) { + PackReverseIndex idx = revref.get(); + if (idx != null) { + reverseIndex = revref; + return idx; + } + } + PackIndex idx = idx(ctx); PackReverseIndex revidx = new PackReverseIndex(idx); - int sz = (int) Math.min( - idx.getObjectCount() * 8, Integer.MAX_VALUE); - reverseIndex = cache.put( - new DfsStreamKey.ForReverseIndex(desc.getStreamKey(INDEX)), - 0, sz, revidx); + long cnt = idx.getObjectCount(); + reverseIndex = cache.putRef(revKey, cnt * 8, revidx); return revidx; } }