From cb5bc195406aaead0de6fa563b4b8aaacb8d20b8 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 15 May 2010 17:37:14 -0700 Subject: [PATCH] Reduce size of PackedObjectLoader by dropping long to int Rather than keep track of both the position of the object, and the position of its data, just keep track of the number of bytes used by the object's header in the pack. This shaves 4 bytes out of the size of the PackedObjectLoader instances. We also can defer the addition instruction to the materialize() operation, avoiding it entirely if the caller never actually uses the loader. This may be relevant for PackWriter invocations, where only 1 loader gets chosen for a given object, even though the object may appear on disk in more than one pack file. Error reporting is now simplified, as we can rely on the object offset rather than its data offset. This is the value displayed by pack debugging tools like `git verify-pack -v`, so its better to use that in our own errors. Because nobody needs getDataOffset() now, we can drop that from the public API. Change-Id: Ic639c0d5a722315f4f5c8ffda6e26643d90e5f42 Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/lib/T0004_PackReader.java | 4 ++-- .../jgit/lib/DeltaOfsPackedObjectLoader.java | 7 +++---- .../jgit/lib/DeltaPackedObjectLoader.java | 18 ++++++++--------- .../jgit/lib/DeltaRefPackedObjectLoader.java | 7 +++---- .../src/org/eclipse/jgit/lib/PackFile.java | 20 +++++++++---------- .../eclipse/jgit/lib/PackedObjectLoader.java | 19 +++++++----------- .../jgit/lib/WholePackedObjectLoader.java | 17 ++++++++-------- 7 files changed, 43 insertions(+), 49 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java index b78b7b855..336bba22c 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java @@ -67,7 +67,7 @@ public class T0004_PackReader extends SampleDataRepositoryTestCase { assertNotNull(or); assertEquals(Constants.OBJ_TREE, or.getType()); assertEquals(35, or.getSize()); - assertEquals(7738, or.getDataOffset()); + assertEquals(7736, or.getObjectOffset()); pr.close(); } @@ -81,6 +81,6 @@ public class T0004_PackReader extends SampleDataRepositoryTestCase { assertTrue(or instanceof PackedObjectLoader); assertEquals(Constants.OBJ_BLOB, or.getType()); assertEquals(18009, or.getSize()); - assertEquals(537, ((PackedObjectLoader) or).getDataOffset()); + assertEquals(516, ((PackedObjectLoader) or).getObjectOffset()); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaOfsPackedObjectLoader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaOfsPackedObjectLoader.java index 9e1b3da19..0b7c6c45f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaOfsPackedObjectLoader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaOfsPackedObjectLoader.java @@ -54,10 +54,9 @@ import org.eclipse.jgit.errors.CorruptObjectException; class DeltaOfsPackedObjectLoader extends DeltaPackedObjectLoader { private final long deltaBase; - DeltaOfsPackedObjectLoader(final PackFile pr, - final long dataOffset, final long objectOffset, final int deltaSz, - final long base) { - super(pr, dataOffset, objectOffset, deltaSz); + DeltaOfsPackedObjectLoader(final PackFile pr, final long objectOffset, + final int headerSz, final int deltaSz, final long base) { + super(pr, objectOffset, headerSz, deltaSz); deltaBase = base; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaPackedObjectLoader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaPackedObjectLoader.java index 867aadfb2..b42549943 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaPackedObjectLoader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaPackedObjectLoader.java @@ -57,9 +57,9 @@ abstract class DeltaPackedObjectLoader extends PackedObjectLoader { private final int deltaSize; - DeltaPackedObjectLoader(final PackFile pr, final long dataOffset, - final long objectOffset, final int deltaSz) { - super(pr, dataOffset, objectOffset); + DeltaPackedObjectLoader(final PackFile pr, final long objectOffset, + final int headerSize, final int deltaSz) { + super(pr, objectOffset, headerSize); objectType = -1; deltaSize = deltaSz; } @@ -71,7 +71,7 @@ abstract class DeltaPackedObjectLoader extends PackedObjectLoader { } if (objectType != OBJ_COMMIT) { - final UnpackedObjectCache.Entry cache = pack.readCache(dataOffset); + UnpackedObjectCache.Entry cache = pack.readCache(objectOffset); if (cache != null) { curs.release(); objectType = cache.type; @@ -84,17 +84,17 @@ abstract class DeltaPackedObjectLoader extends PackedObjectLoader { try { final PackedObjectLoader baseLoader = getBaseLoader(curs); baseLoader.materialize(curs); - cachedBytes = BinaryDelta.apply(baseLoader.getCachedBytes(), - pack.decompress(dataOffset, deltaSize, curs)); + cachedBytes = BinaryDelta.apply(baseLoader.getCachedBytes(), pack + .decompress(objectOffset + headerSize, deltaSize, curs)); curs.release(); objectType = baseLoader.getType(); objectSize = cachedBytes.length; if (objectType != OBJ_COMMIT) - pack.saveCache(dataOffset, cachedBytes, objectType); + pack.saveCache(objectOffset, cachedBytes, objectType); } catch (DataFormatException dfe) { final CorruptObjectException coe; - coe = new CorruptObjectException("Object at " + dataOffset + " in " - + pack.getPackFile() + " has bad zlib stream"); + coe = new CorruptObjectException("Object at " + objectOffset + + " in " + pack.getPackFile() + " has bad zlib stream"); coe.initCause(dfe); throw coe; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaRefPackedObjectLoader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaRefPackedObjectLoader.java index 3616c4107..9f7589c29 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaRefPackedObjectLoader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/DeltaRefPackedObjectLoader.java @@ -54,10 +54,9 @@ import org.eclipse.jgit.errors.MissingObjectException; class DeltaRefPackedObjectLoader extends DeltaPackedObjectLoader { private final ObjectId deltaBase; - DeltaRefPackedObjectLoader(final PackFile pr, - final long dataOffset, final long objectOffset, final int deltaSz, - final ObjectId base) { - super(pr, dataOffset, objectOffset, deltaSz); + DeltaRefPackedObjectLoader(final PackFile pr, final long objectOffset, + final int headerSz, final int deltaSz, final ObjectId base) { + super(pr, objectOffset, headerSz, deltaSz); deltaBase = base; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackFile.java index b1d388667..28edf30cd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackFile.java @@ -269,13 +269,13 @@ public class PackFile implements Iterable { final OutputStream out, final byte buf[], final WindowCursor curs) throws IOException { final long objectOffset = loader.objectOffset; - final long dataOffset = loader.dataOffset; + final long dataOffset = objectOffset + loader.headerSize; final int cnt = (int) (findEndOffset(objectOffset) - dataOffset); final PackIndex idx = idx(); if (idx.hasCRC32Support()) { final CRC32 crc = new CRC32(); - int headerCnt = (int) (dataOffset - objectOffset); + int headerCnt = loader.headerSize; while (headerCnt > 0) { final int toRead = Math.min(headerCnt, buf.length); readFully(objectOffset, buf, 0, toRead, curs); @@ -289,14 +289,14 @@ public class PackFile implements Iterable { final ObjectId id = findObjectForOffset(objectOffset); final long expected = idx.findCRC32(id); if (computed != expected) - throw new CorruptObjectException("Object at " + dataOffset + throw new CorruptObjectException("Object at " + objectOffset + " in " + getPackFile() + " has bad zlib stream"); } else { try { curs.inflateVerify(this, dataOffset); } catch (DataFormatException dfe) { final CorruptObjectException coe; - coe = new CorruptObjectException("Object at " + dataOffset + coe = new CorruptObjectException("Object at " + objectOffset + " in " + getPackFile() + " has bad zlib stream"); coe.initCause(dfe); throw coe; @@ -473,8 +473,8 @@ public class PackFile implements Iterable { case Constants.OBJ_TREE: case Constants.OBJ_BLOB: case Constants.OBJ_TAG: - return new WholePackedObjectLoader(this, objOffset + p, objOffset, - typeCode, (int) dataSize); + return new WholePackedObjectLoader(this, objOffset, p, typeCode, + (int) dataSize); case Constants.OBJ_OFS_DELTA: { c = ib[p++] & 0xff; @@ -485,13 +485,13 @@ public class PackFile implements Iterable { ofs <<= 7; ofs += (c & 127); } - return new DeltaOfsPackedObjectLoader(this, objOffset + p, - objOffset, (int) dataSize, objOffset - ofs); + return new DeltaOfsPackedObjectLoader(this, objOffset, p, + (int) dataSize, objOffset - ofs); } case Constants.OBJ_REF_DELTA: { readFully(objOffset + p, ib, 0, 20, curs); - return new DeltaRefPackedObjectLoader(this, objOffset + p + 20, - objOffset, (int) dataSize, ObjectId.fromRaw(ib)); + return new DeltaRefPackedObjectLoader(this, objOffset, p + 20, + (int) dataSize, ObjectId.fromRaw(ib)); } default: throw new IOException("Unknown object type " + typeCode + "."); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackedObjectLoader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackedObjectLoader.java index 4125579b2..026b008f1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackedObjectLoader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackedObjectLoader.java @@ -55,21 +55,23 @@ import java.io.OutputStream; abstract class PackedObjectLoader extends ObjectLoader { protected final PackFile pack; - protected final long dataOffset; - + /** Position of the first byte of the object's header. */ protected final long objectOffset; + /** Bytes used to express the object header, including delta reference. */ + protected final int headerSize; + protected int objectType; protected int objectSize; protected byte[] cachedBytes; - PackedObjectLoader(final PackFile pr, final long dataOffset, - final long objectOffset) { + PackedObjectLoader(final PackFile pr, final long objectOffset, + final int headerSize) { pack = pr; - this.dataOffset = dataOffset; this.objectOffset = objectOffset; + this.headerSize = headerSize; } /** @@ -113,13 +115,6 @@ abstract class PackedObjectLoader extends ObjectLoader { return objectOffset; } - /** - * @return offset of object data within pack file - */ - public final long getDataOffset() { - return dataOffset; - } - /** * Peg the pack file open to support data copying. *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/WholePackedObjectLoader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WholePackedObjectLoader.java index 31439d489..5005d37f1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/WholePackedObjectLoader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WholePackedObjectLoader.java @@ -54,9 +54,9 @@ import org.eclipse.jgit.errors.CorruptObjectException; class WholePackedObjectLoader extends PackedObjectLoader { private static final int OBJ_COMMIT = Constants.OBJ_COMMIT; - WholePackedObjectLoader(final PackFile pr, final long dataOffset, - final long objectOffset, final int type, final int size) { - super(pr, dataOffset, objectOffset); + WholePackedObjectLoader(final PackFile pr, final long objectOffset, + final int headerSize, final int type, final int size) { + super(pr, objectOffset, headerSize); objectType = type; objectSize = size; } @@ -68,7 +68,7 @@ class WholePackedObjectLoader extends PackedObjectLoader { } if (objectType != OBJ_COMMIT) { - final UnpackedObjectCache.Entry cache = pack.readCache(dataOffset); + UnpackedObjectCache.Entry cache = pack.readCache(objectOffset); if (cache != null) { curs.release(); cachedBytes = cache.data; @@ -77,14 +77,15 @@ class WholePackedObjectLoader extends PackedObjectLoader { } try { - cachedBytes = pack.decompress(dataOffset, objectSize, curs); + cachedBytes = pack.decompress(objectOffset + headerSize, + objectSize, curs); curs.release(); if (objectType != OBJ_COMMIT) - pack.saveCache(dataOffset, cachedBytes, objectType); + pack.saveCache(objectOffset, cachedBytes, objectType); } catch (DataFormatException dfe) { final CorruptObjectException coe; - coe = new CorruptObjectException("Object at " + dataOffset + " in " - + pack.getPackFile() + " has bad zlib stream"); + coe = new CorruptObjectException("Object at " + objectOffset + + " in " + pack.getPackFile() + " has bad zlib stream"); coe.initCause(dfe); throw coe; }