From 89f7378da5ba061f0662d0e7945584d6230876b3 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Tue, 11 Apr 2023 12:50:52 -0700 Subject: [PATCH] DfsPackFile: Extract block aligment code Loading of pack, bitmap and commit-graph copy the same code to adjust the input stream buffering. Extract to a common function. Besides reusing the code, the name hints what it is doing. This block aligment seems unnecessary as the reading is from storage not dfs cache. The channel probably knows better. Left a TODO because I don't know the original intention. Change-Id: I18b77ae8189830fcd4d5932b6b5823b693ed6090 --- .../internal/storage/dfs/DfsPackFile.java | 49 +++++++------------ 1 file changed, 19 insertions(+), 30 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 411777c7a..c745b8e6e 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 @@ -1042,15 +1042,7 @@ private DfsBlockCache.Ref loadPackIndex( 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)); + PackIndex idx = PackIndex.read(alignTo8kBlocks(rc)); ctx.stats.readIdxBytes += rc.position(); index = idx; return new DfsBlockCache.Ref<>( @@ -1094,17 +1086,8 @@ private DfsBlockCache.Ref loadBitmapIndex(DfsReader ctx, 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(ctx), - () -> getReverseIdx(ctx), + bmidx = PackBitmapIndex.read(alignTo8kBlocks(rc), + () -> idx(ctx), () -> getReverseIdx(ctx), ctx.getOptions().shouldLoadRevIndexInParallel()); } finally { size = rc.position(); @@ -1133,16 +1116,7 @@ private DfsBlockCache.Ref loadCommitGraph(DfsReader ctx, long size; CommitGraph cg; 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); - cg = CommitGraphLoader.read(in); + cg = CommitGraphLoader.read(alignTo8kBlocks(rc)); } finally { size = rc.position(); ctx.stats.readCommitGraphBytes += size; @@ -1157,4 +1131,19 @@ private DfsBlockCache.Ref loadCommitGraph(DfsReader ctx, e); } } + + private static InputStream alignTo8kBlocks(ReadableChannel rc) { + // TODO(ifrade): This is not reading from DFS, so the channel should + // know better the right blocksize. I don't know why this was done in + // the first place, verify and remove if not needed. + 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; + } + return new BufferedInputStream(in, bs); + } }