From 682240f47549f1c7952ae63331449478e8f66099 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 24 Dec 2008 18:11:12 -0800 Subject: [PATCH] Defer parsing of the ObjectId while walking a PackIndex Iterator This is a slight performance improvement for the PackReverseIndex construction. The only code that really cares about the ObjectId from a PackIndex entry is test cases. By avoiding construction we can save some CPU time during the several passes we do to make the reverse index data structure within PackWriter. Signed-off-by: Shawn O. Pearce Signed-off-by: Robin Rosenberg --- .../tst/org/spearce/jgit/lib/PackIndexTest.java | 4 +- .../tst/org/spearce/jgit/lib/PackWriterTest.java | 2 +- .../src/org/spearce/jgit/lib/PackIndex.java | 48 +++++++++++++--------- .../src/org/spearce/jgit/lib/PackIndexV1.java | 20 +++++---- .../src/org/spearce/jgit/lib/PackIndexV2.java | 27 +++++++----- 5 files changed, 61 insertions(+), 40 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java index 8ab380e2..71637186 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java @@ -134,10 +134,10 @@ public abstract class PackIndexTest extends RepositoryTestCase { */ public void testCompareEntriesOffsetsWithFindOffsets() { for (MutableEntry me : smallIdx) { - assertEquals(smallIdx.findOffset(me), me.getOffset()); + assertEquals(smallIdx.findOffset(me.toObjectId()), me.getOffset()); } for (MutableEntry me : denseIdx) { - assertEquals(denseIdx.findOffset(me), me.getOffset()); + assertEquals(denseIdx.findOffset(me.toObjectId()), me.getOffset()); } } diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java index 3c02955f..ec138a0e 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java @@ -487,7 +487,7 @@ public class PackWriterTest extends RepositoryTestCase { int i = 0; for (MutableEntry me : entries) { - assertEquals(objectsOrder[i++], me.copy()); + assertEquals(objectsOrder[i++].toObjectId(), me.toObjectId()); } } } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java index 5fb41b17..13acbd1f 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java @@ -239,15 +239,10 @@ public abstract class PackIndex implements Iterable { * in pack (both mutable). * */ - public static class MutableEntry extends MutableObjectId { - private long offset; + public static class MutableEntry { + protected final MutableObjectId idBuffer = new MutableObjectId(); - /** - * Empty constructor. Object fields should be filled in later. - */ - public MutableEntry() { - super(); - } + long offset; /** * Returns offset for this index object entry @@ -258,30 +253,43 @@ public abstract class PackIndex implements Iterable { return offset; } - void setOffset(long offset) { - this.offset = offset; + /** @return hex string describing the object id of this entry. */ + public String name() { + ensureId(); + return idBuffer.name(); } - private MutableEntry(MutableEntry src) { - super(src); - this.offset = src.offset; + /** @return a copy of the object id. */ + public ObjectId toObjectId() { + ensureId(); + return idBuffer.toObjectId(); } - /** - * Returns mutable copy of this mutable entry. - * - * @return copy of this mutable entry - */ + /** @return a complete copy of this entry, that won't modify */ public MutableEntry cloneEntry() { - return new MutableEntry(this); + final MutableEntry r = new MutableEntry(); + ensureId(); + r.idBuffer.w1 = idBuffer.w1; + r.idBuffer.w2 = idBuffer.w2; + r.idBuffer.w3 = idBuffer.w3; + r.idBuffer.w4 = idBuffer.w4; + r.idBuffer.w5 = idBuffer.w5; + r.offset = offset; + return r; + } + + protected void ensureId() { + // Override in implementations. } } protected abstract class EntriesIterator implements Iterator { - protected MutableEntry objectId = new MutableEntry(); + protected final MutableEntry entry = initEntry(); protected long returnedNumber = 0; + protected abstract MutableEntry initEntry(); + public boolean hasNext() { return returnedNumber < getObjectCount(); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java index 02465f6a..90b5f6f7 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java @@ -162,21 +162,27 @@ class PackIndexV1 extends PackIndex { private int levelTwo; + @Override + protected MutableEntry initEntry() { + return new MutableEntry() { + protected void ensureId() { + idBuffer.fromRaw(idxdata[levelOne], levelTwo + - Constants.OBJECT_ID_LENGTH); + } + }; + } + public MutableEntry next() { for (; levelOne < idxdata.length; levelOne++) { if (idxdata[levelOne] == null) continue; - if (levelTwo < idxdata[levelOne].length) { - long offset = NB.decodeUInt32(idxdata[levelOne], levelTwo); - objectId.setOffset(offset); - objectId.fromRaw(idxdata[levelOne], levelTwo + 4); + entry.offset = NB.decodeUInt32(idxdata[levelOne], levelTwo); levelTwo += Constants.OBJECT_ID_LENGTH + 4; returnedNumber++; - return objectId; - } else { - levelTwo = 0; + return entry; } + levelTwo = 0; } throw new NoSuchElementException(); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java index cc3ad653..48a52065 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java @@ -234,25 +234,32 @@ class PackIndexV2 extends PackIndex { private int levelTwo; + @Override + protected MutableEntry initEntry() { + return new MutableEntry() { + protected void ensureId() { + idBuffer.fromRaw(names[levelOne], levelTwo + - Constants.OBJECT_ID_LENGTH / 4); + } + }; + } + public MutableEntry next() { for (; levelOne < names.length; levelOne++) { if (levelTwo < names[levelOne].length) { - objectId.fromRaw(names[levelOne], levelTwo); - int arrayIdx = levelTwo / (Constants.OBJECT_ID_LENGTH / 4) - * 4; - long offset = NB.decodeUInt32(offset32[levelOne], arrayIdx); + int idx = levelTwo / (Constants.OBJECT_ID_LENGTH / 4) * 4; + long offset = NB.decodeUInt32(offset32[levelOne], idx); if ((offset & IS_O64) != 0) { - arrayIdx = (8 * (int) (offset & ~IS_O64)); - offset = NB.decodeUInt64(offset64, arrayIdx); + idx = (8 * (int) (offset & ~IS_O64)); + offset = NB.decodeUInt64(offset64, idx); } - objectId.setOffset(offset); + entry.offset = offset; levelTwo += Constants.OBJECT_ID_LENGTH / 4; returnedNumber++; - return objectId; - } else { - levelTwo = 0; + return entry; } + levelTwo = 0; } throw new NoSuchElementException(); } -- 2.11.4.GIT