From b6d0586befb3417a72730443770abb223b9cae83 Mon Sep 17 00:00:00 2001
From: "Shawn O. Pearce" <spearce@spearce.org>
Date: Sat, 15 May 2010 17:51:03 -0700
Subject: [PATCH] Reduce the size of PackWriter's ObjectToPack instances

Rather than holding onto the PackedObjectLoader, only hold the
PackFile and the object offset.  During a reuse copy that is all
we should need to complete a reuse, and the other parts of the
PackedObjectLoader just waste memory.

This change reduces the per-object memory usage of a PackWriter by
32 bytes on a 32 bit JVM using only OFS_DELTA formatted objects.
The savings is even larger (by another 20 bytes) for REF_DELTAs.
This is close to a 50% reduction in the size of ObjectToPack,
making it rather worthwhile to do.

Beyond the memory reduction, this change will help to make future
refactoring work easier.  We need to redo the API used to support
copying data, and disconnecting it from the PackedObjectLoader is
a good first step.

Change-Id: I24ba4e621e101f14e79a16463aec5379f447aa9b
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/eclipse/jgit/lib/PackWriter.java  | 44 ++++++++++---------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java
index 0c8210f64..4a205fffb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java
@@ -623,7 +623,7 @@ public class PackWriter {
 			selectDeltaReuseForObject(otp, reuseLoaders);
 		}
 		// delta reuse is preferred over object reuse
-		if (reuseObjects && !otp.hasReuseLoader()) {
+		if (reuseObjects && !otp.isCopyable()) {
 			selectObjectReuseForObject(otp, reuseLoaders);
 		}
 	}
@@ -649,7 +649,7 @@ public class PackWriter {
 		}
 
 		if (bestLoader != null) {
-			otp.setReuseLoader(bestLoader);
+			otp.setCopyFromPack(bestLoader);
 			otp.setDeltaBase(bestBase);
 		}
 	}
@@ -670,7 +670,7 @@ public class PackWriter {
 			final Collection<PackedObjectLoader> loaders) {
 		for (final PackedObjectLoader loader : loaders) {
 			if (loader instanceof WholePackedObjectLoader) {
-				otp.setReuseLoader(loader);
+				otp.setCopyFromPack(loader);
 				return;
 			}
 		}
@@ -703,7 +703,7 @@ public class PackWriter {
 			if (deltaBase != null && !deltaBase.isWritten()) {
 				if (deltaBase.wantWrite()) {
 					otp.clearDeltaBase(); // cycle detected
-					otp.disposeLoader();
+					otp.clearSourcePack();
 				} else {
 					writeObject(deltaBase);
 				}
@@ -737,13 +737,9 @@ public class PackWriter {
 	}
 
 	private PackedObjectLoader open(final ObjectToPack otp) throws IOException {
-		for (;;) {
-			PackedObjectLoader reuse = otp.useLoader();
-			if (reuse == null) {
-				return null;
-			}
-
+		while (otp.isCopyable()) {
 			try {
+				PackedObjectLoader reuse = otp.getCopyLoader(windowCursor);
 				reuse.beginCopyRawData();
 				return reuse;
 			} catch (IOException err) {
@@ -751,10 +747,12 @@ public class PackWriter {
 				// it has been overwritten with a different layout.
 				//
 				otp.clearDeltaBase();
+				otp.clearSourcePack();
 				searchForReuse(new ArrayList<PackedObjectLoader>(), otp);
 				continue;
 			}
 		}
+		return null;
 	}
 
 	private void writeWholeObjectDeflate(final ObjectToPack otp)
@@ -904,9 +902,14 @@ public class PackWriter {
 	 *
 	 */
 	static class ObjectToPack extends PackedObjectInfo {
+		/** Other object being packed that this will delta against. */
 		private ObjectId deltaBase;
 
-		private PackedObjectLoader reuseLoader;
+		/** Pack to reuse compressed data from, otherwise null. */
+		private PackFile copyFromPack;
+
+		/** Offset of the object's header in {@link #copyFromPack}. */
+		private long copyOffset;
 
 		/**
 		 * Bit field, from bit 0 to bit 31:
@@ -989,22 +992,21 @@ public class PackWriter {
 			return getOffset() != 0;
 		}
 
-		PackedObjectLoader useLoader() {
-			final PackedObjectLoader r = reuseLoader;
-			reuseLoader = null;
-			return r;
+		boolean isCopyable() {
+			return copyFromPack != null;
 		}
 
-		boolean hasReuseLoader() {
-			return reuseLoader != null;
+		PackedObjectLoader getCopyLoader(WindowCursor curs) throws IOException {
+			return copyFromPack.resolveBase(curs, copyOffset);
 		}
 
-		void setReuseLoader(PackedObjectLoader reuseLoader) {
-			this.reuseLoader = reuseLoader;
+		void setCopyFromPack(PackedObjectLoader loader) {
+			this.copyFromPack = loader.pack;
+			this.copyOffset = loader.objectOffset;
 		}
 
-		void disposeLoader() {
-			this.reuseLoader = null;
+		void clearSourcePack() {
+			copyFromPack = null;
 		}
 
 		int getType() {