git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[JGIT PATCH 14/23] Don't unpack delta chains while writing a pack from a pack v1 index

From
Shawn O. Pearce <spearce@spearce.org>
Date
Dec 25, 2008, 02:11 UTC
Message-ID
<1230171079-17156-15-git-send-email-spearce@spearce.org>
In-Reply-To
<1230171079-17156-14-git-send-email-spearce@spearce.org>

We only need to inflate the delta to verify the content is sane; going all the way through the delta chain by getCachedBytes is a massive expensive that we just cannot afford to pay. This simple change improves clone time for the Linux kernel from 14m50s to 3m20s, due to the improved throughput in our write objects phase.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ByteArrayWindow.java  |   14 ++++++++++++++
 .../src/org/spearce/jgit/lib/ByteBufferWindow.java |   17 +++++++++++++++++
 .../src/org/spearce/jgit/lib/ByteWindow.java       |   10 ++++++++++
 .../src/org/spearce/jgit/lib/PackFile.java         |   18 +++++++++++-------
 .../src/org/spearce/jgit/lib/WindowCursor.java     |   16 ++++++++++++++++
 .../src/org/spearce/jgit/lib/WindowedFile.java     |    5 +++++
 6 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
index b32d4f8..7b7c7a4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
@@ -89,6 +89,20 @@ int inflate(final byte[] array, final int pos, final byte[] b, int o,
 		return o;
 	}
 
