Browse Source

Remove unnecessary inflate stride in DfsBlock

OpenJDK 7 does not benefit from using an inflate stride on the input
array. The implementation of java.util.zip.Inflater supplies the
entire input byte[] to libz, with no regards for the bounds supplied.
Slicing at 512 byte increments in DfsBlock no longer has any benefit.

In OpenJDK 6 the native portion of Inflater used GetByteArrayRegion
to obtain a copy of the input buffer for libz. In this use case
supplying a small stride made sense, it avoided allocating space
for and copying data past the end of the object's compressed stream.

In OpenJDK 7 the native code uses GetPrimitiveArrayCritical,
which tries to avoid copying by freezing Java garbage collection
and accessing the byte[] contents in place. On OpenJDK 7 derived
JVMs it is likely more efficient to supply the entire DfsBlock.

Since OpenJDK 5 and 6 are deprecated and replaced by OpenJDK 7
it is reasonable to suggest any consumers running JGit with DFS
support use an OpenJDK 7 derived JVM. However, JGit still targets
local filesystem support on Java 5, so it is still not reasonble to
apply this same simplification to the internal.storage.file package.

See: JDK-6751338 (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6751338)
Change-Id: Ib248b6d383da5c8aa887d9c355a0df6f3e2247a5
tags/v3.1.0.201309270735-rc1
Shawn Pearce 10 years ago
parent
commit
aa8d5ac26c

+ 2
- 33
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlock.java View File

@@ -55,17 +55,6 @@ import org.eclipse.jgit.internal.storage.pack.PackOutputStream;

/** A cached slice of a {@link DfsPackFile}. */
final class DfsBlock {
/**
* Size in bytes to pass to {@link Inflater} at a time.
* <p>
* Blocks can be large (for example 1 MiB), while compressed objects inside
* of them are very small (for example less than 100 bytes for a delta). JNI
* forces the data supplied to the Inflater to be copied during setInput(),
* so use a smaller stride to reduce the risk that too much unnecessary was
* moved into the native layer.
*/
private static final int INFLATE_STRIDE = 512;

final DfsPackKey pack;

final long start;
@@ -105,29 +94,9 @@ final class DfsBlock {
return n;
}

int inflate(Inflater inf, long pos, byte[] dstbuf, int dstoff)
throws DataFormatException {
void setInput(Inflater inf, long pos) {
int ptr = (int) (pos - start);
int in = Math.min(INFLATE_STRIDE, block.length - ptr);
if (dstoff < dstbuf.length)
in = Math.min(in, dstbuf.length - dstoff);
inf.setInput(block, ptr, in);

for (;;) {
int out = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
if (out == 0) {
if (inf.needsInput()) {
ptr += in;
in = Math.min(INFLATE_STRIDE, block.length - ptr);
if (in == 0)
return dstoff;
inf.setInput(block, ptr, in);
continue;
}
return dstoff;
}
dstoff += out;
}
inf.setInput(block, ptr, block.length - ptr);
}

void crc32(CRC32 out, long pos, int cnt) {

+ 15
- 10
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java View File

@@ -616,19 +616,24 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
boolean headerOnly) throws IOException, DataFormatException {
prepareInflater();
pin(pack, position);
block.setInput(inf, position);
int dstoff = 0;
for (;;) {
dstoff = block.inflate(inf, position, dstbuf, dstoff);

if (headerOnly && dstoff == dstbuf.length)
return dstoff;
if (inf.needsInput()) {
position += block.remaining(position);
pin(pack, position);
} else if (inf.finished())
return dstoff;
else
int n = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
if (n == 0) {
if (headerOnly && dstoff == dstbuf.length)
return dstoff;
if (inf.needsInput()) {
position += block.remaining(position);
pin(pack, position);
block.setInput(inf, position);
continue;
}
if (inf.finished())
return dstoff;
throw new DataFormatException();
}
dstoff += n;
}
}


Loading…
Cancel
Save