From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 1/2] Don't use ByteWindows when checking pack file headers/footers
Date: Mon, 27 Apr 2009 19:26:11 -0700 [thread overview]
Message-ID: <1240885572-1755-1-git-send-email-spearce@spearce.org> (raw)
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>
---
This I think can be applied as-is.
We could quibble about whether or not caching the header and footer
window is worthwhile during the pack open event. But really I
wrote this to remove a deadlock in the next patch. Its just soooo
much simpler to not make PackFile rely on WindowCache.
.../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
next reply other threads:[~2009-04-28 2:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-28 2:26 Shawn O. Pearce [this message]
2009-04-28 2:26 ` [JGIT RFC PATCH 2/2] Rewrite WindowCache to be easier to follow and maintain Shawn O. Pearce
2009-04-28 15:28 ` Shawn O. Pearce
2009-04-28 23:19 ` Robin Rosenberg
2009-04-28 23:30 ` Shawn O. Pearce
2009-04-29 17:16 ` Shawn O. Pearce
2009-04-30 7:34 ` Ferry Huberts (Pelagic)
2009-05-06 14:15 ` [PATCH] Link to the Sun JVM bug mentioned in OffsetCache Shawn O. Pearce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1240885572-1755-1-git-send-email-spearce@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=robin.rosenberg@dewire.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).