DFS block cache: allow multiple passes for blocks before eviction

Let certain pack extensions that are expensive to load from storage
(e.g. pack index, bitmap index) stay in DFS block cache longer than
others by overriding default cache count through DfsBlockCacheConfig

Don't change default behavior when cache override map is empty. Use int
cacheCount instead of boolean hot for Ref<T>

Signed-off-by: Alina Djamankulova <adjama@google.com>
Change-Id: I18062784ec9cc14dbba3e4bb8d9509440cf2d44f

2 files changed, 71 insertions, 12 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java
index 092d035b3d..e87bfe24e6 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java
@@ -39,9 +39,10 @@ import org.eclipse.jgit.internal.storage.pack.PackExt;
  * Its too expensive during object access to be accurate with a least recently
  * used (LRU) algorithm. Strictly ordering every read is a lot of overhead that
  * typically doesn't yield a corresponding benefit to the application. This
- * cache implements a clock replacement algorithm, giving each block one chance
- * to have been accessed during a sweep of the cache to save itself from
- * eviction.
+ * cache implements a clock replacement algorithm, giving each block at least
+ * one chance to have been accessed during a sweep of the cache to save itself
+ * from eviction. The number of swipe chances is configurable per pack
+ * extension.
  * <p>
  * Entities created by the cache are held under hard references, preventing the
  * Java VM from clearing anything. Blocks are discarded by the replacement
@@ -161,6 +162,9 @@ public final class DfsBlockCache {
 	/** Current position of the clock. */
 	private Ref clockHand;
 
+	/** Limits of cache hot count per pack file extension. */
+	private final int[] cacheHotLimits = new int[PackExt.values().length];
+
 	@SuppressWarnings("unchecked")
 	private DfsBlockCache(DfsBlockCacheConfig cfg) {
 		tableSize = tableSize(cfg);
@@ -196,6 +200,15 @@ public final class DfsBlockCache {
 		liveBytes = new AtomicReference<>(newCounters());
 
 		refLockWaitTime = cfg.getRefLockWaitTimeConsumer();
+
+		for (int i = 0; i < PackExt.values().length; ++i) {
+			Integer limit = cfg.getCacheHotMap().get(PackExt.values()[i]);
+			if (limit != null && limit.intValue() > 0) {
+				cacheHotLimits[i] = limit.intValue();
+			} else {
+				cacheHotLimits[i] = DfsBlockCacheConfig.DEFAULT_CACHE_HOT_MAX;
+			}
+		}
 	}
 
 	boolean shouldCopyThroughCache(long length) {
@@ -394,7 +407,7 @@ public final class DfsBlockCache {
 			}
 
 			Ref<DfsBlock> ref = new Ref<>(key, position, v.size(), v);
-			ref.hot = true;
+			ref.markHotter();
 			for (;;) {
 				HashEntry n = new HashEntry(clean(e2), ref);
 				if (table.compareAndSet(slot, e2, n)) {
@@ -424,10 +437,10 @@ public final class DfsBlockCache {
 				Ref prev = clockHand;
 				Ref hand = clockHand.next;
 				do {
-					if (hand.hot) {
-						// Value was recently touched. Clear
-						// hot and give it another chance.
-						hand.hot = false;
+					if (hand.isHot()) {
+						// Value was recently touched. Cache is still hot so
+						// give it another chance, but cool it down a bit.
+						hand.markColder();
 						prev = hand;
 						hand = hand.next;
 						continue;
@@ -525,7 +538,7 @@ public final class DfsBlockCache {
 			}
 			getStat(statMiss, key).incrementAndGet();
 			ref = loader.load();
-			ref.hot = true;
+			ref.markHotter();
 			// Reserve after loading to get the size of the object
 			reserveSpace(ref.size, key);
 			for (;;) {
@@ -568,7 +581,7 @@ public final class DfsBlockCache {
 			}
 
 			ref = new Ref<>(key, pos, size, v);
-			ref.hot = true;
+			ref.markHotter();
 			for (;;) {
 				HashEntry n = new HashEntry(clean(e2), ref);
 				if (table.compareAndSet(slot, e2, n)) {
@@ -692,7 +705,8 @@ public final class DfsBlockCache {
 		final long size;
 		volatile T value;
 		Ref next;
-		volatile boolean hot;
+
+		private volatile int hotCount;
 
 		Ref(DfsStreamKey key, long position, long size, T v) {
 			this.key = key;
@@ -704,7 +718,7 @@ public final class DfsBlockCache {
 		T get() {
 			T v = value;
 			if (v != null) {
-				hot = true;
+				markHotter();
 			}
 			return v;
 		}
@@ -712,6 +726,20 @@ public final class DfsBlockCache {
 		boolean has() {
 			return value != null;
 		}
+
+		void markHotter() {
+			int cap = DfsBlockCache
+					.getInstance().cacheHotLimits[key.packExtPos];
+			hotCount = Math.min(cap, hotCount + 1);
+		}
+
+		void markColder() {
+			hotCount = Math.max(0, hotCount - 1);
+		}
+
+		boolean isHot() {
+			return hotCount > 0;
+		}
 	}
 
 	@FunctionalInterface
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
index 6e7ad3e613..2716f79a1a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
@@ -18,9 +18,12 @@ import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_CONCURRENCY_LEVEL;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_STREAM_RATIO;
 
 import java.text.MessageFormat;
+import java.util.Collections;
+import java.util.Map;
 import java.util.function.Consumer;
 
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.lib.Config;
 
 /**
@@ -34,6 +37,9 @@ public class DfsBlockCacheConfig {
 	/** 1024 {@link #KB} (number of bytes in one mebibyte/megabyte) */
 	public static final int MB = 1024 * KB;
 
+	/** Default number of max cache hits. */
+	public static final int DEFAULT_CACHE_HOT_MAX = 1;
+
 	private long blockLimit;
 	private int blockSize;
 	private double streamRatio;
@@ -41,6 +47,8 @@ public class DfsBlockCacheConfig {
 
 	private Consumer<Long> refLock;
 
+	private Map<PackExt, Integer> cacheHotMap;
+
 	/**
 	 * Create a default configuration.
 	 */
@@ -49,6 +57,7 @@ public class DfsBlockCacheConfig {
 		setBlockSize(64 * KB);
 		setStreamRatio(0.30);
 		setConcurrencyLevel(32);
+		cacheHotMap = Collections.emptyMap();
 	}
 
 	/**
@@ -185,6 +194,28 @@ public class DfsBlockCacheConfig {
 	}
 
 	/**
+	 * Get the map of hot count per pack extension for {@code DfsBlockCache}.
+	 *
+	 * @return map of hot count per pack extension for {@code DfsBlockCache}.
+	 */
+	public Map<PackExt, Integer> getCacheHotMap() {
+		return cacheHotMap;
+	}
+
+	/**
+	 * Set the map of hot count per pack extension for {@code DfsBlockCache}.
+	 *
+	 * @param cacheHotMap
+	 *            map of hot count per pack extension for {@code DfsBlockCache}.
+	 * @return {@code this}
+	 */
+	public DfsBlockCacheConfig setCacheHotMap(
+			Map<PackExt, Integer> cacheHotMap) {
+		this.cacheHotMap = Collections.unmodifiableMap(cacheHotMap);
+		return this;
+	}
+
+	/**
 	 * Update properties by setting fields from the configuration.
 	 * <p>
 	 * If a property is not defined in the configuration, then it is left