+	void inflateVerify(final byte[] array, final int pos, final Inflater inf)
+			throws DataFormatException {
+		ensureLoaded(array);
+		while (!inf.finished()) {
+			if (inf.needsInput()) {
+				inf.setInput(array, pos, array.length - pos);
+				break;
+			}
+			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
+		}
+		while (!inf.finished() && !inf.needsInput())
+			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
+	}
+
 	private synchronized void ensureLoaded(final byte[] array) {
 		if (!loaded) {
 			try {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
index 03bafb1..a6757e8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
@@ -90,4 +90,21 @@ int inflate(final ByteBuffer buffer, final int pos, final byte[] b, int o,
 			o += inf.inflate(b, o, b.length - o);
 		return o;
 	}
+
+	void inflateVerify(final ByteBuffer buffer, final int pos,
+			final Inflater inf) throws DataFormatException {
+		final byte[] tmp = new byte[512];
+		final ByteBuffer s = buffer.slice();
+		s.position(pos);
+		while (s.remaining() > 0 && !inf.finished()) {
+			if (inf.needsInput()) {
+				final int n = Math.min(s.remaining(), tmp.length);
+				s.get(tmp, 0, n);
+				inf.setInput(tmp, 0, n);
+			}
+			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
+		}
+		while (!inf.finished() && !inf.needsInput())
+			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
+	}
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
index 8d37de7..732664b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
@@ -207,4 +207,14 @@ final int inflate(T ref, long pos, byte[] dstbuf, int dstoff, Inflater inf)
 	 */
 	abstract int inflate(T ref, int pos, byte[] dstbuf, int dstoff, Inflater inf)
 			throws DataFormatException;
+
+	protected static final byte[] verifyGarbageBuffer = new byte[2048];
+
+	final void inflateVerify(T ref, long pos, Inflater inf)
+			throws DataFormatException {
+		inflateVerify(ref, (int) (pos - start), inf);
+	}
+
+	abstract void inflateVerify(T ref, int pos, Inflater inf)
+			throws DataFormatException;
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
index 3cdca8f..0de4c55 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -218,15 +218,19 @@ final void copyRawData(final PackedObjectLoader loader,
 			final ObjectId id = findObjectForOffset(objectOffset);
 			final long expected = idx.findCRC32(id);
 			if (computed != expected)
-				throw new CorruptObjectException(id,
-						"Possible data corruption - CRC32 of raw pack data (object offset "
-								+ objectOffset
-								+ ") mismatch CRC32 from pack index");
+				throw new CorruptObjectException("Object at " + dataOffset
+						+ " in " + getPackFile() + " has bad zlib stream");
 		} else {
+			try {
+				pack.verifyCompressed(dataOffset, curs);
+			} catch (DataFormatException dfe) {
+				final CorruptObjectException coe;
+				coe = new CorruptObjectException("Object at " + dataOffset
+						+ " in " + getPackFile() + " has bad zlib stream");
+				coe.initCause(dfe);
+				throw coe;
+			}
 			pack.copyToStream(dataOffset, buf, cnt, out, curs);
-
-			// read to verify against Adler32 zlib checksum
-			loader.getCachedBytes();
 		}
 	}
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java
index 7aad081..5c8bec5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java
@@ -125,6 +125,22 @@ int inflate(final WindowedFile provider, long position,
 		}
 	}
 
+	void inflateVerify(final WindowedFile provider, long position)
+			throws IOException, DataFormatException {
+		if (inf == null)
+			inf = InflaterCache.get();
+		else
+			inf.reset();
+		for (;;) {
+			pin(provider, position);
+			window.inflateVerify(handle, position, inf);
+			if (inf.finished())
+				return;
+			position = window.end;
+		}
+	}
+
+	
 	private void pin(final WindowedFile provider, final long position)
 			throws IOException {
 		final ByteWindow w = window;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
index 7626693..5eb8465 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
@@ -248,6 +248,11 @@ void readCompressed(final long position, final byte[] dstbuf,
 			throw new EOFException("Short compressed stream at " + position);
 	}
 
+	void verifyCompressed(final long position, final WindowCursor curs)
+			throws IOException, DataFormatException {
+		curs.inflateVerify(this, position);
+	}
+
 	/**
 	 * Overridable hook called after the file is opened.
 	 * <p>

-- 
1.6.1.rc4.301.g5497a
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 15 of 26 in “Misc. performance tweaks”
  1. 00/18 Misc. performance tweaksShawn O. Pearce, Dec 25, 2008
  2. 01/23 Improve hit performance on the UnpackedObjectCacheShawn O. Pearce, Dec 25, 2008
  3. 02/23 Add MutableObjectId.clear() to set the id to zeroIdShawn O. Pearce, Dec 25, 2008
  4. 03/23 Allow TreeWalk callers to pass a MutableObjectId to get the current idShawn O. Pearce, Dec 25, 2008
  5. 04/23 Switch ObjectWalk to use the new MutableObjectId form in TreeWalkShawn O. Pearce, Dec 25, 2008
  6. 05/23 Change walker based fetch to use TreeWalk's MutableObjectId accessorShawn O. Pearce, Dec 25, 2008
  7. 06/23 Reduce garbage allocation when using TreeWalkShawn O. Pearce, Dec 25, 2008
  8. 07/23 Switch ObjectWalk to use a naked CanonicalTreeParser because its fasterShawn O. Pearce, Dec 25, 2008
  9. 08/23 Remove the unused PackFile.get(ObjectId) formShawn O. Pearce, Dec 25, 2008
  10. 09/23 Remove getId from ObjectLoader API as its unnecessary overheadShawn O. Pearce, Dec 25, 2008
  11. 10/23 Make mmap mode more reliable by forcing GC at the correct spotShawn O. Pearce, Dec 25, 2008
  12. 11/23 Rewrite WindowCache to use a hash tableShawn O. Pearce, Dec 25, 2008
  13. 12/23 Change ByteArrayWindow to read content outside of WindowCache's lockShawn O. Pearce, Dec 25, 2008
  14. 13/23 Dispose of RevCommit buffers when they aren't used in PackWriterShawn O. Pearce, Dec 25, 2008
  15. 14/23 Don't unpack delta chains while writing a pack from a pack v1 indexShawn O. Pearce, Dec 25, 2008
  16. 15/23 Don't unpack delta chains while converting delta to whole objectShawn O. Pearce, Dec 25, 2008
  17. 16/23 Defer parsing of the ObjectId while walking a PackIndex IteratorShawn O. Pearce, Dec 25, 2008
  18. 17/23 Only do one getCachedBytes per whole object writtenShawn O. Pearce, Dec 25, 2008
  19. 18/23 Correctly use a long for the offsets within a generated packShawn O. Pearce, Dec 25, 2008
  20. 19/23 Allow more direct access to determine isWrittenShawn O. Pearce, Dec 25, 2008
  21. 20/23 Move "wantWrite" field of ObjectToPack into the flags fieldShawn O. Pearce, Dec 25, 2008
  22. 21/23 Use an ArrayList for the reuseLoader collection in PackWriterShawn O. Pearce, Dec 25, 2008
  23. 22/23 Don't cut off existing delta chains if we are reusing deltasShawn O. Pearce, Dec 25, 2008
  24. 23/23 Correctly honor the thin parameter to PackWriter.writePackShawn O. Pearce, Dec 25, 2008
  25. Robin RosenbergDec 27, 2008
  26. 11/23 Rewrite WindowCache to use a hash tableShawn O. Pearce, Dec 27, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.