From a2a5cdddd78a1e066aed7caf86ba9510450710c9 Mon Sep 17 00:00:00 2001 From: Laura Hamelin Date: Fri, 7 Jun 2024 16:11:21 -0700 Subject: [PATCH] PackExtBlockCacheTable: spread extensions over multiple dfs tables The existing DfsBlockCache uses a single table for all extensions (idx, ridx, ...). This change introduces an implementation of the table interface that can keep extensions in different cache tables. This selects the appropriate cache to use for a specific PackExt or DfsStreamKey's PackExt type, allowing the separation of entries from different pack types to help limit churn in cache caused by entries of differing sizes. This is especially useful in fine-tuning caches and influencing interactions by extension type. For example, a table holding INDEX types only will not influence evictions of other PackExt types and vice versa. The PackExtBlockCacheTable allowing setting the underlying DfsBlockCacheTables and mappinh directly, letting users implement and use custom DfsBlockCacheTables. Change-Id: Icee7b644ef6b600aa473d35645469d6aa1bce345 --- .../dfs/PackExtBlockCacheTableTest.java | 588 ++++++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 2 + .../org/eclipse/jgit/internal/JGitText.java | 2 + .../storage/dfs/DfsBlockCacheConfig.java | 26 +- .../storage/dfs/PackExtBlockCacheTable.java | 289 +++++++++ 5 files changed, 905 insertions(+), 2 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java new file mode 100644 index 0000000000..d506bfba40 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java @@ -0,0 +1,588 @@ +/* + * Copyright (c) 2024, Google LLC and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.storage.dfs; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.when; + +import java.util.EnumSet; +import java.util.List; +import java.util.Map; + +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCache.Ref; +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCache.RefLoader; +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCacheConfig.DfsBlockCachePackExtConfig; +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCacheTable.DfsBlockCacheStats; +import org.eclipse.jgit.internal.storage.pack.PackExt; +import org.junit.Test; +import org.mockito.Mockito; + +public class PackExtBlockCacheTableTest { + @Test + public void fromBlockCacheConfigs_createsDfsPackExtBlockCacheTables() { + DfsBlockCacheConfig cacheConfig = new DfsBlockCacheConfig(); + cacheConfig.setPackExtCacheConfigurations( + List.of(new DfsBlockCachePackExtConfig(EnumSet.of(PackExt.PACK), + new DfsBlockCacheConfig()))); + assertNotNull( + PackExtBlockCacheTable.fromBlockCacheConfigs(cacheConfig)); + } + + @Test + public void fromBlockCacheConfigs_noPackExtConfigurationGiven_packExtCacheConfigurationsIsEmpty_throws() { + DfsBlockCacheConfig cacheConfig = new DfsBlockCacheConfig(); + cacheConfig.setPackExtCacheConfigurations(List.of()); + assertThrows(IllegalArgumentException.class, + () -> PackExtBlockCacheTable + .fromBlockCacheConfigs(cacheConfig)); + } + + @Test + public void hasBlock0_packExtMapsToCacheTable_callsBitmapIndexCacheTable() { + DfsStreamKey streamKey = new TestKey(PackExt.BITMAP_INDEX); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.hasBlock0(any(DfsStreamKey.class))) + .thenReturn(true); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertTrue(tables.hasBlock0(streamKey)); + } + + @Test + public void hasBlock0_packExtDoesNotMapToCacheTable_callsDefaultCache() { + DfsStreamKey streamKey = new TestKey(PackExt.PACK); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.hasBlock0(any(DfsStreamKey.class))) + .thenReturn(true); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertTrue(tables.hasBlock0(streamKey)); + } + + @Test + public void getOrLoad_packExtMapsToCacheTable_callsBitmapIndexCacheTable() + throws Exception { + BlockBasedFile blockBasedFile = new BlockBasedFile(null, + mock(DfsPackDescription.class), PackExt.BITMAP_INDEX) { + }; + DfsBlock dfsBlock = mock(DfsBlock.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.getOrLoad(any(BlockBasedFile.class), + anyLong(), any(DfsReader.class), + any(DfsBlockCache.ReadableChannelSupplier.class))) + .thenReturn(mock(DfsBlock.class)); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.getOrLoad(any(BlockBasedFile.class), + anyLong(), any(DfsReader.class), + any(DfsBlockCache.ReadableChannelSupplier.class))) + .thenReturn(dfsBlock); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat( + tables.getOrLoad(blockBasedFile, 0, mock(DfsReader.class), + mock(DfsBlockCache.ReadableChannelSupplier.class)), + sameInstance(dfsBlock)); + } + + @Test + public void getOrLoad_packExtDoesNotMapToCacheTable_callsDefaultCache() + throws Exception { + BlockBasedFile blockBasedFile = new BlockBasedFile(null, + mock(DfsPackDescription.class), PackExt.PACK) { + }; + DfsBlock dfsBlock = mock(DfsBlock.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.getOrLoad(any(BlockBasedFile.class), + anyLong(), any(DfsReader.class), + any(DfsBlockCache.ReadableChannelSupplier.class))) + .thenReturn(dfsBlock); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.getOrLoad(any(BlockBasedFile.class), + anyLong(), any(DfsReader.class), + any(DfsBlockCache.ReadableChannelSupplier.class))) + .thenReturn(mock(DfsBlock.class)); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat( + tables.getOrLoad(blockBasedFile, 0, mock(DfsReader.class), + mock(DfsBlockCache.ReadableChannelSupplier.class)), + sameInstance(dfsBlock)); + } + + @Test + public void getOrLoadRef_packExtMapsToCacheTable_callsBitmapIndexCacheTable() + throws Exception { + Ref ref = mock(Ref.class); + DfsStreamKey dfsStreamKey = new TestKey(PackExt.BITMAP_INDEX); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.getOrLoadRef(any(DfsStreamKey.class), + anyLong(), any(RefLoader.class))).thenReturn(mock(Ref.class)); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.getOrLoadRef(any(DfsStreamKey.class), + anyLong(), any(RefLoader.class))).thenReturn(ref); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.getOrLoadRef(dfsStreamKey, 0, mock(RefLoader.class)), + sameInstance(ref)); + } + + @Test + public void getOrLoadRef_packExtDoesNotMapToCacheTable_callsDefaultCache() + throws Exception { + Ref ref = mock(Ref.class); + DfsStreamKey dfsStreamKey = new TestKey(PackExt.PACK); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.getOrLoadRef(any(DfsStreamKey.class), + anyLong(), any(RefLoader.class))).thenReturn(ref); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.getOrLoadRef(any(DfsStreamKey.class), + anyLong(), any(RefLoader.class))).thenReturn(mock(Ref.class)); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.getOrLoadRef(dfsStreamKey, 0, mock(RefLoader.class)), + sameInstance(ref)); + } + + @Test + public void putDfsBlock_packExtMapsToCacheTable_callsBitmapIndexCacheTable() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.BITMAP_INDEX); + DfsBlock dfsBlock = new DfsBlock(dfsStreamKey, 0, new byte[0]); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + tables.put(dfsBlock); + Mockito.verify(bitmapIndexCacheTable, times(1)).put(dfsBlock); + } + + @Test + public void putDfsBlock_packExtDoesNotMapToCacheTable_callsDefaultCache() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.PACK); + DfsBlock dfsBlock = new DfsBlock(dfsStreamKey, 0, new byte[0]); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + tables.put(dfsBlock); + Mockito.verify(defaultBlockCacheTable, times(1)).put(dfsBlock); + } + + @Test + public void putDfsStreamKey_packExtMapsToCacheTable_callsBitmapIndexCacheTable() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.BITMAP_INDEX); + Ref ref = mock(Ref.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.put(any(DfsStreamKey.class), anyLong(), + anyLong(), anyInt())).thenReturn(mock(Ref.class)); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.put(any(DfsStreamKey.class), anyLong(), + anyLong(), anyInt())).thenReturn(ref); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.put(dfsStreamKey, 0, 0, 0), sameInstance(ref)); + } + + @Test + public void putDfsStreamKey_packExtDoesNotMapToCacheTable_callsDefaultCache() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.PACK); + Ref ref = mock(Ref.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.put(any(DfsStreamKey.class), anyLong(), + anyLong(), anyInt())).thenReturn(ref); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.put(any(DfsStreamKey.class), anyLong(), + anyLong(), anyInt())).thenReturn(mock(Ref.class)); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.put(dfsStreamKey, 0, 0, 0), sameInstance(ref)); + } + + @Test + public void putRef_packExtMapsToCacheTable_callsBitmapIndexCacheTable() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.BITMAP_INDEX); + Ref ref = mock(Ref.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.putRef(any(DfsStreamKey.class), anyLong(), + anyInt())).thenReturn(mock(Ref.class)); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.putRef(any(DfsStreamKey.class), anyLong(), + anyInt())).thenReturn(ref); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.putRef(dfsStreamKey, 0, 0), sameInstance(ref)); + } + + @Test + public void putRef_packExtDoesNotMapToCacheTable_callsDefaultCache() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.PACK); + Ref ref = mock(Ref.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.putRef(any(DfsStreamKey.class), anyLong(), + anyInt())).thenReturn(ref); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.putRef(any(DfsStreamKey.class), anyLong(), + anyInt())).thenReturn(mock(Ref.class)); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.putRef(dfsStreamKey, 0, 0), sameInstance(ref)); + } + + @Test + public void contains_packExtMapsToCacheTable_callsBitmapIndexCacheTable() { + DfsStreamKey streamKey = new TestKey(PackExt.BITMAP_INDEX); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.contains(any(DfsStreamKey.class), anyLong())) + .thenReturn(true); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertTrue(tables.contains(streamKey, 0)); + } + + @Test + public void contains_packExtDoesNotMapToCacheTable_callsDefaultCache() { + DfsStreamKey streamKey = new TestKey(PackExt.PACK); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.contains(any(DfsStreamKey.class), + anyLong())).thenReturn(true); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertTrue(tables.contains(streamKey, 0)); + } + + @Test + public void get_packExtMapsToCacheTable_callsBitmapIndexCacheTable() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.BITMAP_INDEX); + Ref ref = mock(Ref.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.get(any(DfsStreamKey.class), anyLong())) + .thenReturn(mock(Ref.class)); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.get(any(DfsStreamKey.class), anyLong())) + .thenReturn(ref); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.get(dfsStreamKey, 0), sameInstance(ref)); + } + + @Test + public void get_packExtDoesNotMapToCacheTable_callsDefaultCache() { + DfsStreamKey dfsStreamKey = new TestKey(PackExt.PACK); + Ref ref = mock(Ref.class); + DfsBlockCacheTable defaultBlockCacheTable = mock( + DfsBlockCacheTable.class); + when(defaultBlockCacheTable.get(any(DfsStreamKey.class), anyLong())) + .thenReturn(ref); + DfsBlockCacheTable bitmapIndexCacheTable = mock( + DfsBlockCacheTable.class); + when(bitmapIndexCacheTable.get(any(DfsStreamKey.class), anyLong())) + .thenReturn(mock(Ref.class)); + + PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( + defaultBlockCacheTable, + Map.of(PackExt.BITMAP_INDEX, bitmapIndexCacheTable)); + + assertThat(tables.get(dfsStreamKey, 0), sameInstance(ref)); + } + + @Test + public void getBlockCacheStats_getCurrentSize_consolidatesAllTableCurrentSizes() { + long[] currentSizes = createEmptyStatsArray(); + + DfsBlockCacheStats packStats = new DfsBlockCacheStats(); + packStats.addToLiveBytes(new TestKey(PackExt.PACK), 5); + currentSizes[PackExt.PACK.getPosition()] = 5; + + DfsBlockCacheStats bitmapStats = new DfsBlockCacheStats(); + bitmapStats.addToLiveBytes(new TestKey(PackExt.BITMAP_INDEX), 6); + currentSizes[PackExt.BITMAP_INDEX.getPosition()] = 6; + + DfsBlockCacheStats indexStats = new DfsBlockCacheStats(); + indexStats.addToLiveBytes(new TestKey(PackExt.INDEX), 7); + currentSizes[PackExt.INDEX.getPosition()] = 7; + + PackExtBlockCacheTable tables = PackExtBlockCacheTable + .fromCacheTables(cacheTableWithStats(packStats), + Map.of(PackExt.BITMAP_INDEX, + cacheTableWithStats(bitmapStats), PackExt.INDEX, + cacheTableWithStats(indexStats))); + + assertArrayEquals(tables.getBlockCacheStats().getCurrentSize(), + currentSizes); + } + + @Test + public void getBlockCacheStats_GetHitCount_consolidatesAllTableHitCounts() { + long[] hitCounts = createEmptyStatsArray(); + + DfsBlockCacheStats packStats = new DfsBlockCacheStats(); + incrementCounter(5, + () -> packStats.incrementHit(new TestKey(PackExt.PACK))); + hitCounts[PackExt.PACK.getPosition()] = 5; + + DfsBlockCacheStats bitmapStats = new DfsBlockCacheStats(); + incrementCounter(6, () -> bitmapStats + .incrementHit(new TestKey(PackExt.BITMAP_INDEX))); + hitCounts[PackExt.BITMAP_INDEX.getPosition()] = 6; + + DfsBlockCacheStats indexStats = new DfsBlockCacheStats(); + incrementCounter(7, + () -> indexStats.incrementHit(new TestKey(PackExt.INDEX))); + hitCounts[PackExt.INDEX.getPosition()] = 7; + + PackExtBlockCacheTable tables = PackExtBlockCacheTable + .fromCacheTables(cacheTableWithStats(packStats), + Map.of(PackExt.BITMAP_INDEX, + cacheTableWithStats(bitmapStats), PackExt.INDEX, + cacheTableWithStats(indexStats))); + + assertArrayEquals(tables.getBlockCacheStats().getHitCount(), hitCounts); + } + + @Test + public void getBlockCacheStats_getMissCount_consolidatesAllTableMissCounts() { + long[] missCounts = createEmptyStatsArray(); + + DfsBlockCacheStats packStats = new DfsBlockCacheStats(); + incrementCounter(5, + () -> packStats.incrementMiss(new TestKey(PackExt.PACK))); + missCounts[PackExt.PACK.getPosition()] = 5; + + DfsBlockCacheStats bitmapStats = new DfsBlockCacheStats(); + incrementCounter(6, () -> bitmapStats + .incrementMiss(new TestKey(PackExt.BITMAP_INDEX))); + missCounts[PackExt.BITMAP_INDEX.getPosition()] = 6; + + DfsBlockCacheStats indexStats = new DfsBlockCacheStats(); + incrementCounter(7, + () -> indexStats.incrementMiss(new TestKey(PackExt.INDEX))); + missCounts[PackExt.INDEX.getPosition()] = 7; + + PackExtBlockCacheTable tables = PackExtBlockCacheTable + .fromCacheTables(cacheTableWithStats(packStats), + Map.of(PackExt.BITMAP_INDEX, + cacheTableWithStats(bitmapStats), PackExt.INDEX, + cacheTableWithStats(indexStats))); + + assertArrayEquals(tables.getBlockCacheStats().getMissCount(), + missCounts); + } + + @Test + public void getBlockCacheStats_getTotalRequestCount_consolidatesAllTableTotalRequestCounts() { + long[] totalRequestCounts = createEmptyStatsArray(); + + DfsBlockCacheStats packStats = new DfsBlockCacheStats(); + incrementCounter(5, () -> { + packStats.incrementHit(new TestKey(PackExt.PACK)); + packStats.incrementMiss(new TestKey(PackExt.PACK)); + }); + totalRequestCounts[PackExt.PACK.getPosition()] = 10; + + DfsBlockCacheStats bitmapStats = new DfsBlockCacheStats(); + incrementCounter(6, () -> { + bitmapStats.incrementHit(new TestKey(PackExt.BITMAP_INDEX)); + bitmapStats.incrementMiss(new TestKey(PackExt.BITMAP_INDEX)); + }); + totalRequestCounts[PackExt.BITMAP_INDEX.getPosition()] = 12; + + DfsBlockCacheStats indexStats = new DfsBlockCacheStats(); + incrementCounter(7, () -> { + indexStats.incrementHit(new TestKey(PackExt.INDEX)); + indexStats.incrementMiss(new TestKey(PackExt.INDEX)); + }); + totalRequestCounts[PackExt.INDEX.getPosition()] = 14; + + PackExtBlockCacheTable tables = PackExtBlockCacheTable + .fromCacheTables(cacheTableWithStats(packStats), + Map.of(PackExt.BITMAP_INDEX, + cacheTableWithStats(bitmapStats), PackExt.INDEX, + cacheTableWithStats(indexStats))); + + assertArrayEquals(tables.getBlockCacheStats().getTotalRequestCount(), + totalRequestCounts); + } + + @Test + public void getBlockCacheStats_getHitRatio_consolidatesAllTableHitRatios() { + long[] hitRatios = createEmptyStatsArray(); + + DfsBlockCacheStats packStats = new DfsBlockCacheStats(); + incrementCounter(5, + () -> packStats.incrementHit(new TestKey(PackExt.PACK))); + hitRatios[PackExt.PACK.getPosition()] = 100; + + DfsBlockCacheStats bitmapStats = new DfsBlockCacheStats(); + incrementCounter(6, () -> { + bitmapStats.incrementHit(new TestKey(PackExt.BITMAP_INDEX)); + bitmapStats.incrementMiss(new TestKey(PackExt.BITMAP_INDEX)); + }); + hitRatios[PackExt.BITMAP_INDEX.getPosition()] = 50; + + DfsBlockCacheStats indexStats = new DfsBlockCacheStats(); + incrementCounter(7, + () -> indexStats.incrementMiss(new TestKey(PackExt.INDEX))); + hitRatios[PackExt.INDEX.getPosition()] = 0; + + PackExtBlockCacheTable tables = PackExtBlockCacheTable + .fromCacheTables(cacheTableWithStats(packStats), + Map.of(PackExt.BITMAP_INDEX, + cacheTableWithStats(bitmapStats), PackExt.INDEX, + cacheTableWithStats(indexStats))); + + assertArrayEquals(tables.getBlockCacheStats().getHitRatio(), hitRatios); + } + + @Test + public void getBlockCacheStats_getEvictions_consolidatesAllTableEvictions() { + long[] evictions = createEmptyStatsArray(); + + DfsBlockCacheStats packStats = new DfsBlockCacheStats(); + incrementCounter(5, + () -> packStats.incrementEvict(new TestKey(PackExt.PACK))); + evictions[PackExt.PACK.getPosition()] = 5; + + DfsBlockCacheStats bitmapStats = new DfsBlockCacheStats(); + incrementCounter(6, () -> bitmapStats + .incrementEvict(new TestKey(PackExt.BITMAP_INDEX))); + evictions[PackExt.BITMAP_INDEX.getPosition()] = 6; + + DfsBlockCacheStats indexStats = new DfsBlockCacheStats(); + incrementCounter(7, + () -> indexStats.incrementEvict(new TestKey(PackExt.INDEX))); + evictions[PackExt.INDEX.getPosition()] = 7; + + PackExtBlockCacheTable tables = PackExtBlockCacheTable + .fromCacheTables(cacheTableWithStats(packStats), + Map.of(PackExt.BITMAP_INDEX, + cacheTableWithStats(bitmapStats), PackExt.INDEX, + cacheTableWithStats(indexStats))); + + assertArrayEquals(tables.getBlockCacheStats().getEvictions(), + evictions); + } + + private static void incrementCounter(int amount, Runnable fn) { + for (int i = 0; i < amount; i++) { + fn.run(); + } + } + + private static long[] createEmptyStatsArray() { + return new long[PackExt.values().length]; + } + + private static DfsBlockCacheTable cacheTableWithStats( + DfsBlockCacheStats dfsBlockCacheStats) { + DfsBlockCacheTable cacheTable = mock(DfsBlockCacheTable.class); + when(cacheTable.getBlockCacheStats()).thenReturn(dfsBlockCacheStats); + return cacheTable; + } + + private static class TestKey extends DfsStreamKey { + TestKey(PackExt packExt) { + super(0, packExt); + } + + @Override + public boolean equals(Object o) { + return false; + } + } +} diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 9d12facb33..a15fe1f372 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -285,6 +285,8 @@ DIRCUnrecognizedExtendedFlags=Unrecognized extended flags: {0} downloadCancelled=Download cancelled downloadCancelledDuringIndexing=Download cancelled during indexing duplicateAdvertisementsOf=duplicate advertisements of {0} +duplicateCacheTablesGiven=Duplicate cache tables given +duplicatePackExtensionsForCacheTables=Duplicate pack extension {0} in cache tables duplicatePackExtensionsSet=Attempting to configure duplicate pack extensions: {0}.{1}.{2} contains {3} duplicateRef=Duplicate ref: {0} duplicateRefAttribute=Duplicate ref attribute: {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 311d9c22aa..e31533aaa2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -315,6 +315,8 @@ public class JGitText extends TranslationBundle { /***/ public String downloadCancelled; /***/ public String downloadCancelledDuringIndexing; /***/ public String duplicateAdvertisementsOf; + /***/ public String duplicateCacheTablesGiven; + /***/ public String duplicatePackExtensionsForCacheTables; /***/ public String duplicatePackExtensionsSet; /***/ public String duplicateRef; /***/ public String duplicateRefAttribute; 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 fa86701de8..20f4666373 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 @@ -263,6 +263,21 @@ public class DfsBlockCacheConfig { return packExtCacheConfigurations; } + /** + * Set the list of pack ext cache configs. + * + * Made visible for testing. + * + * @param packExtCacheConfigurations + * the list of pack ext cache configs to set. + * @return {@code this} + */ + DfsBlockCacheConfig setPackExtCacheConfigurations( + List packExtCacheConfigurations) { + this.packExtCacheConfigurations = packExtCacheConfigurations; + return this; + } + /** * Update properties by setting fields from the configuration. *

