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

[JGIT PATCH 02/13] Don't use ByteWindows when checking pack file headers/footers

From
Shawn O. Pearce <spearce@spearce.org>
Date
Apr 28, 2009, 21:12 UTC
Message-ID
<1240953146-12878-3-git-send-email-spearce@spearce.org>
In-Reply-To
<1240953146-12878-2-git-send-email-spearce@spearce.org>

Its highly unlikely we need the 8 KiB surrounding the pack file header or footer immediately after opening the pack file. Reading those as full blocks and registering them in the WindowCache is probably just churning garbage through the cache. Instead, read the header with a single 12 byte read, and the footer with a single 20 byte read, and bypass the cache altogether.

This nicely removes a deadlock condition we had previously where the WindowCache was recursively calling itself during the pack file open, and got stuck on its own locks.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/PackFile.java         |    5 +--
 org.spearce.jgit/src/org/spearce/jgit/util/NB.java |   32 ++++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)
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 813ebc7..360442f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -389,10 +389,9 @@ void allocWindow(final WindowCursor curs, final int windowId,
 
 	private void onOpenPack() throws IOException {
 		final PackIndex idx = idx();
-		final WindowCursor curs = new WindowCursor();
 		final byte[] buf = new byte[20];
 
-		readFully(0, buf, 0, 12, curs);
+		NB.readFully(fd.getChannel(), 0, buf, 0, 12);
 		if (RawParseUtils.match(buf, 0, Constants.PACK_SIGNATURE) != 4)
 			throw new IOException("Not a PACK file.");
 		final long vers = NB.decodeUInt32(buf, 4);
@@ -406,7 +405,7 @@ private void onOpenPack() throws IOException {
 					+ " index " + idx.getObjectCount()
 					+ ": " + getPackFile());
 
-		readFully(length - 20, buf, 0, 20, curs);
+		NB.readFully(fd.getChannel(), length - 20, buf, 0, 20);
 		if (!Arrays.equals(buf, idx.packChecksum))
 			throw new PackMismatchException("Pack checksum mismatch:"
 					+ " pack " + ObjectId.fromRaw(buf).name()
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/NB.java b/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
index c65c6fa..4a9c9b9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
@@ -40,6 +40,8 @@
 import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
 
 /** Conversion utilities for network byte order handling. */
 public final class NB {
@@ -71,6 +73,36 @@ public static void readFully(final InputStream fd, final byte[] dst,
 	}
 
 	/**
+	 * Read the entire byte array into memory, or throw an exception.
+	 * 
+	 * @param fd
+	 *            file to read the data from.
+	 * @param pos
+	 *            position to read from the file at.
+	 * @param dst
+	 *            buffer that must be fully populated, [off, off+len).
+	 * @param off
+	 *            position within the buffer to start writing to.
+	 * @param len
+	 *            number of bytes that must be read.
+	 * @throws EOFException
+	 *             the stream ended before dst was fully populated.
+	 * @throws IOException
+	 *             there was an error reading from the stream.
+	 */
+	public static void readFully(final FileChannel fd, long pos,
+			final byte[] dst, int off, int len) throws IOException {
+		while (len > 0) {
+			final int r = fd.read(ByteBuffer.wrap(dst, off, len), pos);
+			if (r <= 0)
+				throw new EOFException("Short read of block.");
+			pos += r;
+			off += r;
+			len -= r;
+		}
+	}
+
+	/**
 	 * Skip an entire region of an input stream.
 	 * <p>
 	 * The input stream's position is moved forward by the number of requested
-- 
1.6.3.rc1.205.g37f8
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 3 of 18 in “Misc. bug fixes and cleanups”
  1. 00/13 Misc. bug fixes and cleanupsShawn O. Pearce, Apr 28, 2009
  2. 01/13 Fix performance problem recently introduced to DeltaPackedObjectLoaderShawn O. Pearce, Apr 28, 2009
  3. 02/13 Don't use ByteWindows when checking pack file headers/footersShawn O. Pearce, Apr 28, 2009
  4. 03/13 Rewrite WindowCache to be easier to follow and maintainShawn O. Pearce, Apr 28, 2009
  5. 04/13 Document the IllegalArgumentException thrown by WindowCache.reconfigureShawn O. Pearce, Apr 28, 2009
  6. 05/13 Create the new WindowCache before clearing the old oneShawn O. Pearce, Apr 28, 2009
  7. 06/13 Better handle concurrent reads during a WindowCache reconfigurationShawn O. Pearce, Apr 28, 2009
  8. 07/13 Clear dead OffsetCache cells when clearing a referenceShawn O. Pearce, Apr 28, 2009
  9. 08/13 Work around Sun JVM bug "Cleared SoftReference not added to queue"Shawn O. Pearce, Apr 28, 2009
  10. 09/13 Replace inefficient new String(String) constructor to silence FindBugsShawn O. Pearce, Apr 28, 2009
  11. 10/13 Replace inefficient new Long(long) constructor to silence FindBugsShawn O. Pearce, Apr 28, 2009
  12. 11/13 Change IndexPack to use ObjectIdSubclassMap instead of ObjectIdMapShawn O. Pearce, Apr 28, 2009
  13. 12/13 Use getCachedBytes in IndexPack to avoid an unnecessary copyShawn O. Pearce, Apr 28, 2009
  14. 13/13 Remove ObjectIdMap from the JGit libraryShawn O. Pearce, Apr 28, 2009
  15. Robin RosenbergApr 29, 2009
  16. 12/11 Add a test for LongMap, IndexPack's helper classShawn O. Pearce, Apr 29, 2009
  17. Robin RosenbergApr 29, 2009
  18. Shawn O. PearceApr 29, 2009

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.