From 566384fa45a9dde5dc463ee90b1593f917eb227c Mon Sep 17 00:00:00 2001 From: Masaya Suzuki Date: Sat, 15 Jun 2019 12:07:35 -0700 Subject: [PATCH 1/4] dfs: Move the deeply nested code to its own method This is a pure code move. Change-Id: I6304d1cb2b22cfce969b7b5eaaec911ea28579c0 Signed-off-by: Masaya Suzuki --- .../internal/storage/dfs/DfsPackFile.java | 173 +++++++++--------- 1 file changed, 89 insertions(+), 84 deletions(-) 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 be1387ed0..57d08e425 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 @@ -195,44 +195,7 @@ public final class DfsPackFile extends BlockBasedFile { try { DfsStreamKey idxKey = desc.getStreamKey(INDEX); DfsBlockCache.Ref idxref = cache.getOrLoadRef(idxKey, - () -> { - try { - ctx.stats.readIdx++; - long start = System.nanoTime(); - try (ReadableChannel rc = ctx.db.openFile(desc, - INDEX)) { - InputStream in = Channels - .newInputStream(rc); - int wantSize = 8192; - int bs = rc.blockSize(); - if (0 < bs && bs < wantSize) { - bs = (wantSize / bs) * bs; - } else if (bs <= 0) { - bs = wantSize; - } - PackIndex idx = PackIndex.read( - new BufferedInputStream(in, bs)); - int sz = (int) Math.min( - idx.getObjectCount() * REC_SIZE, - Integer.MAX_VALUE); - ctx.stats.readIdxBytes += rc.position(); - index = idx; - return new DfsBlockCache.Ref<>(idxKey, 0, - sz, idx); - } 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); - } - }); + () -> loadPackIndex(ctx, idxKey)); PackIndex idx = idxref.get(); if (index == null && idx != null) { index = idx; @@ -267,44 +230,9 @@ public final class DfsPackFile extends BlockBasedFile { PackIndex idx = idx(ctx); PackReverseIndex revidx = getReverseIdx(ctx); DfsStreamKey bitmapKey = desc.getStreamKey(BITMAP_INDEX); - DfsBlockCache.Ref idxref = cache - .getOrLoadRef(bitmapKey, () -> { - ctx.stats.readBitmap++; - long start = System.nanoTime(); - try (ReadableChannel rc = ctx.db.openFile(desc, - BITMAP_INDEX)) { - long size; - PackBitmapIndex bmidx; - try { - InputStream in = Channels.newInputStream(rc); - int wantSize = 8192; - int bs = rc.blockSize(); - if (0 < bs && bs < wantSize) { - bs = (wantSize / bs) * bs; - } else if (bs <= 0) { - bs = wantSize; - } - in = new BufferedInputStream(in, bs); - bmidx = PackBitmapIndex.read(in, idx, revidx); - } finally { - size = rc.position(); - ctx.stats.readIdxBytes += size; - ctx.stats.readIdxMicros += elapsedMicros(start); - } - int sz = (int) Math.min(size, Integer.MAX_VALUE); - bitmapIndex = bmidx; - return new DfsBlockCache.Ref<>(bitmapKey, 0, sz, - bmidx); - } catch (EOFException e) { - throw new IOException(MessageFormat.format( - DfsText.get().shortReadOfIndex, - desc.getFileName(BITMAP_INDEX)), e); - } catch (IOException e) { - throw new IOException(MessageFormat.format( - DfsText.get().cannotReadIndex, - desc.getFileName(BITMAP_INDEX)), e); - } - }); + DfsBlockCache.Ref idxref = cache.getOrLoadRef( + bitmapKey, + () -> loadBitmapIndex(ctx, bitmapKey, idx, revidx)); PackBitmapIndex bmidx = idxref.get(); if (bitmapIndex == null && bmidx != null) { bitmapIndex = bmidx; @@ -326,14 +254,8 @@ public final class DfsPackFile extends BlockBasedFile { PackIndex idx = idx(ctx); DfsStreamKey revKey = new DfsStreamKey.ForReverseIndex( desc.getStreamKey(INDEX)); - DfsBlockCache.Ref revref = cache - .getOrLoadRef(revKey, () -> { - PackReverseIndex revidx = new PackReverseIndex(idx); - int sz = (int) Math.min(idx.getObjectCount() * 8, - Integer.MAX_VALUE); - reverseIndex = revidx; - return new DfsBlockCache.Ref<>(revKey, 0, sz, revidx); - }); + DfsBlockCache.Ref revref = cache.getOrLoadRef( + revKey, () -> loadReverseIdx(ctx, revKey, idx)); PackReverseIndex revidx = revref.get(); if (reverseIndex == null && revidx != null) { reverseIndex = revidx; @@ -1091,4 +1013,87 @@ public final class DfsPackFile extends BlockBasedFile { list.add(offset); } } + + private DfsBlockCache.Ref loadPackIndex( + DfsReader ctx, DfsStreamKey idxKey) throws IOException { + try { + ctx.stats.readIdx++; + long start = System.nanoTime(); + try (ReadableChannel rc = ctx.db.openFile(desc, INDEX)) { + InputStream in = Channels.newInputStream(rc); + int wantSize = 8192; + int bs = rc.blockSize(); + if (0 < bs && bs < wantSize) { + bs = (wantSize / bs) * bs; + } else if (bs <= 0) { + bs = wantSize; + } + PackIndex idx = PackIndex.read(new BufferedInputStream(in, bs)); + int sz = (int) Math.min( + idx.getObjectCount() * REC_SIZE, + Integer.MAX_VALUE); + ctx.stats.readIdxBytes += rc.position(); + index = idx; + return new DfsBlockCache.Ref<>(idxKey, 0, sz, idx); + } 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 DfsBlockCache.Ref loadReverseIdx( + DfsReader ctx, DfsStreamKey revKey, PackIndex idx) { + PackReverseIndex revidx = new PackReverseIndex(idx); + int sz = (int) Math.min(idx.getObjectCount() * 8, Integer.MAX_VALUE); + reverseIndex = revidx; + return new DfsBlockCache.Ref<>(revKey, 0, sz, revidx); + } + + private DfsBlockCache.Ref loadBitmapIndex( + DfsReader ctx, + DfsStreamKey bitmapKey, + PackIndex idx, + PackReverseIndex revidx) throws IOException { + ctx.stats.readBitmap++; + long start = System.nanoTime(); + try (ReadableChannel rc = ctx.db.openFile(desc, BITMAP_INDEX)) { + long size; + PackBitmapIndex bmidx; + try { + InputStream in = Channels.newInputStream(rc); + int wantSize = 8192; + int bs = rc.blockSize(); + if (0 < bs && bs < wantSize) { + bs = (wantSize / bs) * bs; + } else if (bs <= 0) { + bs = wantSize; + } + in = new BufferedInputStream(in, bs); + bmidx = PackBitmapIndex.read(in, idx, revidx); + } finally { + size = rc.position(); + ctx.stats.readIdxBytes += size; + ctx.stats.readIdxMicros += elapsedMicros(start); + } + int sz = (int) Math.min(size, Integer.MAX_VALUE); + bitmapIndex = bmidx; + return new DfsBlockCache.Ref<>(bitmapKey, 0, sz, bmidx); + } catch (EOFException e) { + throw new IOException(MessageFormat.format( + DfsText.get().shortReadOfIndex, + desc.getFileName(BITMAP_INDEX)), e); + } catch (IOException e) { + throw new IOException(MessageFormat.format( + DfsText.get().cannotReadIndex, + desc.getFileName(BITMAP_INDEX)), e); + } + } } From e837bdd0face058ad4f4f7ebdb4802514719bb94 Mon Sep 17 00:00:00 2001 From: Masaya Suzuki Date: Sat, 15 Jun 2019 13:10:00 -0700 Subject: [PATCH 2/4] dfs: Add a position argument This makes DfsBlockCache methods more unified. Also this reduces a magic number embedded in DfsBlockCache. Change-Id: I61e6c93ca283c0395738103bd2d94091edbccd4e Signed-off-by: Masaya Suzuki --- .../jgit/internal/storage/dfs/DfsBlockCache.java | 11 +++++++---- .../jgit/internal/storage/dfs/DfsPackFile.java | 16 +++++++++++----- 2 files changed, 18 insertions(+), 9 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 c6e2fae42..fae302282 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 @@ -521,17 +521,20 @@ public final class DfsBlockCache { * * @param key * the stream key of the pack. + * @param position + * the position in the key. The default should be 0. * @param loader * the function to load the reference. * @return the object reference. * @throws IOException * the reference was not in the cache and could not be loaded. */ - Ref getOrLoadRef(DfsStreamKey key, RefLoader loader) + Ref getOrLoadRef( + DfsStreamKey key, long position, RefLoader loader) throws IOException { - int slot = slot(key, 0); + int slot = slot(key, position); HashEntry e1 = table.get(slot); - Ref ref = scanRef(e1, key, 0); + Ref ref = scanRef(e1, key, position); if (ref != null) { getStat(statHit, key).incrementAndGet(); return ref; @@ -543,7 +546,7 @@ public final class DfsBlockCache { try { HashEntry e2 = table.get(slot); if (e2 != e1) { - ref = scanRef(e2, key, 0); + ref = scanRef(e2, key, position); if (ref != null) { getStat(statHit, key).incrementAndGet(); return ref; 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 57d08e425..bca891a52 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 @@ -89,6 +89,7 @@ import org.eclipse.jgit.util.LongList; */ public final class DfsPackFile extends BlockBasedFile { private static final int REC_SIZE = Constants.OBJECT_ID_LENGTH + 8; + private static final long REF_POSITION = 0; /** * Lock for initialization of {@link #index} and {@link #corruptObjects}. @@ -194,7 +195,9 @@ public final class DfsPackFile extends BlockBasedFile { try { DfsStreamKey idxKey = desc.getStreamKey(INDEX); - DfsBlockCache.Ref idxref = cache.getOrLoadRef(idxKey, + DfsBlockCache.Ref idxref = cache.getOrLoadRef( + idxKey, + REF_POSITION, () -> loadPackIndex(ctx, idxKey)); PackIndex idx = idxref.get(); if (index == null && idx != null) { @@ -232,6 +235,7 @@ public final class DfsPackFile extends BlockBasedFile { DfsStreamKey bitmapKey = desc.getStreamKey(BITMAP_INDEX); DfsBlockCache.Ref idxref = cache.getOrLoadRef( bitmapKey, + REF_POSITION, () -> loadBitmapIndex(ctx, bitmapKey, idx, revidx)); PackBitmapIndex bmidx = idxref.get(); if (bitmapIndex == null && bmidx != null) { @@ -255,7 +259,9 @@ public final class DfsPackFile extends BlockBasedFile { DfsStreamKey revKey = new DfsStreamKey.ForReverseIndex( desc.getStreamKey(INDEX)); DfsBlockCache.Ref revref = cache.getOrLoadRef( - revKey, () -> loadReverseIdx(ctx, revKey, idx)); + revKey, + REF_POSITION, + () -> loadReverseIdx(ctx, revKey, idx)); PackReverseIndex revidx = revref.get(); if (reverseIndex == null && revidx != null) { reverseIndex = revidx; @@ -1034,7 +1040,7 @@ public final class DfsPackFile extends BlockBasedFile { Integer.MAX_VALUE); ctx.stats.readIdxBytes += rc.position(); index = idx; - return new DfsBlockCache.Ref<>(idxKey, 0, sz, idx); + return new DfsBlockCache.Ref<>(idxKey, REF_POSITION, sz, idx); } finally { ctx.stats.readIdxMicros += elapsedMicros(start); } @@ -1054,7 +1060,7 @@ public final class DfsPackFile extends BlockBasedFile { PackReverseIndex revidx = new PackReverseIndex(idx); int sz = (int) Math.min(idx.getObjectCount() * 8, Integer.MAX_VALUE); reverseIndex = revidx; - return new DfsBlockCache.Ref<>(revKey, 0, sz, revidx); + return new DfsBlockCache.Ref<>(revKey, REF_POSITION, sz, revidx); } private DfsBlockCache.Ref loadBitmapIndex( @@ -1085,7 +1091,7 @@ public final class DfsPackFile extends BlockBasedFile { } int sz = (int) Math.min(size, Integer.MAX_VALUE); bitmapIndex = bmidx; - return new DfsBlockCache.Ref<>(bitmapKey, 0, sz, bmidx); + return new DfsBlockCache.Ref<>(bitmapKey, REF_POSITION, sz, bmidx); } catch (EOFException e) { throw new IOException(MessageFormat.format( DfsText.get().shortReadOfIndex, From 46678b303c6625c039109b1a06c0df0ec82dd290 Mon Sep 17 00:00:00 2001 From: Masaya Suzuki Date: Sat, 15 Jun 2019 13:53:27 -0700 Subject: [PATCH 3/4] dfs: Read at the aligned position The position is anyway aligned in BlockBasedFile, so this is no-op. Change-Id: Iba037e0ecff339393dd2c03fc5ae4fe858031e4f Signed-off-by: Masaya Suzuki --- .../org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 fae302282..cd5ada64a 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 @@ -412,8 +412,7 @@ public final class DfsBlockCache { getStat(statMiss, key).incrementAndGet(); boolean credit = true; try { - v = file.readOneBlock(requestedPosition, ctx, - fileChannel.get()); + v = file.readOneBlock(position, ctx, fileChannel.get()); credit = false; } finally { if (credit) { From c0cfdcd2f1a14eba2240ce26b3969b9e9267027d Mon Sep 17 00:00:00 2001 From: Masaya Suzuki Date: Sun, 16 Jun 2019 09:09:16 -0700 Subject: [PATCH 4/4] dfs: Take size as long instead of int Practically we wouldn't have 2GB+ objects in the DfsBlockCache, but by making it long, we can clean up some long-to-integer conversions. Change-Id: I1217f5f273a1420d80e2307ac9ff4a52460237a2 Signed-off-by: Masaya Suzuki --- .../internal/storage/dfs/DfsBlockCache.java | 14 ++++++------- .../internal/storage/dfs/DfsPackFile.java | 20 +++++++++++-------- 2 files changed, 19 insertions(+), 15 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 cd5ada64a..16e7a0d53 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 @@ -449,7 +449,7 @@ public final class DfsBlockCache { } @SuppressWarnings("unchecked") - private void reserveSpace(int reserve, DfsStreamKey key) { + private void reserveSpace(long reserve, DfsStreamKey key) { clockLock.lock(); try { long live = LongStream.of(getCurrentSize()).sum() + reserve; @@ -486,7 +486,7 @@ public final class DfsBlockCache { } } - private void creditSpace(int credit, DfsStreamKey key) { + private void creditSpace(long credit, DfsStreamKey key) { clockLock.lock(); try { getStat(liveBytes, key).addAndGet(-credit); @@ -496,7 +496,7 @@ public final class DfsBlockCache { } @SuppressWarnings("unchecked") - private void addToClock(Ref ref, int credit) { + private void addToClock(Ref ref, long credit) { clockLock.lock(); try { if (credit != 0) { @@ -576,10 +576,10 @@ public final class DfsBlockCache { } Ref putRef(DfsStreamKey key, long size, T v) { - return put(key, 0, (int) Math.min(size, Integer.MAX_VALUE), v); + return put(key, 0, size, v); } - Ref put(DfsStreamKey key, long pos, int size, T v) { + Ref put(DfsStreamKey key, long pos, long size, T v) { int slot = slot(key, pos); HashEntry e1 = table.get(slot); Ref ref = scanRef(e1, key, pos); @@ -722,12 +722,12 @@ public final class DfsBlockCache { static final class Ref { final DfsStreamKey key; final long position; - final int size; + final long size; volatile T value; Ref next; volatile boolean hot; - Ref(DfsStreamKey key, long position, int size, T v) { + Ref(DfsStreamKey key, long position, long size, T v) { this.key = key; this.position = position; this.size = size; 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 bca891a52..6c69019e0 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 @@ -1035,12 +1035,13 @@ public final class DfsPackFile extends BlockBasedFile { bs = wantSize; } PackIndex idx = PackIndex.read(new BufferedInputStream(in, bs)); - int sz = (int) Math.min( - idx.getObjectCount() * REC_SIZE, - Integer.MAX_VALUE); ctx.stats.readIdxBytes += rc.position(); index = idx; - return new DfsBlockCache.Ref<>(idxKey, REF_POSITION, sz, idx); + return new DfsBlockCache.Ref<>( + idxKey, + REF_POSITION, + idx.getObjectCount() * REC_SIZE, + idx); } finally { ctx.stats.readIdxMicros += elapsedMicros(start); } @@ -1058,9 +1059,12 @@ public final class DfsPackFile extends BlockBasedFile { private DfsBlockCache.Ref loadReverseIdx( DfsReader ctx, DfsStreamKey revKey, PackIndex idx) { PackReverseIndex revidx = new PackReverseIndex(idx); - int sz = (int) Math.min(idx.getObjectCount() * 8, Integer.MAX_VALUE); reverseIndex = revidx; - return new DfsBlockCache.Ref<>(revKey, REF_POSITION, sz, revidx); + return new DfsBlockCache.Ref<>( + revKey, + REF_POSITION, + idx.getObjectCount() * 8, + revidx); } private DfsBlockCache.Ref loadBitmapIndex( @@ -1089,9 +1093,9 @@ public final class DfsPackFile extends BlockBasedFile { ctx.stats.readIdxBytes += size; ctx.stats.readIdxMicros += elapsedMicros(start); } - int sz = (int) Math.min(size, Integer.MAX_VALUE); bitmapIndex = bmidx; - return new DfsBlockCache.Ref<>(bitmapKey, REF_POSITION, sz, bmidx); + return new DfsBlockCache.Ref<>( + bitmapKey, REF_POSITION, size, bmidx); } catch (EOFException e) { throw new IOException(MessageFormat.format( DfsText.get().shortReadOfIndex,