From eb17495ca4ce95c63bacf81af16ab19ff042b65c Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 10 Apr 2013 09:30:23 -0700 Subject: [PATCH] Disable CRC32 computation when no PackIndex will be created If a server is streaming 3GiB worth of pack data to a client there is no reason to compute the CRC32 checksum on the objects. The CRC32 code computed by PackWriter is used only in the new index created by writeIndex(), which is never invoked for the native Git network protocols. Object reuse may still compute its own CRC32 to verify the data being copied from an existing pack has not been corrupted. This check is done by the ObjectReader that implements ObjectReuseAsIs and has no relationship to the CRC32 being skipped during output. Change-Id: I05626f2e0d6ce19119b57d8a27193922636d60a7 --- .../storage/pack/PackOutputStream.java | 15 ------- .../internal/storage/pack/PackWriter.java | 40 +++++++++++++++---- .../transport/BasePackPushConnection.java | 1 + .../eclipse/jgit/transport/BundleWriter.java | 1 + .../eclipse/jgit/transport/UploadPack.java | 1 + 5 files changed, 35 insertions(+), 23 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackOutputStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackOutputStream.java index 0b73e6a69..ea6781495 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackOutputStream.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackOutputStream.java @@ -47,7 +47,6 @@ import java.io.IOException; import java.io.OutputStream; import java.security.MessageDigest; -import java.util.zip.CRC32; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.Constants; @@ -64,8 +63,6 @@ public final class PackOutputStream extends OutputStream { private final PackWriter packWriter; - private final CRC32 crc = new CRC32(); - private final MessageDigest md = Constants.newMessageDigest(); private long count; @@ -102,7 +99,6 @@ public PackOutputStream(final ProgressMonitor writeMonitor, public void write(final int b) throws IOException { count++; out.write(b); - crc.update(b); md.update((byte) b); } @@ -122,7 +118,6 @@ public void write(final byte[] b, int off, int len) } out.write(b, off, n); - crc.update(b, off, n); md.update(b, off, n); off += n; @@ -235,16 +230,6 @@ public long length() { return count; } - /** @return obtain the current CRC32 register. */ - int getCRC32() { - return (int) crc.getValue(); - } - - /** Reinitialize the CRC32 register for a new region. */ - void resetCRC32() { - crc.reset(); - } - /** @return obtain the current SHA-1 digest. */ byte[] getDigest() { return md.digest(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java index 6577cec83..2e6812c1c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java @@ -75,6 +75,8 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.zip.CRC32; +import java.util.zip.CheckedOutputStream; import java.util.zip.Deflater; import java.util.zip.DeflaterOutputStream; @@ -275,12 +277,16 @@ public static Iterable getInstances() { private boolean canBuildBitmaps; + private boolean indexDisabled; + private int depth; private Collection unshallowObjects; private PackBitmapIndexBuilder writeBitmaps; + private CRC32 crc32; + /** * Create writer for specified repository. *

@@ -471,6 +477,19 @@ public void setUseBitmaps(boolean useBitmaps) { this.useBitmaps = useBitmaps; } + /** @return true if the index file cannot be created by this PackWriter. */ + public boolean isIndexDisabled() { + return indexDisabled || !cachedPacks.isEmpty(); + } + + /** + * @param noIndex + * true to disable creation of the index file. + */ + public void setIndexDisabled(boolean noIndex) { + this.indexDisabled = noIndex; + } + /** * @return true to ignore objects that are uninteresting and also not found * on local disk; false to throw a {@link MissingObjectException} @@ -855,7 +874,7 @@ public int getIndexVersion() { * the index data could not be written to the supplied stream. */ public void writeIndex(final OutputStream indexStream) throws IOException { - if (!cachedPacks.isEmpty()) + if (isIndexDisabled()) throw new IOException(JGitText.get().cachedPacksPreventsIndexCreation); long writeStart = System.currentTimeMillis(); @@ -996,8 +1015,13 @@ public void writePack(ProgressMonitor compressMonitor, if (config.isDeltaCompress()) searchForDeltas(compressMonitor); - final PackOutputStream out = new PackOutputStream(writeMonitor, - packStream, this); + crc32 = new CRC32(); + final PackOutputStream out = new PackOutputStream( + writeMonitor, + isIndexDisabled() + ? packStream + : new CheckedOutputStream(packStream, crc32), + this); long objCnt = getObjectCount(); stats.totalObjects = objCnt; @@ -1484,12 +1508,12 @@ private void writeObjectImpl(PackOutputStream out, ObjectToPack otp) if (otp.isWritten()) return; // Delta chain cycle caused this to write already. - out.resetCRC32(); + crc32.reset(); otp.setOffset(out.length()); try { reuseSupport.copyObjectAsIs(out, otp, reuseValidate); out.endObject(); - otp.setCRC(out.getCRC32()); + otp.setCRC((int) crc32.getValue()); typeStats.reusedObjects++; if (otp.isDeltaRepresentation()) { typeStats.reusedDeltas++; @@ -1523,7 +1547,7 @@ private void writeObjectImpl(PackOutputStream out, ObjectToPack otp) else writeWholeObjectDeflate(out, otp); out.endObject(); - otp.setCRC(out.getCRC32()); + otp.setCRC((int) crc32.getValue()); } private void writeBase(PackOutputStream out, ObjectToPack base) @@ -1537,7 +1561,7 @@ private void writeWholeObjectDeflate(PackOutputStream out, final Deflater deflater = deflater(); final ObjectLoader ldr = reader.open(otp, otp.getType()); - out.resetCRC32(); + crc32.reset(); otp.setOffset(out.length()); out.writeHeader(otp, ldr.getSize()); @@ -1551,7 +1575,7 @@ private void writeDeltaObjectDeflate(PackOutputStream out, final ObjectToPack otp) throws IOException { writeBase(out, otp.getDeltaBase()); - out.resetCRC32(); + crc32.reset(); otp.setOffset(out.length()); DeltaCache.Ref ref = otp.popCachedDelta(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java index 60985e7c2..22b458c92 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java @@ -291,6 +291,7 @@ private void writePack(final Map refUpdates, newObjects.add(r.getNewObjectId()); } + writer.setIndexDisabled(true); writer.setUseCachedPacks(true); writer.setUseBitmaps(true); writer.setThin(thinPack); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java index 54c8bf904..4f5cda7ab 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java @@ -200,6 +200,7 @@ public void writeBundle(ProgressMonitor monitor, OutputStream os) inc.addAll(include.values()); for (final RevCommit r : assume) exc.add(r.getId()); + packWriter.setIndexDisabled(true); packWriter.setDeltaBaseAsOffset(true); packWriter.setThin(exc.size() > 0); packWriter.setReuseValidatingObjects(false); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 1377a37cd..5347eb713 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -1116,6 +1116,7 @@ private void sendPack(final boolean sideband) throws IOException { cfg = new PackConfig(db); final PackWriter pw = new PackWriter(cfg, walk.getObjectReader()); try { + pw.setIndexDisabled(true); pw.setUseCachedPacks(true); pw.setUseBitmaps(true); pw.setReuseDeltaCommits(true);