From 7d4f3c22ab26b98b703873c3554fec514f109752 Mon Sep 17 00:00:00 2001 From: Alina Djamankulova Date: Mon, 23 Aug 2021 18:22:41 -0700 Subject: [PATCH] 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 Signed-off-by: Alina Djamankulova Change-Id: I18062784ec9cc14dbba3e4bb8d9509440cf2d44f --- .../storage/dfs/DfsBlockCacheTest.java | 41 +++++++++++++++ .../internal/storage/dfs/DfsBlockCache.java | 52 ++++++++++++++----- .../storage/dfs/DfsBlockCacheConfig.java | 31 +++++++++++ 3 files changed, 112 insertions(+), 12 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTest.java index cea00daad8..4f300bcd8d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTest.java @@ -16,9 +16,12 @@ import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.stream.LongStream; +import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.junit.TestRng; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; @@ -103,6 +106,44 @@ public class DfsBlockCacheTest { } } + @SuppressWarnings("resource") + @Test + public void hasCacheHotMap() throws Exception { + Map cacheHotMap = new HashMap<>(); + // Pack index will be kept in cache longer. + cacheHotMap.put(PackExt.INDEX, Integer.valueOf(3)); + DfsBlockCache.reconfigure(new DfsBlockCacheConfig().setBlockSize(512) + .setBlockLimit(512 * 4).setCacheHotMap(cacheHotMap)); + cache = DfsBlockCache.getInstance(); + + DfsRepositoryDescription repo = new DfsRepositoryDescription("test"); + InMemoryRepository r1 = new InMemoryRepository(repo); + byte[] content = rng.nextBytes(424242); + ObjectId id; + try (ObjectInserter ins = r1.newObjectInserter()) { + id = ins.insert(OBJ_BLOB, content); + ins.flush(); + } + + try (ObjectReader rdr = r1.newObjectReader()) { + byte[] actual = rdr.open(id, OBJ_BLOB).getBytes(); + assertTrue(Arrays.equals(content, actual)); + } + // All cache entries are hot and cache is at capacity. + assertTrue(LongStream.of(cache.getHitCount()).sum() > 0); + assertEquals(99, cache.getFillPercentage()); + + InMemoryRepository r2 = new InMemoryRepository(repo); + content = rng.nextBytes(424242); + try (ObjectInserter ins = r2.newObjectInserter()) { + ins.insert(OBJ_BLOB, content); + ins.flush(); + } + assertEquals(0, LongStream.of(cache.getMissCount()).sum()); + assertTrue(cache.getEvictions()[PackExt.PACK.getPosition()] > 0); + assertEquals(0, cache.getEvictions()[PackExt.INDEX.getPosition()]); + } + private void resetCache() { DfsBlockCache.reconfigure(new DfsBlockCacheConfig() .setBlockSize(512) 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. *

* 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 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 refLock; + private Map cacheHotMap; + /** * Create a default configuration. */ @@ -49,6 +57,7 @@ public class DfsBlockCacheConfig { setBlockSize(64 * KB); setStreamRatio(0.30); setConcurrencyLevel(32); + cacheHotMap = Collections.emptyMap(); } /** @@ -184,6 +193,28 @@ public class DfsBlockCacheConfig { return this; } + /** + * 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 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 cacheHotMap) { + this.cacheHotMap = Collections.unmodifiableMap(cacheHotMap); + return this; + } + /** * Update properties by setting fields from the configuration. *

-- 2.39.5