diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java index b9758bd64..600b1a44d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java @@ -48,7 +48,6 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.text.MessageFormat; -import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.PackInvalidException; import org.eclipse.jgit.internal.storage.pack.PackExt; @@ -130,18 +129,18 @@ abstract class BlockBasedFile { } DfsBlock getOrLoadBlock(long pos, DfsReader ctx) throws IOException { - return cache.getOrLoad(this, pos, ctx, null); + try (ReadableChannel rc = ctx.db.openFile(desc, ext)) { + return cache.getOrLoad(this, pos, ctx, () -> rc); + } } - DfsBlock readOneBlock(long pos, DfsReader ctx, - @Nullable ReadableChannel fileChannel) throws IOException { + DfsBlock readOneBlock(long pos, DfsReader ctx, ReadableChannel rc) + throws IOException { if (invalid) throw new PackInvalidException(getFileName()); ctx.stats.readBlock++; long start = System.nanoTime(); - ReadableChannel rc = fileChannel != null ? fileChannel - : ctx.db.openFile(desc, ext); try { int size = blockSize(rc); pos = (pos / size) * size; @@ -189,9 +188,6 @@ abstract class BlockBasedFile { return new DfsBlock(key, pos, buf); } finally { - if (rc != fileChannel) { - rc.close(); - } ctx.stats.readBlockMicros += elapsedMicros(start); } } 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 a7c16de25..d9e9327d3 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 @@ -52,7 +52,6 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import java.util.stream.LongStream; -import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.pack.PackExt; @@ -375,13 +374,13 @@ public final class DfsBlockCache { * @param ctx * current thread's reader. * @param fileChannel - * optional channel to read {@code pack}. + * supplier for channel to read {@code pack}. * @return the object reference. * @throws IOException * the reference was not in the cache and could not be loaded. */ DfsBlock getOrLoad(BlockBasedFile file, long position, DfsReader ctx, - @Nullable ReadableChannel fileChannel) throws IOException { + ReadableChannelSupplier fileChannel) throws IOException { final long requestedPosition = position; position = file.alignToBlock(position); @@ -413,7 +412,8 @@ public final class DfsBlockCache { getStat(statMiss, key).incrementAndGet(); boolean credit = true; try { - v = file.readOneBlock(requestedPosition, ctx, fileChannel); + v = file.readOneBlock(requestedPosition, ctx, + fileChannel.get()); credit = false; } finally { if (credit) { @@ -749,4 +749,16 @@ public final class DfsBlockCache { interface RefLoader { Ref load() throws IOException; } + + /** + * Supplier for readable channel + */ + @FunctionalInterface + public interface ReadableChannelSupplier { + /** + * @return ReadableChannel + * @throws IOException + */ + ReadableChannel get() throws IOException; + } } 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 a7073f695..7d891b523 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 @@ -438,99 +438,77 @@ public final class DfsPackFile extends BlockBasedFile { return dstbuf; } - void copyPackAsIs(PackOutputStream out, DfsReader ctx) - throws IOException { + void copyPackAsIs(PackOutputStream out, DfsReader ctx) throws IOException { // If the length hasn't been determined yet, pin to set it. if (length == -1) { ctx.pin(this, 0); ctx.unpin(); } - if (cache.shouldCopyThroughCache(length)) { - copyPackThroughCache(out, ctx); - } else { - copyPackBypassCache(out, ctx); + try (ReadableChannel rc = ctx.db.openFile(desc, PACK)) { + int sz = ctx.getOptions().getStreamPackBufferSize(); + if (sz > 0) { + rc.setReadAheadBytes(sz); + } + if (cache.shouldCopyThroughCache(length)) { + copyPackThroughCache(out, ctx, rc); + } else { + copyPackBypassCache(out, rc); + } } } - private void copyPackThroughCache(PackOutputStream out, DfsReader ctx) - throws IOException { - @SuppressWarnings("resource") // Explicitly closed in finally block - ReadableChannel rc = null; - try { - long position = 12; - long remaining = length - (12 + 20); - while (0 < remaining) { - DfsBlock b; - if (rc != null) { - b = cache.getOrLoad(this, position, ctx, rc); - } else { - b = cache.get(key, alignToBlock(position)); - if (b == null) { - rc = ctx.db.openFile(desc, PACK); - int sz = ctx.getOptions().getStreamPackBufferSize(); - if (sz > 0) { - rc.setReadAheadBytes(sz); - } - b = cache.getOrLoad(this, position, ctx, rc); - } - } + private void copyPackThroughCache(PackOutputStream out, DfsReader ctx, + ReadableChannel rc) throws IOException { + long position = 12; + long remaining = length - (12 + 20); + while (0 < remaining) { + DfsBlock b = cache.getOrLoad(this, position, ctx, () -> rc); + int ptr = (int) (position - b.start); + int n = (int) Math.min(b.size() - ptr, remaining); + b.write(out, position, n); + position += n; + remaining -= n; + } + } + private long copyPackBypassCache(PackOutputStream out, ReadableChannel rc) + throws IOException { + ByteBuffer buf = newCopyBuffer(out, rc); + long position = 12; + long remaining = length - (12 + 20); + boolean packHeadSkipped = false; + while (0 < remaining) { + DfsBlock b = cache.get(key, alignToBlock(position)); + if (b != null) { int ptr = (int) (position - b.start); int n = (int) Math.min(b.size() - ptr, remaining); b.write(out, position, n); position += n; remaining -= n; + rc.position(position); + packHeadSkipped = true; + continue; } - } finally { - if (rc != null) { - rc.close(); - } - } - } - private long copyPackBypassCache(PackOutputStream out, DfsReader ctx) - throws IOException { - try (ReadableChannel rc = ctx.db.openFile(desc, PACK)) { - ByteBuffer buf = newCopyBuffer(out, rc); - if (ctx.getOptions().getStreamPackBufferSize() > 0) { - rc.setReadAheadBytes(ctx.getOptions().getStreamPackBufferSize()); + buf.position(0); + int n = read(rc, buf); + if (n <= 0) { + throw packfileIsTruncated(); + } else if (n > remaining) { + n = (int) remaining; } - long position = 12; - long remaining = length - (12 + 20); - boolean packHeadSkipped = false; - while (0 < remaining) { - DfsBlock b = cache.get(key, alignToBlock(position)); - if (b != null) { - int ptr = (int) (position - b.start); - int n = (int) Math.min(b.size() - ptr, remaining); - b.write(out, position, n); - position += n; - remaining -= n; - rc.position(position); - packHeadSkipped = true; - continue; - } - buf.position(0); - int n = read(rc, buf); - if (n <= 0) { - throw packfileIsTruncated(); - } else if (n > remaining) { - n = (int) remaining; - } - - if (!packHeadSkipped) { - // Need skip the 'PACK' header for the first read - out.write(buf.array(), 12, n - 12); - packHeadSkipped = true; - } else { - out.write(buf.array(), 0, n); - } - position += n; - remaining -= n; + if (!packHeadSkipped) { + // Need skip the 'PACK' header for the first read + out.write(buf.array(), 12, n - 12); + packHeadSkipped = true; + } else { + out.write(buf.array(), 0, n); } - return position; + position += n; + remaining -= n; } + return position; } private ByteBuffer newCopyBuffer(PackOutputStream out, ReadableChannel rc) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftable.java index 7502471b0..a80797730 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftable.java @@ -128,7 +128,9 @@ public class DfsReftable extends BlockBasedFile { open().setReadAheadBytes(readAhead); } - DfsBlock block = cache.getOrLoad(file, pos, ctx, ch); + DfsBlock block = cache.getOrLoad(file, pos, ctx, () -> { + return open(); + }); if (block.start == pos && block.size() >= cnt) { return block.zeroCopyByteBuffer(cnt); }