From 2bb8defa5447c15a4fcbf23aaace9f11995f3ad0 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 08:52:43 -0700 Subject: [PATCH] IndexPack: Tighten up new and base object bookkeeping The only current consumer of these collections is ReceivePack, where it needs to test ObjectId equality between a RevObject and an ObjectId. There we were copying from a traditional HashSet into an ObjectIdSubclassMap, as the latter can perform hashing using ObjectId's native value support, bypassing RevObject's override on hashCode() and equals(). Instead of doing that copy, directly create ObjectIdSubclassMap instances inside of ReceivePack. We also only need to record the objects that do not appear in the incoming pack, and were therefore copied from the local repositiory in order to complete delta resolution. Instead of listing everything that used an OBJ_REF_DELTA format, list only the objects that we pulled from the destination repository via a normal ObjectLoader. ReceivePack can now discard the IndexPack object, and all of its other data, as soon as these collections are held by the check connectivity method. This frees up memory for the ObjectWalk's own RevObject pool. Change-Id: I22ef71b45c2045a0202e7fd550a770ee1f6f38a6 Signed-off-by: Shawn O. Pearce --- .../org/eclipse/jgit/transport/IndexPack.java | 52 +++++++++++-------- .../eclipse/jgit/transport/ReceivePack.java | 18 +++---- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java index 29f200c52..6eeccea84 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java @@ -54,10 +54,7 @@ import java.security.MessageDigest; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.zip.CRC32; import java.util.zip.DataFormatException; import java.util.zip.Deflater; @@ -173,7 +170,14 @@ public static IndexPack create(final Repository db, final InputStream is) private PackedObjectInfo[] entries; - private Set newObjectIds; + /** + * Every object contained within the incoming pack. + *

+ * This is a subset of {@link #entries}, as thin packs can add additional + * objects to {@code entries} by copying already existing objects from the + * repository onto the end of the thin pack to make it self-contained. + */ + private ObjectIdSubclassMap newObjectIds; private int deltaCount; @@ -183,7 +187,14 @@ public static IndexPack create(final Repository db, final InputStream is) private ObjectIdSubclassMap baseById; - private Set baseIds; + /** + * Objects referenced by their name from deltas, that aren't in this pack. + *

+ * This is the set of objects that were copied onto the end of this pack to + * make it complete. These objects were not transmitted by the remote peer, + * but instead were assumed to already exist in the local repository. + */ + private ObjectIdSubclassMap baseObjectIds; private LongMap baseByPos; @@ -287,7 +298,7 @@ public void setKeepEmpty(final boolean empty) { */ public void setNeedNewObjectIds(boolean b) { if (b) - newObjectIds = new HashSet(); + newObjectIds = new ObjectIdSubclassMap(); else newObjectIds = null; } @@ -311,17 +322,17 @@ public void setNeedBaseObjectIds(boolean b) { } /** @return the new objects that were sent by the user */ - public Set getNewObjectIds() { - return newObjectIds == null ? - Collections.emptySet() : newObjectIds; + public ObjectIdSubclassMap getNewObjectIds() { + if (newObjectIds != null) + return newObjectIds; + return new ObjectIdSubclassMap(); } - /** - * @return the set of objects the incoming pack assumed for delta purposes - */ - public Set getBaseObjectIds() { - return baseIds == null ? - Collections.emptySet() : baseIds; + /** @return set of objects the incoming pack assumed for delta purposes */ + public ObjectIdSubclassMap getBaseObjectIds() { + if (baseObjectIds != null) + return baseObjectIds; + return new ObjectIdSubclassMap(); } /** @@ -390,12 +401,6 @@ public void index(final ProgressMonitor progress) throws IOException { if (packOut == null) throw new IOException("need packOut"); resolveDeltas(progress); - if (needBaseObjectIds) { - baseIds = new HashSet(); - for (DeltaChain c : baseById) { - baseIds.add(c); - } - } if (entryCount < objectCount) { if (!fixThin) { throw new IOException("pack has " @@ -566,6 +571,9 @@ private void resolveChildDeltaChain(final int type, final byte[] data, private void fixThinPack(final ProgressMonitor progress) throws IOException { growEntries(); + if (needBaseObjectIds) + baseObjectIds = new ObjectIdSubclassMap(); + packDigest.reset(); originalEOF = packOut.length() - 20; final Deflater def = new Deflater(Deflater.DEFAULT_COMPRESSION, false); @@ -574,6 +582,8 @@ private void fixThinPack(final ProgressMonitor progress) throws IOException { for (final DeltaChain baseId : baseById) { if (baseId.head == null) continue; + if (needBaseObjectIds) + baseObjectIds.add(baseId); final ObjectLoader ldr = repo.openObject(readCurs, baseId); if (ldr == null) { missing.add(baseId); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 8dbeb9598..cce0a17d0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -776,12 +776,14 @@ private void receivePack() throws IOException { } private void checkConnectivity() throws IOException { - final Set baseObjects; + ObjectIdSubclassMap baseObjects = null; + ObjectIdSubclassMap providedObjects = null; - if (ensureObjectsProvidedVisible) + if (ensureObjectsProvidedVisible) { baseObjects = ip.getBaseObjectIds(); - else - baseObjects = Collections.emptySet(); + providedObjects = ip.getNewObjectIds(); + } + ip = null; final ObjectWalk ow = new ObjectWalk(db); for (final ReceiveCommand cmd : commands) { @@ -805,21 +807,17 @@ private void checkConnectivity() throws IOException { } } - ObjectIdSubclassMap provided = - new ObjectIdSubclassMap(); if (ensureObjectsProvidedVisible) { for (ObjectId id : baseObjects) { RevObject b = ow.lookupAny(id, Constants.OBJ_BLOB); if (!b.has(RevFlag.UNINTERESTING)) throw new MissingObjectException(b, b.getType()); } - for (ObjectId id : ip.getNewObjectIds()) - provided.add(id); } RevCommit c; while ((c = ow.next()) != null) { - if (ensureObjectsProvidedVisible && !provided.contains(c)) + if (ensureObjectsProvidedVisible && !providedObjects.contains(c)) throw new MissingObjectException(c, Constants.TYPE_COMMIT); } @@ -828,7 +826,7 @@ private void checkConnectivity() throws IOException { if (o instanceof RevBlob && !db.hasObject(o)) throw new MissingObjectException(o, Constants.TYPE_BLOB); - if (ensureObjectsProvidedVisible && !provided.contains(o)) + if (ensureObjectsProvidedVisible && !providedObjects.contains(o)) throw new MissingObjectException(o, Constants.TYPE_BLOB); } }