Fix NPE in DfsGarbageCollector and further reduce memory

DfsGarbageCollector asks PackWriter for the set of objects packed
after the bitmap index is written out.  This is now null as it was
cleared to release memory. Instead use PackBitmapIndexBuilder to
build the set as it also has the objects.

Reduce memory in PackBitmapIndexBuilder by fully discarding the
ObjectToPack instances. This was the original intent of commit
4bb523475d ("PackWriter: shed memory while creating bitmaps")
but failed as the instances were still held live here.

Switch to BlockList instead of ObjectToPack[]. This allows the
JVM to allocate many smaller arrays instead of one contiguous
array with 5.2M reference pointers. In a tight heap the smaller
allocations are more feasible.

Reduce the initial EWAHCompressedBitmaps for the 4 type maps.  On
average a typical repository is 30% commits, 30% trees and 30% blobs.
These bitmaps are typically very dense.  PackWriter orders objects by
commit, tree, blob when writing the file so these should always be a
very dense run of 1s with some 0s before and after. So even the 1/3rd
allocation is likely too large, but the later trim() will reduce the
internal buffer anyway.

Change-Id: If0b80a31cb00894f1485ff8f53ef7ae5a759a046

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexBuilder.java

@@ -44,7 +44,7 @@
 package org.eclipse.jgit.internal.storage.file;
 
 import java.text.MessageFormat;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