@@ -435,7 +450,15 @@ public class DfsBlockCacheConfig { // Configuration for the cache instance. private final DfsBlockCacheConfig packExtCacheConfiguration; - private DfsBlockCachePackExtConfig(EnumSet packExts, + /** + * Made visible for testing. + * + * @param packExts + * Set of {@link PackExt}s associated to this cache config. + * @param packExtCacheConfiguration + * {@link DfsBlockCacheConfig} for this cache config. + */ + DfsBlockCachePackExtConfig(EnumSet packExts, DfsBlockCacheConfig packExtCacheConfiguration) { this.packExts = packExts; this.packExtCacheConfiguration = packExtCacheConfiguration; @@ -475,6 +498,5 @@ public class DfsBlockCacheConfig { return new DfsBlockCachePackExtConfig(EnumSet.copyOf(packExts), dfsBlockCacheConfig); } - } } \ No newline at end of file diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java new file mode 100644 index 0000000000..858f731b70 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java @@ -0,0 +1,289 @@ +/* + * Copyright (c) 2024, Google LLC and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.storage.dfs; + +import java.io.IOException; +import java.text.MessageFormat; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCache.ReadableChannelSupplier; +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCache.Ref; +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCache.RefLoader; +import org.eclipse.jgit.internal.storage.dfs.DfsBlockCacheConfig.DfsBlockCachePackExtConfig; +import org.eclipse.jgit.internal.storage.pack.PackExt; + +/** + * A table that holds multiple cache tables accessed by {@link PackExt} types. + * + *

+ * Allows the separation of entries from different {@link PackExt} types to + * limit churn in cache caused by entries of differing sizes. + *

+ * Separating these tables enables the fine-tuning of cache tables per extension + * type. + */ +class PackExtBlockCacheTable implements DfsBlockCacheTable { + private final DfsBlockCacheTable defaultBlockCacheTable; + + // Holds the unique tables backing the extBlockCacheTables values. + private final List blockCacheTableList; + + // Holds the mapping of PackExt to DfsBlockCacheTables. + // The relation between the size of extBlockCacheTables entries and + // blockCacheTableList entries is: + // blockCacheTableList.size() <= extBlockCacheTables.size() + private final Map extBlockCacheTables; + + /** + * Builds the PackExtBlockCacheTable from a list of + * {@link DfsBlockCachePackExtConfig}s. + * + * @param cacheConfig + * {@link DfsBlockCacheConfig} containing + * {@link DfsBlockCachePackExtConfig}s used to configure + * PackExtBlockCacheTable. The {@link DfsBlockCacheConfig} holds + * the configuration for the default cache table. + * @return the cache table built from the given configs. + * @throws IllegalArgumentException + * when no {@link DfsBlockCachePackExtConfig} exists in the + * {@link DfsBlockCacheConfig}. + */ + static PackExtBlockCacheTable fromBlockCacheConfigs( + DfsBlockCacheConfig cacheConfig) { + DfsBlockCacheTable defaultTable = new ClockBlockCacheTable(cacheConfig); + Map packExtBlockCacheTables = new HashMap<>(); + List packExtConfigs = cacheConfig + .getPackExtCacheConfigurations(); + if (packExtConfigs == null || packExtConfigs.size() == 0) { + throw new IllegalArgumentException( + JGitText.get().noPackExtConfigurationGiven); + } + for (DfsBlockCachePackExtConfig packExtCacheConfig : packExtConfigs) { + DfsBlockCacheTable table = new ClockBlockCacheTable( + packExtCacheConfig.getPackExtCacheConfiguration()); + for (PackExt packExt : packExtCacheConfig.getPackExts()) { + if (packExtBlockCacheTables.containsKey(packExt)) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().duplicatePackExtensionsForCacheTables, + packExt)); + } + packExtBlockCacheTables.put(packExt, table); + } + } + return fromCacheTables(defaultTable, packExtBlockCacheTables); + } + + /** + * Creates a new PackExtBlockCacheTable from the combination of a default + * {@link DfsBlockCacheTable} and a map of {@link PackExt}s to + * {@link DfsBlockCacheTable}s. + *

+ * This method allows for the PackExtBlockCacheTable to handle a mapping of + * {@link PackExt}s to arbitrarily defined {@link DfsBlockCacheTable} + * implementations. This is especially useful for users wishing to implement + * custom cache tables. + *

+ * This is currently made visible for testing. + * + * @param defaultBlockCacheTable + * the default table used when a handling a {@link PackExt} type + * that does not map to a {@link DfsBlockCacheTable} mapped by + * packExtsCacheTablePairs. + * @param packExtBlockCacheTables + * the mapping of {@link PackExt}s to + * {@link DfsBlockCacheTable}s. A single + * {@link DfsBlockCacheTable} can be defined for multiple + * {@link PackExt}s in a many-to-one relationship. + * @return the PackExtBlockCacheTable created from the + * defaultBlockCacheTable and packExtsCacheTablePairs mapping. + * @throws IllegalArgumentException + * when a {@link PackExt} is defined for multiple + * {@link DfsBlockCacheTable}s. + */ + static PackExtBlockCacheTable fromCacheTables( + DfsBlockCacheTable defaultBlockCacheTable, + Map packExtBlockCacheTables) { + Set blockCacheTables = new HashSet<>(); + blockCacheTables.add(defaultBlockCacheTable); + blockCacheTables.addAll(packExtBlockCacheTables.values()); + return new PackExtBlockCacheTable(defaultBlockCacheTable, + List.copyOf(blockCacheTables), packExtBlockCacheTables); + } + + private PackExtBlockCacheTable(DfsBlockCacheTable defaultBlockCacheTable, + List blockCacheTableList, + Map extBlockCacheTables) { + this.defaultBlockCacheTable = defaultBlockCacheTable; + this.blockCacheTableList = blockCacheTableList; + this.extBlockCacheTables = extBlockCacheTables; + } + + @Override + public boolean hasBlock0(DfsStreamKey key) { + return getTable(key).hasBlock0(key); + } + + @Override + public DfsBlock getOrLoad(BlockBasedFile file, long position, + DfsReader dfsReader, ReadableChannelSupplier fileChannel) + throws IOException { + return getTable(file.ext).getOrLoad(file, position, dfsReader, + fileChannel); + } + + @Override + public Ref getOrLoadRef(DfsStreamKey key, long position, + RefLoader loader) throws IOException { + return getTable(key).getOrLoadRef(key, position, loader); + } + + @Override + public void put(DfsBlock v) { + getTable(v.stream).put(v); + } + + @Override + public Ref put(DfsStreamKey key, long pos, long size, T v) { + return getTable(key).put(key, pos, size, v); + } + + @Override + public Ref putRef(DfsStreamKey key, long size, T v) { + return getTable(key).putRef(key, size, v); + } + + @Override + public boolean contains(DfsStreamKey key, long position) { + return getTable(key).contains(key, position); + } + + @Override + public T get(DfsStreamKey key, long position) { + return getTable(key).get(key, position); + } + + @Override + public BlockCacheStats getBlockCacheStats() { + return new CacheStats(blockCacheTableList.stream() + .map(DfsBlockCacheTable::getBlockCacheStats) + .collect(Collectors.toList())); + } + + private DfsBlockCacheTable getTable(PackExt packExt) { + return extBlockCacheTables.getOrDefault(packExt, + defaultBlockCacheTable); + } + + private DfsBlockCacheTable getTable(DfsStreamKey key) { + return extBlockCacheTables.getOrDefault(getPackExt(key), + defaultBlockCacheTable); + } + + private static PackExt getPackExt(DfsStreamKey key) { + return PackExt.values()[key.packExtPos]; + } + + private static class CacheStats implements BlockCacheStats { + private final List blockCacheStats; + + private CacheStats(List blockCacheStats) { + this.blockCacheStats = blockCacheStats; + } + + @Override + public long[] getCurrentSize() { + long[] sums = emptyPackStats(); + for (BlockCacheStats blockCacheStatsEntry : blockCacheStats) { + sums = add(sums, blockCacheStatsEntry.getCurrentSize()); + } + return sums; + } + + @Override + public long[] getHitCount() { + long[] sums = emptyPackStats(); + for (BlockCacheStats blockCacheStatsEntry : blockCacheStats) { + sums = add(sums, blockCacheStatsEntry.getHitCount()); + } + return sums; + } + + @Override + public long[] getMissCount() { + long[] sums = emptyPackStats(); + for (BlockCacheStats blockCacheStatsEntry : blockCacheStats) { + sums = add(sums, blockCacheStatsEntry.getMissCount()); + } + return sums; + } + + @Override + public long[] getTotalRequestCount() { + long[] sums = emptyPackStats(); + for (BlockCacheStats blockCacheStatsEntry : blockCacheStats) { + sums = add(sums, blockCacheStatsEntry.getTotalRequestCount()); + } + return sums; + } + + @Override + public long[] getHitRatio() { + long[] hit = getHitCount(); + long[] miss = getMissCount(); + long[] ratio = new long[Math.max(hit.length, miss.length)]; + for (int i = 0; i < ratio.length; i++) { + if (i >= hit.length) { + ratio[i] = 0; + } else if (i >= miss.length) { + ratio[i] = 100; + } else { + long total = hit[i] + miss[i]; + ratio[i] = total == 0 ? 0 : hit[i] * 100 / total; + } + } + return ratio; + } + + @Override + public long[] getEvictions() { + long[] sums = emptyPackStats(); + for (BlockCacheStats blockCacheStatsEntry : blockCacheStats) { + sums = add(sums, blockCacheStatsEntry.getEvictions()); + } + return sums; + } + + private static long[] emptyPackStats() { + return new long[PackExt.values().length]; + } + + private static long[] add(long[] first, long[] second) { + long[] sums = new long[Integer.max(first.length, second.length)]; + int i; + for (i = 0; i < Integer.min(first.length, second.length); i++) { + sums[i] = first[i] + second[i]; + } + for (int j = i; j < first.length; j++) { + sums[j] = first[i]; + } + for (int j = i; j < second.length; j++) { + sums[j] = second[i]; + } + return sums; + } + } +} -- 2.39.5