DfsBlockCacheConfig: propagate hotmap configs to pack ext cache configs

CacheHotMap is currently only set on the base DfsBlockCacheConfig and is
not propagated down to PackExt specific caches.

Because CacheHotMap is set from a method call rather than from Configs,
this change sets per-PackExt CacheHotMap configs on PackExt cache
configs both when DfsBlockCacheConfig#setCacheHotMap(...) is called, and
when DfsBlockCacheConfig#configure(...) is called after setCacheHotMap.

The outer DfsBlockCacheConfig keeps the full CacheHotMap for the same
reason that the CacheHotMap config is propagated in both setCacheHotMap
and configure: the order of operations setting the configuration from
Configs and calling setCacheHotMap is not guaranteed.

Change-Id: Id9dc32fedca99ecc83c9dc90c24d9616873a202e

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java
index 65774e6d6ca78afbfd40ee9d9dfcbb25e086f91e..afa3179cdee3b11082ac8b140735dd1d3270b1af 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java
@@ -179,6 +179,76 @@ public class DfsBlockCacheConfigTest {
                                is(indexConfig));
        }
 
+       @Test
+       public void fromConfig_withExistingCacheHotMap_configWithPackExtConfigsHasHotMaps() {
+               Config config = new Config();
+               addPackExtConfigEntry(config, "pack", List.of(PackExt.PACK),
+                               /* blockLimit= */ 20 * 512, /* blockSize= */ 512);
+
+               addPackExtConfigEntry(config, "bitmap", List.of(PackExt.BITMAP_INDEX),
+                               /* blockLimit= */ 25 * 1024, /* blockSize= */ 1024);
+
+               addPackExtConfigEntry(config, "index",
+                               List.of(PackExt.INDEX, PackExt.OBJECT_SIZE_INDEX,
+                                               PackExt.REVERSE_INDEX),
+                               /* blockLimit= */ 30 * 1024, /* blockSize= */ 1024);
+
+               Map<PackExt, Integer> cacheHotMap = Map.of(PackExt.PACK, 1,
+                               PackExt.BITMAP_INDEX, 2, PackExt.INDEX, 3, PackExt.REFTABLE, 4);
+
+               DfsBlockCacheConfig cacheConfig = new DfsBlockCacheConfig();
+               cacheConfig.setCacheHotMap(cacheHotMap);
+               cacheConfig.fromConfig(config);
+
+               var configs = cacheConfig.getPackExtCacheConfigurations();
+               assertThat(cacheConfig.getCacheHotMap(), is(cacheHotMap));
+               assertThat(configs, hasSize(3));
+               var packConfig = getConfigForExt(configs, PackExt.PACK);
+               assertThat(packConfig.getCacheHotMap(), is(Map.of(PackExt.PACK, 1)));
+
+               var bitmapConfig = getConfigForExt(configs, PackExt.BITMAP_INDEX);
+               assertThat(bitmapConfig.getCacheHotMap(),
+                               is(Map.of(PackExt.BITMAP_INDEX, 2)));
+
+               var indexConfig = getConfigForExt(configs, PackExt.INDEX);
+               assertThat(indexConfig.getCacheHotMap(), is(Map.of(PackExt.INDEX, 3)));
+       }
+
+       @Test
+       public void setCacheHotMap_configWithPackExtConfigs_setsHotMaps() {
+               Config config = new Config();
+               addPackExtConfigEntry(config, "pack", List.of(PackExt.PACK),
+                               /* blockLimit= */ 20 * 512, /* blockSize= */ 512);
+
+               addPackExtConfigEntry(config, "bitmap", List.of(PackExt.BITMAP_INDEX),
+                               /* blockLimit= */ 25 * 1024, /* blockSize= */ 1024);
+
+               addPackExtConfigEntry(config, "index",
+                               List.of(PackExt.INDEX, PackExt.OBJECT_SIZE_INDEX,
+                                               PackExt.REVERSE_INDEX),
+                               /* blockLimit= */ 30 * 1024, /* blockSize= */ 1024);
+
+               Map<PackExt, Integer> cacheHotMap = Map.of(PackExt.PACK, 1,
+                               PackExt.BITMAP_INDEX, 2, PackExt.INDEX, 3, PackExt.REFTABLE, 4);
+
+               DfsBlockCacheConfig cacheConfig = new DfsBlockCacheConfig()
+                               .fromConfig(config);
+               cacheConfig.setCacheHotMap(cacheHotMap);
+
+               var configs = cacheConfig.getPackExtCacheConfigurations();
+               assertThat(cacheConfig.getCacheHotMap(), is(cacheHotMap));
+               assertThat(configs, hasSize(3));
+               var packConfig = getConfigForExt(configs, PackExt.PACK);
+               assertThat(packConfig.getCacheHotMap(), is(Map.of(PackExt.PACK, 1)));
+
+               var bitmapConfig = getConfigForExt(configs, PackExt.BITMAP_INDEX);
+               assertThat(bitmapConfig.getCacheHotMap(),
+                               is(Map.of(PackExt.BITMAP_INDEX, 2)));
+
+               var indexConfig = getConfigForExt(configs, PackExt.INDEX);
+               assertThat(indexConfig.getCacheHotMap(), is(Map.of(PackExt.INDEX, 3)));
+       }
+
        @Test
        public void fromConfigs_baseConfigOnly_nameSetFromConfigDfsSubSection() {
                Config config = new Config();
@@ -291,6 +361,7 @@ public class DfsBlockCacheConfigTest {
                                                "  Name: dfs.pack", "    BlockLimit: " + 20 * 512,
                                                "    BlockSize: 512", "    StreamRatio: 0.3",
                                                "    ConcurrencyLevel: 32",
+                                               "    CacheHotMapEntry: " + PackExt.PACK + " : " + 10,
                                                "    PackExts: " + List.of(PackExt.PACK))));
        }
 

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 acd7add5a904e25f834aed0d5d4e9406fe618ad6..d63c208bbbd1aab3f6f34f23f6808c1f875ee24b 100644 (file)
--- 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
@@ -30,6 +30,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import org.eclipse.jgit.internal.JGitText;
@@ -300,12 +301,24 @@ public class DfsBlockCacheConfig {
         *            map of hot count per pack extension for {@code DfsBlockCache}.
         * @return {@code this}
         */
+       /*
+        * TODO The cache HotMap configuration should be set as a config option and
+        * not passed in through a setter.
+        */
        public DfsBlockCacheConfig setCacheHotMap(
                        Map<PackExt, Integer> cacheHotMap) {
                this.cacheHotMap = Collections.unmodifiableMap(cacheHotMap);
+               setCacheHotMapToPackExtConfigs(this.cacheHotMap);
                return this;
        }
 
+       private void setCacheHotMapToPackExtConfigs(
+                       Map<PackExt, Integer> cacheHotMap) {
+               for (DfsBlockCachePackExtConfig packExtConfig : packExtCacheConfigurations) {
+                       packExtConfig.setCacheHotMap(cacheHotMap);
+               }
+       }
+
        /**
         * Get the consumer of cache index events.
         *
@@ -433,6 +446,7 @@ public class DfsBlockCacheConfig {
                        cacheConfigs.add(cacheConfig);
                }
                packExtCacheConfigurations = cacheConfigs;
+               setCacheHotMapToPackExtConfigs(this.cacheHotMap);
        }
 
        private static <T> Set<T> intersection(Set<T> first, Set<T> second) {
@@ -551,6 +565,14 @@ public class DfsBlockCacheConfig {
                        return packExtCacheConfiguration;
                }
 
+               void setCacheHotMap(Map<PackExt, Integer> cacheHotMap) {
+                       Map<PackExt, Integer> packExtHotMap = packExts.stream()
+                                       .filter(cacheHotMap::containsKey)
+                                       .collect(Collectors.toUnmodifiableMap(Function.identity(),
+                                                       cacheHotMap::get));
+                       packExtCacheConfiguration.setCacheHotMap(packExtHotMap);
+               }
+
                private static DfsBlockCachePackExtConfig fromConfig(Config config,
                                String section, String subSection) {
                        String packExtensions = config.getString(section, subSection,