{"thread":{"id":"19006","subject":"[JGIT PATCH] Combine WindowedFile into PackFile to simplify the internals","startedAt":"2009-04-22T17:33:36Z","lastAt":"2009-04-22T22:56:04Z","messageCount":2,"participants":["Shawn O. Pearce","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"111968","messageId":"1240421616-14662-1-git-send-email-spearce@spearce.org","threadId":"19006","inReplyTo":null,"subject":"[JGIT PATCH] Combine WindowedFile into PackFile to simplify the internals","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-22T17:33:36Z","receivedAt":"2009-04-22T17:33:36Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"The only user of WindowedFile is PackFile, and it did so by making\nan anonymous inner subclass within its constructor.  This is simply\ntoo confusing to work with.  The two classes are tightly integrated\nand work together to implement pack access.  Half of WindowedFile\nis just exposing accessors for PackFile to call through.\n\nLong ago both PackFile and PackIndex used WindowedFile to read\ndata on demand, so having the WindowedFile class made some degree\nof sense.  But in 667a84b6997e9048e0771168c98d2ffbca791937 (almost 2\nyears ago) Robin changed the pack index data to be read in one shot,\nas that performed better than paging in the index on demand.\n\nSince that change, the only real user of WindowedFile has been\nPackFile.  Its time they got married and reduced the overall\ncomplexity of the library internals.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/lib/ByteArrayWindow.java  |    9 +-\n .../src/org/spearce/jgit/lib/ByteBufferWindow.java |    4 +-\n .../src/org/spearce/jgit/lib/ByteWindow.java       |   12 +-\n .../src/org/spearce/jgit/lib/PackFile.java         |  201 ++++++++---\n .../org/spearce/jgit/lib/UnpackedObjectCache.java  |   10 +-\n .../src/org/spearce/jgit/lib/WindowCache.java      |   14 +-\n .../src/org/spearce/jgit/lib/WindowCursor.java     |   14 +-\n .../src/org/spearce/jgit/lib/WindowedFile.java     |  355 --------------------\n 8 files changed, 176 insertions(+), 443 deletions(-)\n delete mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java\nindex c07a260..5dc3d28 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java\n@@ -53,17 +53,16 @@\n \t * Constructor for ByteWindow.\n \t * \n \t * @param o\n-\t *            the WindowedFile providing data access\n+\t *            the PackFile providing data access\n \t * @param p\n \t *            the file offset.\n \t * @param d\n-\t *            an id provided by the WindowedFile. See\n-\t *            {@link WindowCache#get(WindowCursor, WindowedFile, long)}.\n+\t *            an id provided by the PackFile. See\n+\t *            {@link WindowCache#get(WindowCursor, PackFile, long)}.\n \t * @param b\n \t *            byte array for storage\n \t */\n-\tByteArrayWindow(final WindowedFile o, final long p, final int d,\n-\t\t\tfinal byte[] b) {\n+\tByteArrayWindow(final PackFile o, final long p, final int d, final byte[] b) {\n \t\tsuper(o, p, d, b, b.length);\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java\nindex 5beb79b..01956fd 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java\n@@ -53,12 +53,12 @@\n \t *\n \t * @See ByteWindow\n \t *\n-\t * @param o The WindowedFile\n+\t * @param o The PackFile\n \t * @param p the file offset.\n \t * @param d Window id\n \t * @param b ByteBuffer storage\n \t */\n-\tByteBufferWindow(final WindowedFile o, final long p, final int d,\n+\tByteBufferWindow(final PackFile o, final long p, final int d,\n \t\t\tfinal ByteBuffer b) {\n \t\tsuper(o, p, d, b, b.capacity());\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java\nindex e957138..0d01fca 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java\n@@ -64,7 +64,7 @@\n \n \tByteWindow<?> lruNext;\n \n-\tfinal WindowedFile provider;\n+\tfinal PackFile provider;\n \n \tfinal int id;\n \n@@ -78,19 +78,19 @@\n \t * Constructor for ByteWindow.\n \t * \n \t * @param o\n-\t *            the WindowedFile providing data access\n+\t *            the PackFile providing data access\n \t * @param pos\n \t *            the position in the file the data comes from.\n \t * @param d\n-\t *            an id provided by the WindowedFile. See\n-\t *            {@link WindowCache#get(WindowCursor, WindowedFile, long)}.\n+\t *            an id provided by the PackFile. See\n+\t *            {@link WindowCache#get(WindowCursor, PackFile, long)}.\n \t * @param ref\n \t *            the object value required to perform data access.\n \t * @param sz\n \t *            the total number of bytes in this window.\n \t */\n \t@SuppressWarnings(\"unchecked\")\n-\tByteWindow(final WindowedFile o, final long pos, final int d, final T ref,\n+\tByteWindow(final PackFile o, final long pos, final int d, final T ref,\n \t\t\tfinal int sz) {\n \t\tsuper(ref, (ReferenceQueue<T>) WindowCache.clearedWindowQueue);\n \t\tprovider = o;\n@@ -100,7 +100,7 @@ ByteWindow(final WindowedFile o, final long pos, final int d, final T ref,\n \t\tend = start + size;\n \t}\n \n-\tfinal boolean contains(final WindowedFile neededFile, final long neededPos) {\n+\tfinal boolean contains(final PackFile neededFile, final long neededPos) {\n \t\treturn provider == neededFile && start <= neededPos && neededPos < end;\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\nindex 74ffef8..dccc067 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n@@ -42,6 +42,9 @@\n import java.io.File;\n import java.io.IOException;\n import java.io.OutputStream;\n+import java.io.RandomAccessFile;\n+import java.nio.MappedByteBuffer;\n+import java.nio.channels.FileChannel.MapMode;\n import java.util.Arrays;\n import java.util.Collections;\n import java.util.Comparator;\n@@ -53,6 +56,7 @@\n import org.spearce.jgit.errors.CorruptObjectException;\n import org.spearce.jgit.errors.PackMismatchException;\n import org.spearce.jgit.util.NB;\n+import org.spearce.jgit.util.RawParseUtils;\n \n /**\n  * A Git version 2 pack file representation. A pack file contains Git objects in\n@@ -69,7 +73,16 @@ public int compare(final PackFile a, final PackFile b) {\n \n \tprivate final File idxFile;\n \n-\tprivate final WindowedFile pack;\n+\tprivate final File packFile;\n+\n+\tfinal int hash;\n+\n+\tRandomAccessFile fd;\n+\n+\tlong length;\n+\n+\t/** Total number of windows actively in the associated cache. */\n+\tint openCount;\n \n \tprivate int packLastModified;\n \n@@ -89,18 +102,14 @@ public int compare(final PackFile a, final PackFile b) {\n \t */\n \tpublic PackFile(final File idxFile, final File packFile) {\n \t\tthis.idxFile = idxFile;\n+\t\tthis.packFile = packFile;\n \t\tthis.packLastModified = (int) (packFile.lastModified() >> 10);\n-\t\tpack = new WindowedFile(packFile) {\n-\t\t\t@Override\n-\t\t\tprotected void onOpen() throws IOException {\n-\t\t\t\ttry {\n-\t\t\t\t\tonOpenPack();\n-\t\t\t\t} catch (IOException e) {\n-\t\t\t\t\tinvalid = true;\n-\t\t\t\t\tthrow e;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t};\n+\n+\t\t// Multiply by 31 here so we can more directly combine with another\n+\t\t// value in WindowCache.hash(), without doing the multiply there.\n+\t\t//\n+\t\thash = System.identityHashCode(this) * 31;\n+\t\tlength = Long.MAX_VALUE;\n \t}\n \n \tprivate synchronized PackIndex idx() throws IOException {\n@@ -122,7 +131,7 @@ final PackedObjectLoader resolveBase(final WindowCursor curs, final long ofs)\n \n \t/** @return the File object which locates this pack on disk. */\n \tpublic File getPackFile() {\n-\t\treturn pack.getFile();\n+\t\treturn packFile;\n \t}\n \n \t/**\n@@ -164,8 +173,8 @@ public PackedObjectLoader get(final WindowCursor curs, final AnyObjectId id)\n \t * Close the resources utilized by this repository\n \t */\n \tpublic void close() {\n-\t\tUnpackedObjectCache.purge(pack);\n-\t\tpack.close();\n+\t\tUnpackedObjectCache.purge(this);\n+\t\tWindowCache.purge(this);\n \t\tsynchronized (this) {\n \t\t\tloadedIdx = null;\n \t\t\treverseIdx = null;\n@@ -219,17 +228,18 @@ ObjectId findObjectForOffset(final long offset) throws IOException {\n \t}\n \n \tfinal UnpackedObjectCache.Entry readCache(final long position) {\n-\t\treturn UnpackedObjectCache.get(pack, position);\n+\t\treturn UnpackedObjectCache.get(this, position);\n \t}\n \n \tfinal void saveCache(final long position, final byte[] data, final int type) {\n-\t\tUnpackedObjectCache.store(pack, position, data, type);\n+\t\tUnpackedObjectCache.store(this, position, data, type);\n \t}\n \n \tfinal byte[] decompress(final long position, final int totalSize,\n \t\t\tfinal WindowCursor curs) throws DataFormatException, IOException {\n \t\tfinal byte[] dstbuf = new byte[totalSize];\n-\t\tpack.readCompressed(position, dstbuf, curs);\n+\t\tif (curs.inflate(this, position, dstbuf, 0) != totalSize)\n+\t\t\tthrow new EOFException(\"Short compressed stream at \" + position);\n \t\treturn dstbuf;\n \t}\n \n@@ -245,15 +255,13 @@ final void copyRawData(final PackedObjectLoader loader,\n \t\t\tfinal CRC32 crc = new CRC32();\n \t\t\tint headerCnt = (int) (dataOffset - objectOffset);\n \t\t\twhile (headerCnt > 0) {\n-\t\t\t\tint toRead = Math.min(headerCnt, buf.length);\n-\t\t\t\tint read = pack.read(objectOffset, buf, 0, toRead, curs);\n-\t\t\t\tif (read != toRead)\n-\t\t\t\t\tthrow new EOFException();\n-\t\t\t\tcrc.update(buf, 0, read);\n+\t\t\t\tfinal int toRead = Math.min(headerCnt, buf.length);\n+\t\t\t\treadFully(objectOffset, buf, 0, toRead, curs);\n+\t\t\t\tcrc.update(buf, 0, toRead);\n \t\t\t\theaderCnt -= toRead;\n \t\t\t}\n \t\t\tfinal CheckedOutputStream crcOut = new CheckedOutputStream(out, crc);\n-\t\t\tpack.copyToStream(dataOffset, buf, cnt, crcOut, curs);\n+\t\t\tcopyToStream(dataOffset, buf, cnt, crcOut, curs);\n \t\t\tfinal long computed = crc.getValue();\n \n \t\t\tfinal ObjectId id = findObjectForOffset(objectOffset);\n@@ -263,7 +271,7 @@ final void copyRawData(final PackedObjectLoader loader,\n \t\t\t\t\t\t+ \" in \" + getPackFile() + \" has bad zlib stream\");\n \t\t} else {\n \t\t\ttry {\n-\t\t\t\tpack.verifyCompressed(dataOffset, curs);\n+\t\t\t\tcurs.inflateVerify(this, dataOffset);\n \t\t\t} catch (DataFormatException dfe) {\n \t\t\t\tfinal CorruptObjectException coe;\n \t\t\t\tcoe = new CorruptObjectException(\"Object at \" + dataOffset\n@@ -271,7 +279,7 @@ final void copyRawData(final PackedObjectLoader loader,\n \t\t\t\tcoe.initCause(dfe);\n \t\t\t\tthrow coe;\n \t\t\t}\n-\t\t\tpack.copyToStream(dataOffset, buf, cnt, out, curs);\n+\t\t\tcopyToStream(dataOffset, buf, cnt, out, curs);\n \t\t}\n \t}\n \n@@ -283,44 +291,127 @@ boolean invalid() {\n \t\treturn invalid;\n \t}\n \n+\tprivate void readFully(final long position, final byte[] dstbuf,\n+\t\t\tint dstoff, final int cnt, final WindowCursor curs)\n+\t\t\tthrows IOException {\n+\t\tif (curs.copy(this, position, dstbuf, dstoff, cnt) != cnt)\n+\t\t\tthrow new EOFException();\n+\t}\n+\n+\tprivate void copyToStream(long position, final byte[] buf, long cnt,\n+\t\t\tfinal OutputStream out, final WindowCursor curs)\n+\t\t\tthrows IOException, EOFException {\n+\t\twhile (cnt > 0) {\n+\t\t\tfinal int toRead = (int) Math.min(cnt, buf.length);\n+\t\t\treadFully(position, buf, 0, toRead, curs);\n+\t\t\tposition += toRead;\n+\t\t\tcnt -= toRead;\n+\t\t\tout.write(buf, 0, toRead);\n+\t\t}\n+\t}\n+\n+\tvoid cacheOpen() throws IOException {\n+\t\tfd = new RandomAccessFile(packFile, \"r\");\n+\t\tlength = fd.length();\n+\t\ttry {\n+\t\t\tonOpenPack();\n+\t\t} catch (IOException ioe) {\n+\t\t\tinvalid = true;\n+\t\t\tcacheClose();\n+\t\t\tthrow ioe;\n+\t\t} catch (RuntimeException re) {\n+\t\t\tinvalid = true;\n+\t\t\tcacheClose();\n+\t\t\tthrow re;\n+\t\t} catch (Error re) {\n+\t\t\tinvalid = true;\n+\t\t\tcacheClose();\n+\t\t\tthrow re;\n+\t\t}\n+\t}\n+\n+\tvoid cacheClose() {\n+\t\tif (fd != null) {\n+\t\t\ttry {\n+\t\t\t\tfd.close();\n+\t\t\t} catch (IOException err) {\n+\t\t\t\t// Ignore a close event. We had it open only for reading.\n+\t\t\t\t// There should not be errors related to network buffers\n+\t\t\t\t// not flushed, etc.\n+\t\t\t}\n+\t\t\tfd = null;\n+\t\t}\n+\t}\n+\n+\tvoid allocWindow(final WindowCursor curs, final int windowId,\n+\t\t\tfinal long pos, final int size) {\n+\t\tif (WindowCache.mmap) {\n+\t\t\tMappedByteBuffer map;\n+\t\t\ttry {\n+\t\t\t\tmap = fd.getChannel().map(MapMode.READ_ONLY, pos, size);\n+\t\t\t} catch (IOException e) {\n+\t\t\t\t// The most likely reason this failed is the JVM has run out\n+\t\t\t\t// of virtual memory. We need to discard quickly, and try to\n+\t\t\t\t// force the GC to finalize and release any existing mappings.\n+\t\t\t\ttry {\n+\t\t\t\t\tcurs.release();\n+\t\t\t\t\tSystem.gc();\n+\t\t\t\t\tSystem.runFinalization();\n+\t\t\t\t\tmap = fd.getChannel().map(MapMode.READ_ONLY, pos, size);\n+\t\t\t\t} catch (IOException ioe2) {\n+\t\t\t\t\t// Temporarily disable mmap and do buffered disk IO.\n+\t\t\t\t\t//\n+\t\t\t\t\tmap = null;\n+\t\t\t\t\tSystem.err.println(\"warning: mmap failure: \"+ioe2);\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (map != null) {\n+\t\t\t\tif (map.hasArray()) {\n+\t\t\t\t\tfinal byte[] b = map.array();\n+\t\t\t\t\tfinal ByteArrayWindow w;\n+\t\t\t\t\tw = new ByteArrayWindow(this, pos, windowId, b);\n+\t\t\t\t\tw.loaded = true;\n+\t\t\t\t\tcurs.window = w;\n+\t\t\t\t\tcurs.handle = b;\n+\t\t\t\t} else {\n+\t\t\t\t\tcurs.window = new ByteBufferWindow(this, pos, windowId, map);\n+\t\t\t\t\tcurs.handle = map;\n+\t\t\t\t}\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t}\n+\n+\t\tfinal byte[] b = new byte[size];\n+\t\tcurs.window = new ByteArrayWindow(this, pos, windowId, b);\n+\t\tcurs.handle = b;\n+\t\topenCount++; // Until the window loads, we must stay open.\n+\t}\n+\n \tprivate void onOpenPack() throws IOException {\n \t\tfinal PackIndex idx = idx();\n \t\tfinal WindowCursor curs = new WindowCursor();\n-\t\tlong position = 0;\n-\t\tfinal byte[] sig = new byte[Constants.PACK_SIGNATURE.length];\n-\t\tfinal byte[] intbuf = new byte[4];\n-\t\tfinal long vers;\n+\t\tfinal byte[] buf = new byte[20];\n \n-\t\tif (pack.read(position, sig, curs) != Constants.PACK_SIGNATURE.length)\n+\t\treadFully(0, buf, 0, 12, curs);\n+\t\tif (RawParseUtils.match(buf, 0, Constants.PACK_SIGNATURE) != 4)\n \t\t\tthrow new IOException(\"Not a PACK file.\");\n-\t\tfor (int k = 0; k < Constants.PACK_SIGNATURE.length; k++) {\n-\t\t\tif (sig[k] != Constants.PACK_SIGNATURE[k])\n-\t\t\t\tthrow new IOException(\"Not a PACK file.\");\n-\t\t}\n-\t\tposition += Constants.PACK_SIGNATURE.length;\n-\n-\t\tpack.readFully(position, intbuf, curs);\n-\t\tvers = NB.decodeUInt32(intbuf, 0);\n+\t\tfinal long vers = NB.decodeUInt32(buf, 4);\n+\t\tfinal long packCnt = NB.decodeUInt32(buf, 8);\n \t\tif (vers != 2 && vers != 3)\n \t\t\tthrow new IOException(\"Unsupported pack version \" + vers + \".\");\n-\t\tposition += 4;\n \n-\t\tpack.readFully(position, intbuf, curs);\n-\t\tfinal long packCnt = NB.decodeUInt32(intbuf, 0);\n-\t\tfinal long idxCnt = idx.getObjectCount();\n-\t\tif (idxCnt != packCnt)\n+\t\tif (packCnt != idx.getObjectCount())\n \t\t\tthrow new PackMismatchException(\"Pack object count mismatch:\"\n \t\t\t\t\t+ \" pack \" + packCnt\n-\t\t\t\t\t+ \" index \" + idxCnt\n-\t\t\t\t\t+ \": \" + pack.getName());\n+\t\t\t\t\t+ \" index \" + idx.getObjectCount()\n+\t\t\t\t\t+ \": \" + getPackFile());\n \n-\t\tfinal byte[] csumbuf = new byte[20];\n-\t\tpack.readFully(pack.length() - 20, csumbuf, curs);\n-\t\tif (!Arrays.equals(csumbuf, idx.packChecksum))\n+\t\treadFully(length - 20, buf, 0, 20, curs);\n+\t\tif (!Arrays.equals(buf, idx.packChecksum))\n \t\t\tthrow new PackMismatchException(\"Pack checksum mismatch:\"\n-\t\t\t\t\t+ \" pack \" + ObjectId.fromRaw(csumbuf).name()\n+\t\t\t\t\t+ \" pack \" + ObjectId.fromRaw(buf).name()\n \t\t\t\t\t+ \" index \" + ObjectId.fromRaw(idx.packChecksum).name()\n-\t\t\t\t\t+ \": \" + pack.getName());\n+\t\t\t\t\t+ \": \" + getPackFile());\n \t}\n \n \tprivate PackedObjectLoader reader(final WindowCursor curs,\n@@ -328,7 +419,7 @@ private PackedObjectLoader reader(final WindowCursor curs,\n \t\tlong pos = objOffset;\n \t\tint p = 0;\n \t\tfinal byte[] ib = curs.tempId;\n-\t\tpack.readFully(pos, ib, curs);\n+\t\treadFully(pos, ib, 0, 20, curs);\n \t\tint c = ib[p++] & 0xff;\n \t\tfinal int typeCode = (c >> 4) & 7;\n \t\tlong dataSize = c & 15;\n@@ -349,7 +440,7 @@ private PackedObjectLoader reader(final WindowCursor curs,\n \t\t\t\t\ttypeCode, (int) dataSize);\n \n \t\tcase Constants.OBJ_OFS_DELTA: {\n-\t\t\tpack.readFully(pos, ib, curs);\n+\t\t\treadFully(pos, ib, 0, 20, curs);\n \t\t\tp = 0;\n \t\t\tc = ib[p++] & 0xff;\n \t\t\tlong ofs = c & 127;\n@@ -363,7 +454,7 @@ private PackedObjectLoader reader(final WindowCursor curs,\n \t\t\t\t\tobjOffset, (int) dataSize, objOffset - ofs);\n \t\t}\n \t\tcase Constants.OBJ_REF_DELTA: {\n-\t\t\tpack.readFully(pos, ib, curs);\n+\t\t\treadFully(pos, ib, 0, 20, curs);\n \t\t\treturn new DeltaRefPackedObjectLoader(curs, this, pos + ib.length,\n \t\t\t\t\tobjOffset, (int) dataSize, ObjectId.fromRaw(ib));\n \t\t}\n@@ -374,7 +465,7 @@ private PackedObjectLoader reader(final WindowCursor curs,\n \n \tprivate long findEndOffset(final long startOffset)\n \t\t\tthrows IOException, CorruptObjectException {\n-\t\tfinal long maxOffset = pack.length() - Constants.OBJECT_ID_LENGTH;\n+\t\tfinal long maxOffset = length - 20;\n \t\treturn getReverseIdx().findNextOffset(startOffset, maxOffset);\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java\nindex 13861bf..3d76a90 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java\n@@ -75,7 +75,7 @@ static synchronized void reconfigure(final WindowCacheConfig cfg) {\n \t\t}\n \t}\n \n-\tstatic synchronized Entry get(final WindowedFile pack, final long position) {\n+\tstatic synchronized Entry get(final PackFile pack, final long position) {\n \t\tfinal Slot e = cache[hash(position)];\n \t\tif (e.provider == pack && e.position == position) {\n \t\t\tfinal Entry buf = e.data.get();\n@@ -87,8 +87,8 @@ static synchronized Entry get(final WindowedFile pack, final long position) {\n \t\treturn null;\n \t}\n \n-\tstatic synchronized void store(final WindowedFile pack,\n-\t\t\tfinal long position, final byte[] data, final int objectType) {\n+\tstatic synchronized void store(final PackFile pack, final long position,\n+\t\t\tfinal byte[] data, final int objectType) {\n \t\tif (data.length > maxByteCount)\n \t\t\treturn; // Too large to cache.\n \n@@ -122,7 +122,7 @@ private static void releaseMemory() {\n \t\t}\n \t}\n \n-\tstatic synchronized void purge(final WindowedFile file) {\n+\tstatic synchronized void purge(final PackFile file) {\n \t\tfor (final Slot e : cache) {\n \t\t\tif (e.provider == file) {\n \t\t\t\tclearEntry(e);\n@@ -178,7 +178,7 @@ Entry(final byte[] aData, final int aType) {\n \n \t\tSlot lruNext;\n \n-\t\tWindowedFile provider;\n+\t\tPackFile provider;\n \n \t\tlong position;\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java\nindex f755dad..03e531a 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java\n@@ -217,14 +217,14 @@ else if (maxFileCount > cfg.getPackedGitOpenFiles()) {\n \t *             the window was not found in the cache and the given provider\n \t *             was unable to load the window on demand.\n \t */\n-\tpublic static final void get(final WindowCursor curs,\n-\t\t\tfinal WindowedFile wp, final long position) throws IOException {\n+\tpublic static final void get(final WindowCursor curs, final PackFile wp,\n+\t\t\tfinal long position) throws IOException {\n \t\tgetImpl(curs, wp, position);\n \t\tcurs.window.ensureLoaded(curs.handle);\n \t}\n \n \tprivate static synchronized final void getImpl(final WindowCursor curs,\n-\t\t\tfinal WindowedFile wp, final long position) throws IOException {\n+\t\t\tfinal PackFile wp, final long position) throws IOException {\n \t\tfinal int id = (int) (position >> windowSizeShift);\n \t\tfinal int idx = hash(wp, id);\n \t\tfor (ByteWindow<?> e = cache[idx]; e != null; e = e.chainNext) {\n@@ -333,7 +333,7 @@ private static boolean isOverLimit() {\n \t *            the window provider whose windows should be removed from the\n \t *            cache.\n \t */\n-\tpublic static synchronized final void purge(final WindowedFile wp) {\n+\tpublic static synchronized final void purge(final PackFile wp) {\n \t\tfor (ByteWindow e : cache) {\n \t\t\tfor (; e != null; e = e.chainNext) {\n \t\t\t\tif (e.provider == wp)\n@@ -413,15 +413,15 @@ private static void insertLRU(final ByteWindow<?> e) {\n \t\tlruHead = e;\n \t}\n \n-\tprivate static int hash(final WindowedFile wp, final int id) {\n+\tprivate static int hash(final PackFile wp, final int id) {\n \t\t// wp.hash was already \"stirred up\" a bit by * 31 when\n \t\t// it was created. Its reasonable to just add here.\n \t\t//\n \t\treturn ((wp.hash + id) >>> 1) % cache.length;\n \t}\n \n-\tprivate static int windowSize(final WindowedFile file, final int id) {\n-\t\tfinal long len = file.length();\n+\tprivate static int windowSize(final PackFile file, final int id) {\n+\t\tfinal long len = file.length;\n \t\tfinal long pos = id << windowSizeShift;\n \t\treturn len < pos + windowSize ? (int) (len - pos) : windowSize;\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java\nindex 363ec6d..fb9d348 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCursor.java\n@@ -74,9 +74,9 @@\n \t *             this cursor does not match the provider or id and the proper\n \t *             window could not be acquired through the provider's cache.\n \t */\n-\tint copy(final WindowedFile provider, long position, final byte[] dstbuf,\n+\tint copy(final PackFile provider, long position, final byte[] dstbuf,\n \t\t\tint dstoff, final int cnt) throws IOException {\n-\t\tfinal long length = provider.length();\n+\t\tfinal long length = provider.length;\n \t\tint need = cnt;\n \t\twhile (need > 0 && position < length) {\n \t\t\tpin(provider, position);\n@@ -109,9 +109,8 @@ int copy(final WindowedFile provider, long position, final byte[] dstbuf,\n \t *             the inflater encountered an invalid chunk of data. Data\n \t *             stream corruption is likely.\n \t */\n-\tint inflate(final WindowedFile provider, long position,\n-\t\t\tfinal byte[] dstbuf, int dstoff)\n-\t\t\tthrows IOException, DataFormatException {\n+\tint inflate(final PackFile provider, long position, final byte[] dstbuf,\n+\t\t\tint dstoff) throws IOException, DataFormatException {\n \t\tif (inf == null)\n \t\t\tinf = InflaterCache.get();\n \t\telse\n@@ -125,7 +124,7 @@ int inflate(final WindowedFile provider, long position,\n \t\t}\n \t}\n \n-\tvoid inflateVerify(final WindowedFile provider, long position)\n+\tvoid inflateVerify(final PackFile provider, long position)\n \t\t\tthrows IOException, DataFormatException {\n \t\tif (inf == null)\n \t\t\tinf = InflaterCache.get();\n@@ -140,8 +139,7 @@ void inflateVerify(final WindowedFile provider, long position)\n \t\t}\n \t}\n \n-\n-\tprivate void pin(final WindowedFile provider, final long position)\n+\tprivate void pin(final PackFile provider, final long position)\n \t\t\tthrows IOException {\n \t\tfinal ByteWindow w = window;\n \t\tif (w == null || !w.contains(provider, position))\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\ndeleted file mode 100644\nindex 9293eb9..0000000\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n+++ /dev/null\n@@ -1,355 +0,0 @@\n-/*\n- * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>\n- *\n- * All rights reserved.\n- *\n- * Redistribution and use in source and binary forms, with or\n- * without modification, are permitted provided that the following\n- * conditions are met:\n- *\n- * - Redistributions of source code must retain the above copyright\n- *   notice, this list of conditions and the following disclaimer.\n- *\n- * - Redistributions in binary form must reproduce the above\n- *   copyright notice, this list of conditions and the following\n- *   disclaimer in the documentation and/or other materials provided\n- *   with the distribution.\n- *\n- * - Neither the name of the Git Development Community nor the\n- *   names of its contributors may be used to endorse or promote\n- *   products derived from this software without specific prior\n- *   written permission.\n- *\n- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND\n- * CONTRIBUTORS \"AS IS\" AND ANY EXPRESS OR IMPLIED WARRANTIES,\n- * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES\n- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE\n- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR\n- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,\n- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT\n- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;\n- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER\n- * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,\n- * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)\n- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF\n- * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n- */\n-\n-package org.spearce.jgit.lib;\n-\n-import java.io.EOFException;\n-import java.io.File;\n-import java.io.IOException;\n-import java.io.OutputStream;\n-import java.io.RandomAccessFile;\n-import java.nio.MappedByteBuffer;\n-import java.nio.channels.FileChannel.MapMode;\n-import java.util.zip.DataFormatException;\n-\n-/**\n- * Read-only cached file access.\n- * <p>\n- * Supports reading data from large read-only files by loading and caching\n- * regions (windows) of the file in memory. Windows may be loaded using either\n- * traditional byte[] or by taking advantage of the operating system's virtual\n- * memory manager and mapping the file into the JVM's address space.\n- * </p>\n- * <p>\n- * Using no MapMode is the most portable way to access a file and does not run\n- * into problems with garbage collection, but will take longer to access as the\n- * entire window must be copied from the operating system into the Java byte[]\n- * before any single byte can be accessed.\n- * </p>\n- * <p>\n- * Using a specific MapMode will avoid the complete copy by mmaping in the\n- * operating system's file buffers, however this may cause problems if a large\n- * number of windows are being heavily accessed as the Java garbage collector\n- * may not be able to unmap old windows fast enough to permit new windows to be\n- * mapped.\n- * </p>\n- */\n-public class WindowedFile {\n-\tprivate final File fPath;\n-\n-\tfinal int hash;\n-\n-\tRandomAccessFile fd;\n-\n-\tprivate long length;\n-\n-\t/** Total number of windows actively in the associated cache. */\n-\tint openCount;\n-\n-\t/**\n-\t * Open a file for reading through window caching.\n-\t * \n-\t * @param file\n-\t *            the file to open.\n-\t */\n-\tpublic WindowedFile(final File file) {\n-\t\tfPath = file;\n-\n-\t\t// Multiply by 31 here so we can more directly combine with another\n-\t\t// value in WindowCache.hash(), without doing the multiply there.\n-\t\t//\n-\t\thash = System.identityHashCode(this) * 31;\n-\t\tlength = Long.MAX_VALUE;\n-\t}\n-\n-\t/**\n-\t * Get the total number of bytes available in this file.\n-\t * \n-\t * @return the number of bytes contained within this file.\n-\t */\n-\tpublic long length() {\n-\t\treturn length;\n-\t}\n-\n-\t/** @return the absolute file object this file reads from. */\n-\tpublic File getFile() {\n-\t\treturn fPath.getAbsoluteFile();\n-\t}\n-\n-\t/**\n-\t * Get the path name of this file.\n-\t * \n-\t * @return the absolute path name of the file.\n-\t */\n-\tpublic String getName() {\n-\t\treturn getFile().getPath();\n-\t}\n-\n-\t/**\n-\t * Read the bytes into a buffer until it is full.\n-\t * <p>\n-\t * This routine always reads until either the requested number of bytes has\n-\t * been copied or EOF has been reached. Consequently callers do not need to\n-\t * invoke it in a loop to make sure their buffer has been fully populated.\n-\t * </p>\n-\t * \n-\t * @param position\n-\t *            the starting offset, as measured in bytes from the beginning\n-\t *            of this file, to copy from.\n-\t * @param dstbuf\n-\t *            buffer to copy the bytes into.\n-\t * @param curs\n-\t *            current cursor for reading data from the file.\n-\t * @return total number of bytes read. Always <code>dstbuf.length</code>\n-\t *         unless the requested range to copy is over the end of the file.\n-\t * @throws IOException\n-\t *             a necessary window was not found in the window cache and\n-\t *             trying to load it in from the operating system failed.\n-\t */\n-\tpublic int read(final long position, final byte[] dstbuf,\n-\t\t\tfinal WindowCursor curs) throws IOException {\n-\t\treturn read(position, dstbuf, 0, dstbuf.length, curs);\n-\t}\n-\n-\t/**\n-\t * Read the requested number of bytes into a buffer.\n-\t * <p>\n-\t * This routine always reads until either the requested number of bytes has\n-\t * been copied or EOF has been reached. Consequently callers do not need to\n-\t * invoke it in a loop to make sure their buffer has been fully populated.\n-\t * </p>\n-\t * \n-\t * @param position\n-\t *            the starting offset, as measured in bytes from the beginning\n-\t *            of this file, to copy from.\n-\t * @param dstbuf\n-\t *            buffer to copy the bytes into.\n-\t * @param dstoff\n-\t *            offset within <code>dstbuf</code> to start copying into.\n-\t * @param cnt\n-\t *            number of bytes to copy. Must not exceed\n-\t *            <code>dstbuf.length - dstoff</code>.\n-\t * @param curs\n-\t *            current cursor for reading data from the file.\n-\t * @return total number of bytes read. Always <code>length</code> unless\n-\t *         the requested range to copy is over the end of the file.\n-\t * @throws IOException\n-\t *             a necessary window was not found in the window cache and\n-\t *             trying to load it in from the operating system failed.\n-\t */\n-\tpublic int read(long position, final byte[] dstbuf, int dstoff,\n-\t\t\tfinal int cnt, final WindowCursor curs) throws IOException {\n-\t\treturn curs.copy(this, position, dstbuf, dstoff, cnt);\n-\t}\n-\n-\t/**\n-\t * Read the bytes into a buffer until it is full.\n-\t * <p>\n-\t * This routine always reads until either the requested number of bytes has\n-\t * been copied or EOF has been reached. Consequently callers do not need to\n-\t * invoke it in a loop to make sure their buffer has been fully populated.\n-\t * </p>\n-\t * \n-\t * @param position\n-\t *            the starting offset, as measured in bytes from the beginning\n-\t *            of this file, to copy from.\n-\t * @param dstbuf\n-\t *            buffer to copy the bytes into.\n-\t * @param curs\n-\t *            current cursor for reading data from the file.\n-\t * @throws IOException\n-\t *             a necessary window was not found in the window cache and\n-\t *             trying to load it in from the operating system failed.\n-\t * @throws EOFException\n-\t *             the file ended before <code>dstbuf.length</code> bytes\n-\t *             could be read.\n-\t */\n-\tpublic void readFully(final long position, final byte[] dstbuf,\n-\t\t\tfinal WindowCursor curs) throws IOException {\n-\t\tif (read(position, dstbuf, 0, dstbuf.length, curs) != dstbuf.length)\n-\t\t\tthrow new EOFException();\n-\t}\n-\n-\t/**\n-\t * Copy the requested number of bytes to the provided output stream.\n-\t * <p>\n-\t * This routine always reads until either the requested number of bytes has\n-\t * been copied or EOF has been reached.\n-\t * </p>\n-\t *\n-\t * @param position\n-\t *            the starting offset, as measured in bytes from the beginning\n-\t *            of this file, to copy from.\n-\t * @param buf\n-\t *            temporary buffer to copy bytes into. In case of a big amount\n-\t *            of data to copy, size of at least few kB is recommended. It\n-\t *            does not need to be of size <code>cnt</code>, however.\n-\t * @param cnt\n-\t *            number of bytes to copy. Must not exceed\n-\t *            <code>file.length - position</code>.\n-\t * @param out\n-\t *            output stream where read data is written out. No buffering is\n-\t *            guaranteed by this method.\n-\t * @param curs\n-\t *            current cursor for reading data from the file.\n-\t * @throws IOException\n-\t *             a necessary window was not found in the window cache and\n-\t *             trying to load it in from the operating system failed.\n-\t * @throws EOFException\n-\t *             the file ended before <code>cnt</code> bytes could be read.\n-\t */\n-\tpublic void copyToStream(long position, final byte[] buf, long cnt,\n-\t\t\tfinal OutputStream out, final WindowCursor curs)\n-\t\t\tthrows IOException, EOFException {\n-\t\twhile (cnt > 0) {\n-\t\t\tint toRead = (int) Math.min(cnt, buf.length);\n-\t\t\tint read = read(position, buf, 0, toRead, curs);\n-\t\t\tif (read != toRead)\n-\t\t\t\tthrow new EOFException();\n-\t\t\tposition += read;\n-\t\t\tcnt -= read;\n-\t\t\tout.write(buf, 0, read);\n-\t\t}\n-\t}\n-\n-\tvoid readCompressed(final long position, final byte[] dstbuf,\n-\t\t\tfinal WindowCursor curs) throws IOException, DataFormatException {\n-\t\tif (curs.inflate(this, position, dstbuf, 0) != dstbuf.length)\n-\t\t\tthrow new EOFException(\"Short compressed stream at \" + position);\n-\t}\n-\n-\tvoid verifyCompressed(final long position, final WindowCursor curs)\n-\t\t\tthrows IOException, DataFormatException {\n-\t\tcurs.inflateVerify(this, position);\n-\t}\n-\n-\t/**\n-\t * Overridable hook called after the file is opened.\n-\t * <p>\n-\t * This hook is invoked each time the file is opened for reading, but before\n-\t * the first window is mapped into the cache. Implementers are free to use\n-\t * any of the window access methods to obtain data, however doing so may\n-\t * pollute the window cache with otherwise unnecessary windows.\n-\t * </p>\n-\t * \n-\t * @throws IOException\n-\t *             something is wrong with the file, for example the caller does\n-\t *             not understand its version header information.\n-\t */\n-\tprotected void onOpen() throws IOException {\n-\t\t// Do nothing by default.\n-\t}\n-\n-\t/** Close this file and remove all open windows. */\n-\tpublic void close() {\n-\t\tWindowCache.purge(this);\n-\t}\n-\n-\tvoid cacheOpen() throws IOException {\n-\t\tfd = new RandomAccessFile(fPath, \"r\");\n-\t\tlength = fd.length();\n-\t\ttry {\n-\t\t\tonOpen();\n-\t\t} catch (IOException ioe) {\n-\t\t\tcacheClose();\n-\t\t\tthrow ioe;\n-\t\t} catch (RuntimeException re) {\n-\t\t\tcacheClose();\n-\t\t\tthrow re;\n-\t\t} catch (Error re) {\n-\t\t\tcacheClose();\n-\t\t\tthrow re;\n-\t\t}\n-\t}\n-\n-\tvoid cacheClose() {\n-\t\tif (fd != null) {\n-\t\t\ttry {\n-\t\t\t\tfd.close();\n-\t\t\t} catch (IOException err) {\n-\t\t\t\t// Ignore a close event. We had it open only for reading.\n-\t\t\t\t// There should not be errors related to network buffers\n-\t\t\t\t// not flushed, etc.\n-\t\t\t}\n-\t\t\tfd = null;\n-\t\t}\n-\t}\n-\n-\tvoid allocWindow(final WindowCursor curs, final int windowId,\n-\t\t\tfinal long pos, final int size) {\n-\t\tif (WindowCache.mmap) {\n-\t\t\tMappedByteBuffer map;\n-\t\t\ttry {\n-\t\t\t\tmap = fd.getChannel().map(MapMode.READ_ONLY, pos, size);\n-\t\t\t} catch (IOException e) {\n-\t\t\t\t// The most likely reason this failed is the JVM has run out\n-\t\t\t\t// of virtual memory. We need to discard quickly, and try to\n-\t\t\t\t// force the GC to finalize and release any existing mappings.\n-\t\t\t\ttry {\n-\t\t\t\t\tcurs.release();\n-\t\t\t\t\tSystem.gc();\n-\t\t\t\t\tSystem.runFinalization();\n-\t\t\t\t\tmap = fd.getChannel().map(MapMode.READ_ONLY, pos, size);\n-\t\t\t\t} catch (IOException ioe2) {\n-\t\t\t\t\t// Temporarily disable mmap and do buffered disk IO.\n-\t\t\t\t\t//\n-\t\t\t\t\tmap = null;\n-\t\t\t\t\tSystem.err.println(\"warning: mmap failure: \"+ioe2);\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\tif (map != null) {\n-\t\t\t\tif (map.hasArray()) {\n-\t\t\t\t\tfinal byte[] b = map.array();\n-\t\t\t\t\tfinal ByteArrayWindow w;\n-\t\t\t\t\tw = new ByteArrayWindow(this, pos, windowId, b);\n-\t\t\t\t\tw.loaded = true;\n-\t\t\t\t\tcurs.window = w;\n-\t\t\t\t\tcurs.handle = b;\n-\t\t\t\t} else {\n-\t\t\t\t\tcurs.window = new ByteBufferWindow(this, pos, windowId, map);\n-\t\t\t\t\tcurs.handle = map;\n-\t\t\t\t}\n-\t\t\t\treturn;\n-\t\t\t}\n-\t\t}\n-\n-\t\tfinal byte[] b = new byte[size];\n-\t\tcurs.window = new ByteArrayWindow(this, pos, windowId, b);\n-\t\tcurs.handle = b;\n-\t\topenCount++; // Until the window loads, we must stay open.\n-\t}\n-}\n-- \n1.6.3.rc1.205.g37f8\n"},{"id":"112026","messageId":"200904230056.05424.robin.rosenberg.lists@dewire.com","threadId":"19006","inReplyTo":"1240421616-14662-1-git-send-email-spearce@spearce.org","subject":"Re: [JGIT PATCH] Combine WindowedFile into PackFile to simplify the internals","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-04-22T22:56:04Z","receivedAt":"2009-04-22T22:56:04Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"onsdag 22 april 2009 19:33:36 skrev \"Shawn O. Pearce\" <spearce@spearce.org>:\n> The only user of WindowedFile is PackFile, and it did so by making\n> an anonymous inner subclass within its constructor.  This is simply\n> too confusing to work with.  The two classes are tightly integrated\n> and work together to implement pack access.  Half of WindowedFile\n> is just exposing accessors for PackFile to call through.\nThanks for clearing that out.\n\nNested classes are E-V-I-L. For UI-code you cannot sanely get away,\nwithout them, where they are typically used more like an approximation\nof closures and anonymous functions so the yield is high. In most\nplaces they just confuse.\n\n> Long ago both PackFile and PackIndex used WindowedFile to read\n> data on demand, so having the WindowedFile class made some degree\n> of sense.  But in 667a84b6997e9048e0771168c98d2ffbca791937 (almost 2\n> years ago) Robin changed the pack index data to be read in one shot,\n> as that performed better than paging in the index on demand.\n\nThe really real reason was that on Windows the index was locked when\nmapped until the Great Collector kicks in, with no way of forcing unlock. \nFaster or not, using memory mapping was simply not possible for the\nindex or any file that must be rewritten from time to time. That it was\nfaster is perhaps not so strange since a few big I/O's presumably takes\nless time than many small.\n\n-- robin\n"}]}