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

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 0b73e6a694ee503f0f9c8b0c642c55a97766c9d4..ea6781495de2646b4a4712b091ccdb6699cedb3f 100644 (file)
--- 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 @@ package org.eclipse.jgit.internal.storage.pack;
 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 final class PackOutputStream extends OutputStream {
        public void write(final int b) throws IOException {
                count++;
                out.write(b);
-               crc.update(b);
                md.update((byte) b);
        }
 
@@ -122,7 +118,6 @@ public final class PackOutputStream extends OutputStream {
                        }
 
                        out.write(b, off, n);
-                       crc.update(b, off, n);
                        md.update(b, off, n);
 
                        off += n;
@@ -235,16 +230,6 @@ public final class PackOutputStream extends OutputStream {
                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 6577cec8397f35642b4ae6280fcd60c3f25535f3..2e6812c1c75d63847333463a7c7bbc26ceb75c2b 100644 (file)
--- 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.ExecutorService;
 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 class PackWriter {
 
        private boolean canBuildBitmaps;
 
+       private boolean indexDisabled;
+
        private int depth;
 
        private Collection<? extends ObjectId> unshallowObjects;
 
        private PackBitmapIndexBuilder writeBitmaps;
 
+       private CRC32 crc32;
+
        /**
         * Create writer for specified repository.
         * <p>
@@ -471,6 +477,19 @@ public class PackWriter {
                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 class PackWriter {
         *             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 class PackWriter {
                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 @@ public class PackWriter {
                        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 @@ public class PackWriter {
                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 @@ public class PackWriter {
                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 @@ public class PackWriter {
                        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 60985e7c28cef40d2d90efec2cdbdaeecb18bef7..22b458c92fb4deae134d706bf52c69f01d1ba4b7 100644 (file)
--- 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 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
                                        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 54c8bf9045729f5bdc99e12a7e15fded68cc8654..4f5cda7abd517a0a92f4c690e6af22bfee352fbb 100644 (file)
--- 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 class BundleWriter {
                        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 1377a37cd8fe52c9bd04f13b1475b1926d1fbd57..5347eb713cf94ab978acbcc292a2f65aa8e098e4 100644 (file)
--- 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 @@ public class UploadPack {
                        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);