@@ -73,7 +73,7 @@ public class PackBitmapIndexBuilder extends BasePackBitmapIndex {
 	private final EWAHCompressedBitmap trees;
 	private final EWAHCompressedBitmap blobs;
 	private final EWAHCompressedBitmap tags;
-	private final ObjectToPack[] byOffset;
+	private final BlockList<PositionEntry> byOffset;
 	private final BlockList<StoredBitmap>
 			byAddOrder = new BlockList<StoredBitmap>();
 	private final ObjectIdOwnerMap<PositionEntry>
@@ -83,20 +83,25 @@ public class PackBitmapIndexBuilder extends BasePackBitmapIndex {
 	 * Creates a PackBitmapIndex used for building the contents of an index
 	 * file.
 	 *
-	 * @param byName
+	 * @param objects
-	 *            objects sorted by name.
+	 *            objects sorted by name. The list must be initially sorted by
+	 *            ObjectId (name); it will be resorted in place.
 	 */
-	public PackBitmapIndexBuilder(List<ObjectToPack> byName) {
+	public PackBitmapIndexBuilder(List<ObjectToPack> objects) {
 		super(new ObjectIdOwnerMap<StoredBitmap>());
-		byOffset = sortByOffset(byName);
+		byOffset = new BlockList<>(objects.size());
+		sortByOffsetAndIndex(byOffset, positionEntries, objects);
 
-		int sizeInWords = Math.max(byOffset.length / 64, 4);
+		// 64 objects fit in a single long word (64 bits).
+		// On average a repository is 30% commits, 30% trees, 30% blobs.
+		// Initialize bitmap capacity for worst case to minimize growing.
+		int sizeInWords = Math.max(4, byOffset.size() / 64 / 3);
 		commits = new EWAHCompressedBitmap(sizeInWords);
 		trees = new EWAHCompressedBitmap(sizeInWords);
 		blobs = new EWAHCompressedBitmap(sizeInWords);
 		tags = new EWAHCompressedBitmap(sizeInWords);
-		for (int i = 0; i < byOffset.length; i++) {
+		for (int i = 0; i < objects.size(); i++) {
-			int type = byOffset[i].getType();
+			int type = objects.get(i).getType();
 			switch (type) {
 			case Constants.OBJ_COMMIT:
 				commits.set(i);
@@ -121,20 +126,33 @@ public class PackBitmapIndexBuilder extends BasePackBitmapIndex {
 		tags.trim();
 	}
 
-	private ObjectToPack[] sortByOffset(List<ObjectToPack> entries) {
+	private static void sortByOffsetAndIndex(BlockList<PositionEntry> byOffset,
-		ObjectToPack[] result = new ObjectToPack[entries.size()];
+			ObjectIdOwnerMap<PositionEntry> positionEntries,
-		for (int i = 0; i < result.length; i++) {
+			List<ObjectToPack> entries) {
-			result[i] = entries.get(i);
+		for (int i = 0; i < entries.size(); i++) {
-			positionEntries.add(new PositionEntry(result[i], i));
+			positionEntries.add(new PositionEntry(entries.get(i), i));
 		}
-		Arrays.sort(result, new Comparator<ObjectToPack>() {
+		Collections.sort(entries, new Comparator<ObjectToPack>() {
 			public int compare(ObjectToPack a, ObjectToPack b) {
 				return Long.signum(a.getOffset() - b.getOffset());
 			}
 		});
-		for (int i = 0; i < result.length; i++)
+		for (int i = 0; i < entries.size(); i++) {
-			positionEntries.get(result[i]).offsetPosition = i;
+			PositionEntry e = positionEntries.get(entries.get(i));
-		return result;
+			e.offsetPosition = i;
+			byOffset.add(e);
+		}
+	}
+
+	/** @return set of objects included in the pack. */
+	public ObjectIdOwnerMap<ObjectIdOwnerMap.Entry> getObjectSet() {
+		ObjectIdOwnerMap<ObjectIdOwnerMap.Entry> r = new ObjectIdOwnerMap<>();
+		for (PositionEntry e : byOffset) {
+			r.add(new ObjectIdOwnerMap.Entry(e) {
+				// A new entry that copies the ObjectId
+			});
+		}
+		return r;
 	}
 
 	/**
@@ -204,7 +222,7 @@ public class PackBitmapIndexBuilder extends BasePackBitmapIndex {
 
 	@Override
 	public ObjectId getObject(int position) throws IllegalArgumentException {
-		ObjectId objectId = byOffset[position];
+		ObjectId objectId = byOffset.get(position);
 		if (objectId == null)
 			throw new IllegalArgumentException();
 		return objectId;
@@ -248,7 +266,7 @@ public class PackBitmapIndexBuilder extends BasePackBitmapIndex {
 
 	@Override
 	public int getObjectCount() {
-		return byOffset.length;
+		return byOffset.size();
 	}
 
 	/** @return an iterator over the xor compressed entries. */

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java

@@ -608,12 +608,12 @@ public class PackWriter implements AutoCloseable {
 
 	/**
 	 * Returns the object ids in the pack file that was created by this writer.
-	 *
+	 * <p>
 	 * This method can only be invoked after
 	 * {@link #writePack(ProgressMonitor, ProgressMonitor, OutputStream)} has
 	 * been invoked and completed successfully.
 	 *
-	 * @return number of objects in pack.
+	 * @return set of objects in pack.
 	 * @throws IOException
 	 *             a cached pack cannot supply its object ids.
 	 */
@@ -623,17 +623,20 @@ public class PackWriter implements AutoCloseable {
 			throw new IOException(
 					JGitText.get().cachedPacksPreventsListingObjects);
 
-		ObjectIdOwnerMap<ObjectIdOwnerMap.Entry> objs = new ObjectIdOwnerMap<
+		if (writeBitmaps != null) {
-				ObjectIdOwnerMap.Entry>();
+			return writeBitmaps.getObjectSet();
+		}
+
+		ObjectIdOwnerMap<ObjectIdOwnerMap.Entry> r = new ObjectIdOwnerMap<>();
 		for (BlockList<ObjectToPack> objList : objectsLists) {
 			if (objList != null) {
 				for (ObjectToPack otp : objList)
-					objs.add(new ObjectIdOwnerMap.Entry(otp) {
+					r.add(new ObjectIdOwnerMap.Entry(otp) {
 						// A new entry that copies the ObjectId
 					});
 			}
 		}
-		return objs;
+		return r;
 	}
 
 	/**