From b1bdeeb0ee670acbcfab089a39db5749d13ff9f1 Mon Sep 17 00:00:00 2001
From: Shawn Pearce <spearce@spearce.org>
Date: Tue, 18 Jul 2017 21:37:51 -0700
Subject: [PATCH] dfs: Use special ForReverseIndex DfsStreamKey wrapper instead
 of derive

While implementing a custom subclass of DfsStreamKey it became obvious
the required derive(String) was making it impossible to construct an
efficient key in all cases.

Instead, use a special wrapper type ForReverseIndex around the INDEX's
own DfsStreamKey to denote the reverse index stream in the
DfsBlockCache.  This adds a smaller layer of boxing, but eliminates
weird issues for DFS implementors using specialized DfsStreamKey
implementations for space efficiency reasons.

Now that DfsStreamKey is reasonably light-weight, avoid allocating the
index and reverse index keys until necessary.  DfsPackFile mostly
holds the DfsBlockCache.Ref handle to the object, and only needs the
DfsStreamKey when its looking up the handle.

Change-Id: Icea78e8f7f1514087b94ef5f525d9573ea2913f2
---
 .../internal/storage/dfs/DfsPackFile.java     | 21 +++++-------
 .../internal/storage/dfs/DfsStreamKey.java    | 32 +++++++++----------
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
index b4a882893..5620df42e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
@@ -88,10 +88,6 @@ import org.eclipse.jgit.util.LongList;
  * objects are similar.
  */
 public final class DfsPackFile extends BlockBasedFile {
-	final DfsStreamKey idxKey;
-	final DfsStreamKey reverseIdxKey;
-	DfsStreamKey bitmapKey;
-
 	/**
 	 * Lock for initialization of {@link #index} and {@link #corruptObjects}.
 	 * <p>
@@ -127,8 +123,6 @@ public final class DfsPackFile extends BlockBasedFile {
 	 */
 	DfsPackFile(DfsBlockCache cache, DfsPackDescription desc) {
 		super(cache, desc, PACK);
-		idxKey = desc.getStreamKey(INDEX);
-		reverseIdxKey = idxKey.derive("r"); //$NON-NLS-1$
 		length = desc.getFileSize(PACK);
 		if (length <= 0)
 			length = -1;
@@ -151,7 +145,7 @@ public final class DfsPackFile extends BlockBasedFile {
 		long objCnt = idx.getObjectCount();
 		int recSize = Constants.OBJECT_ID_LENGTH + 8;
 		int sz = (int) Math.min(objCnt * recSize, Integer.MAX_VALUE);
-		index = cache.put(idxKey, 0, sz, idx);
+		index = cache.put(desc.getStreamKey(INDEX), 0, sz, idx);
 	}
 
 	/**
@@ -254,9 +248,7 @@ public final class DfsPackFile extends BlockBasedFile {
 				if (idx != null)
 					return idx;
 			}
-			if (bitmapKey == null) {
-				bitmapKey = desc.getStreamKey(BITMAP_INDEX);
-			}
+
 			long size;
 			PackBitmapIndex idx;
 			try {
@@ -294,8 +286,9 @@ public final class DfsPackFile extends BlockBasedFile {
 				throw e2;
 			}
 
-			bitmapIndex = cache.put(bitmapKey, 0,
-					(int) Math.min(size, Integer.MAX_VALUE), idx);
+			bitmapIndex = cache.put(
+					desc.getStreamKey(BITMAP_INDEX),
+					0, (int) Math.min(size, Integer.MAX_VALUE), idx);
 			return idx;
 		}
 	}
@@ -320,7 +313,9 @@ public final class DfsPackFile extends BlockBasedFile {
 			PackReverseIndex revidx = new PackReverseIndex(idx);
 			int sz = (int) Math.min(
 					idx.getObjectCount() * 8, Integer.MAX_VALUE);
-			reverseIndex = cache.put(reverseIdxKey, 0, sz, revidx);
+			reverseIndex = cache.put(
+					new DfsStreamKey.ForReverseIndex(desc.getStreamKey(INDEX)),
+					0, sz, revidx);
 			return revidx;
 		}
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsStreamKey.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsStreamKey.java
index 5106666d5..f0a5da01f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsStreamKey.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsStreamKey.java
@@ -80,15 +80,6 @@ public abstract class DfsStreamKey {
 		this.hash = hash * 31;
 	}
 
-	/**
-	 * Derive a new StreamKey based on this existing key.
-	 *
-	 * @param suffix
-	 *            a derivation suffix.
-	 * @return derived stream key.
-	 */
-	public abstract DfsStreamKey derive(String suffix);
-
 	@Override
 	public int hashCode() {
 		return hash;
@@ -111,14 +102,6 @@ public abstract class DfsStreamKey {
 			this.name = name;
 		}
 
-		@Override
-		public DfsStreamKey derive(String suffix) {
-			byte[] s = suffix.getBytes(UTF_8);
-			byte[] n = Arrays.copyOf(name, name.length + s.length);
-			System.arraycopy(s, 0, n, name.length, s.length);
-			return new ByteArrayDfsStreamKey(n);
-		}
-
 		@Override
 		public boolean equals(Object o) {
 			if (o instanceof ByteArrayDfsStreamKey) {
@@ -128,4 +111,19 @@ public abstract class DfsStreamKey {
 			return false;
 		}
 	}
+
+	static final class ForReverseIndex extends DfsStreamKey {
+		private final DfsStreamKey idxKey;
+
+		ForReverseIndex(DfsStreamKey idxKey) {
+			super(idxKey.hash + 1);
+			this.idxKey = idxKey;
+		}
+
+		@Override
+		public boolean equals(Object o) {
+			return o instanceof ForReverseIndex
+					&& idxKey.equals(((ForReverseIndex) o).idxKey);
+		}
+	}
 }