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: Icea78e8f7f1514087b94ef5f525d9573ea2913f2tags/v4.9.0.201710071750-r
@@ -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; | |||
} | |||
} |
@@ -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); | |||
} | |||
} | |||
} |