Replace DeltaWindow array with circularly linked list

Typical window sizes are 10 and 250 (although others are accepted).
In either case the pointer overhead of 1 pointer in an array or
2 pointers for a double linked list is trivial.  A doubly linked
list as used here for window=250 is only another 1024 bytes on a
32 bit machine, or 2048 bytes on a 64 bit machine.

The critical search loops scan through the array in either the
previous direction or the next direction until the cycle is finished,
or some other scan abort condition is reached.  Loading the next
object's pointer from a field in the current object avoids the
branch required to test for wrapping around the edge of the array.
It also saves the array bounds check on each access.

When a delta is chosen the window is shuffled to hoist the currently
selected base as an earlier candidate for the next object. Moving
the window entry is easier in a double-linked list than sliding a
group of array entries.

Change-Id: I9ccf20c3362a78678aede0f0f2cda165e509adff

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

@@ -65,8 +65,6 @@ final class DeltaWindow {
 	private final ObjectReader reader;
 	private final ProgressMonitor monitor;
 
-	private final DeltaWindowEntry[] window;
-
 	/** Maximum number of bytes to admit to the window at once. */
 	private final long maxMemory;
 
@@ -74,9 +72,7 @@ final class DeltaWindow {
 	private final int maxDepth;
 
 	private final ObjectToPack[] toSearch;
-
 	private int cur;
-
 	private int end;
 
 	/** Amount of memory we have loaded right now. */
@@ -84,9 +80,6 @@ final class DeltaWindow {
 
 	// The object we are currently considering needs a lot of state:
 
-	/** Position of {@link #res} within {@link #window} array. */
-	private int resSlot;
-
 	/**
 	 * Maximum delta chain depth the current object can have.
 	 * <p>
@@ -100,8 +93,8 @@ final class DeltaWindow {
 	/** If we have a delta for {@link #res}, this is the shortest found yet. */
 	private TemporaryBuffer.Heap bestDelta;
 
-	/** If we have {@link #bestDelta}, the window position it was created by. */
+	/** If we have {@link #bestDelta}, the window entry it was created from. */
-	private int bestSlot;
+	private DeltaWindowEntry bestBase;
 
 	/** Used to compress cached deltas. */
 	private Deflater deflater;
@@ -117,23 +110,9 @@ final class DeltaWindow {
 		cur = beginIndex;
 		end = endIndex;
 
-		// C Git increases the window size supplied by the user by 1.
-		// We don't know why it does this, but if the user asks for
-		// window=10, it actually processes with window=11. Because
-		// the window size has the largest direct impact on the final
-		// pack file size, we match this odd behavior here to give us
-		// a better chance of producing a similar sized pack as C Git.
-		//
-		// We would prefer to directly honor the user's request since
-		// PackWriter has a minimum of 2 for the window size, but then
-		// users might complain that JGit is creating a bigger pack file.
-		//
-		window = new DeltaWindowEntry[config.getDeltaSearchWindowSize() + 1];
-		for (int i = 0; i < window.length; i++)
-			window[i] = new DeltaWindowEntry();
-
 		maxMemory = Math.max(0, config.getDeltaSearchMemoryLimit());
 		maxDepth = config.getMaxDeltaDepth();
+		res = DeltaWindowEntry.createWindow(config.getDeltaSearchWindowSize());
 	}
 
 	synchronized int remaining() {
@@ -167,15 +146,12 @@ final class DeltaWindow {
 						break;
 					next = toSearch[cur++];
 				}
-				res = window[resSlot];
 				if (maxMemory != 0) {
 					clear(res);
-					int tail = next(resSlot);
 					final long need = estimateSize(next);
-					while (maxMemory < loaded + need && tail != resSlot) {
+					DeltaWindowEntry n = res.next;
-						clear(window[tail]);
+					for (; maxMemory < loaded + need && n != res; n = n.next)
-						tail = next(tail);
+						clear(n);
-					}
 				}
 				res.set(next);
 
@@ -231,11 +207,10 @@ final class DeltaWindow {
 		// Loop through the window backwards, considering every entry.
 		// This lets us look at the bigger objects that came before.
 		//
-		for (int srcSlot = prior(resSlot); srcSlot != resSlot; srcSlot = prior(srcSlot)) {
+		for (DeltaWindowEntry src = res.prev; src != res; src = src.prev) {
-			DeltaWindowEntry src = window[srcSlot];
 			if (src.empty())
 				break;
-			if (delta(src, srcSlot) /* == NEXT_SRC */)
+			if (delta(src) /* == NEXT_SRC */)
 				continue;
 			bestDelta = null;
 			return;
@@ -251,7 +226,7 @@ final class DeltaWindow {
 
 		// Select this best matching delta as the base for the object.
 		//
-		ObjectToPack srcObj = window[bestSlot].object;
+		ObjectToPack srcObj = bestBase.object;
 		ObjectToPack resObj = res.object;
 		if (srcObj.isEdge()) {
 			// The source (the delta base) is an edge object outside of the
@@ -285,7 +260,7 @@ final class DeltaWindow {
 		keepInWindow();
 	}
 
-	private boolean delta(final DeltaWindowEntry src, final int srcSlot)
+	private boolean delta(final DeltaWindowEntry src)
 			throws IOException {
 		// Objects must use only the same type as their delta base.
 		// If we are looking at something where that isn't true we
@@ -319,12 +294,12 @@ final class DeltaWindow {
 			srcIndex = index(src);
 		} catch (LargeObjectException tooBig) {
 			// If the source is too big to work on, skip it.
-			dropFromWindow(srcSlot);
+			dropFromWindow(src);
 			return NEXT_SRC;
 		} catch (IOException notAvailable) {
 			if (src.object.isEdge()) {
 				// This is an edge that is suddenly not available.
-				dropFromWindow(srcSlot);
+				dropFromWindow(src);
 				return NEXT_SRC;
 			} else {
 				throw notAvailable;
@@ -355,7 +330,7 @@ final class DeltaWindow {
 
 		if (isBetterDelta(src, delta)) {
 			bestDelta = delta;
-			bestSlot = srcSlot;
+			bestBase = src;
 		}
 
 		return NEXT_SRC;
@@ -390,39 +365,17 @@ final class DeltaWindow {
 	}
 
 	private void shuffleBaseUpInPriority() {
-		// Shuffle the entire window so that the best match we just used
+		// Reorder the window so that the best match we just used
-		// is at our current index, and our current object is at the index
+		// is the current one, and the now current object is before.
-		// before it. Slide any entries in between to make space.
+		res.makeNext(bestBase);
-		//
+		res = bestBase;
-		window[resSlot] = window[bestSlot];
-
-		DeltaWindowEntry next = res;
-		int slot = prior(resSlot);
-		for (; slot != bestSlot; slot = prior(slot)) {
-			DeltaWindowEntry e = window[slot];
-			window[slot] = next;
-			next = e;
-		}
-		window[slot] = next;
 	}
 
 	private void keepInWindow() {
-		resSlot = next(resSlot);
+		res = res.next;
-	}
-
-	private int next(int slot) {
-		if (++slot == window.length)
-			return 0;
-		return slot;
-	}
-
-	private int prior(int slot) {
-		if (slot == 0)
-			return window.length - 1;
-		return slot - 1;
 	}
 
-	private void dropFromWindow(@SuppressWarnings("unused") int srcSlot) {
+	private void dropFromWindow(@SuppressWarnings("unused") DeltaWindowEntry src) {
 		// We should drop the current source entry from the window,
 		// it is somehow invalid for us to work with.
 	}
@@ -437,7 +390,7 @@ final class DeltaWindow {
 		// to access during reads.
 		//
 		if (resDelta.length() == bestDelta.length())
-			return src.depth() < window[bestSlot].depth();
+			return src.depth() < bestBase.depth();
 
 		return resDelta.length() < bestDelta.length();
 	}
@@ -501,14 +454,12 @@ final class DeltaWindow {
 		if (maxMemory == 0)
 			return;
 
-		int tail = next(resSlot);
+		DeltaWindowEntry n = res.next;
-		while (maxMemory < loaded + need) {
+		for (; maxMemory < loaded + need; n = n.next) {
-			DeltaWindowEntry cur = window[tail];
+			clear(n);
-			clear(cur);
+			if (n == ent)
-			if (cur == ent)
 				throw new LargeObjectException.ExceedsLimit(
 						maxMemory, loaded + need);
-			tail = next(tail);
 		}
 	}
 

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

@@ -44,6 +44,8 @@
 package org.eclipse.jgit.internal.storage.pack;
 
 final class DeltaWindowEntry {
+	DeltaWindowEntry prev;
+	DeltaWindowEntry next;
 	ObjectToPack object;
 
 	/** Complete contents of this object. Lazily loaded. */
@@ -77,4 +79,40 @@ final class DeltaWindowEntry {
 	final boolean empty() {
 		return object == null;
 	}
+
+	final void makeNext(DeltaWindowEntry e) {
+		// Disconnect e from the chain.
+		e.prev.next = e.next;
+		e.next.prev = e.prev;
+
+		// Insert e after this.
+		e.next = next;
+		e.prev = this;
+		next.prev = e;
+		next = e;
+	}
+
+	static DeltaWindowEntry createWindow(int cnt) {
+		// C Git increases the window size supplied by the user by 1.
+		// We don't know why it does this, but if the user asks for
+		// window=10, it actually processes with window=11. Because
+		// the window size has the largest direct impact on the final
+		// pack file size, we match this odd behavior here to give us
+		// a better chance of producing a similar sized pack as C Git.
+		//
+		// We would prefer to directly honor the user's request since
+		// PackWriter has a minimum of 2 for the window size, but then
+		// users might complain that JGit is creating a bigger pack file.
+		DeltaWindowEntry res = new DeltaWindowEntry();
+		DeltaWindowEntry p = res;
+		for (int i = 0; i < cnt; i++) {
+			DeltaWindowEntry e = new DeltaWindowEntry();
+			e.prev = p;
+			p.next = e;
+			p = e;
+		}
+		p.next = res;
+		res.prev = p;
+		return res;
+	}
 }