From 2b9dd32a828577bc1026ad53ec4f735a9b219022 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 3 Jan 2020 00:52:37 +0100 Subject: Add config constants for WindowCache configuration options Change-Id: Icc5265f87ae58aa1e667ed1827075c4a30f75c32 Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/lib/ConfigConstants.java | 24 ++++++++++++++++ .../jgit/storage/file/WindowCacheConfig.java | 32 ++++++++++++++-------- 2 files changed, 44 insertions(+), 12 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 43776e0817..9d292f642c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -200,6 +200,30 @@ public final class ConfigConstants { /** The "streamFileThreshold" key */ public static final String CONFIG_KEY_STREAM_FILE_TRESHOLD = "streamFileThreshold"; + /** + * The "packedGitMmap" key + * @since 5.1.13 + */ + public static final String CONFIG_KEY_PACKED_GIT_MMAP = "packedgitmmap"; + + /** + * The "packedGitWindowSize" key + * @since 5.1.13 + */ + public static final String CONFIG_KEY_PACKED_GIT_WINDOWSIZE = "packedgitwindowsize"; + + /** + * The "packedGitLimit" key + * @since 5.1.13 + */ + public static final String CONFIG_KEY_PACKED_GIT_LIMIT = "packedgitlimit"; + + /** + * The "packedGitOpenFiles" key + * @since 5.1.13 + */ + public static final String CONFIG_KEY_PACKED_GIT_OPENFILES = "packedgitopenfiles"; + /** The "remote" key */ public static final String CONFIG_KEY_REMOTE = "remote"; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java index c2e6a42001..c04b677df1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java @@ -43,6 +43,14 @@ package org.eclipse.jgit.storage.file; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_CORE_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_DELTA_BASE_CACHE_LIMIT; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_LIMIT; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_MMAP; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_OPENFILES; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_WINDOWSIZE; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_STREAM_FILE_TRESHOLD; + import org.eclipse.jgit.internal.storage.file.WindowCache; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.storage.pack.PackConfig; @@ -227,20 +235,20 @@ public class WindowCacheConfig { * @since 3.0 */ public WindowCacheConfig fromConfig(Config rc) { - setPackedGitOpenFiles(rc.getInt( - "core", null, "packedgitopenfiles", getPackedGitOpenFiles())); //$NON-NLS-1$ //$NON-NLS-2$ - setPackedGitLimit(rc.getLong( - "core", null, "packedgitlimit", getPackedGitLimit())); //$NON-NLS-1$ //$NON-NLS-2$ - setPackedGitWindowSize(rc.getInt( - "core", null, "packedgitwindowsize", getPackedGitWindowSize())); //$NON-NLS-1$ //$NON-NLS-2$ - setPackedGitMMAP(rc.getBoolean( - "core", null, "packedgitmmap", isPackedGitMMAP())); //$NON-NLS-1$ //$NON-NLS-2$ - setDeltaBaseCacheLimit(rc.getInt( - "core", null, "deltabasecachelimit", getDeltaBaseCacheLimit())); //$NON-NLS-1$ //$NON-NLS-2$ + setPackedGitOpenFiles(rc.getInt(CONFIG_CORE_SECTION, null, + CONFIG_KEY_PACKED_GIT_OPENFILES, getPackedGitOpenFiles())); + setPackedGitLimit(rc.getLong(CONFIG_CORE_SECTION, null, + CONFIG_KEY_PACKED_GIT_LIMIT, getPackedGitLimit())); + setPackedGitWindowSize(rc.getInt(CONFIG_CORE_SECTION, null, + CONFIG_KEY_PACKED_GIT_WINDOWSIZE, getPackedGitWindowSize())); + setPackedGitMMAP(rc.getBoolean(CONFIG_CORE_SECTION, null, + CONFIG_KEY_PACKED_GIT_MMAP, isPackedGitMMAP())); + setDeltaBaseCacheLimit(rc.getInt(CONFIG_CORE_SECTION, null, + CONFIG_KEY_DELTA_BASE_CACHE_LIMIT, getDeltaBaseCacheLimit())); long maxMem = Runtime.getRuntime().maxMemory(); - long sft = rc.getLong( - "core", null, "streamfilethreshold", getStreamFileThreshold()); //$NON-NLS-1$ //$NON-NLS-2$ + long sft = rc.getLong(CONFIG_CORE_SECTION, null, + CONFIG_KEY_STREAM_FILE_TRESHOLD, getStreamFileThreshold()); sft = Math.min(sft, maxMem / 4); // don't use more than 1/4 of the heap sft = Math.min(sft, Integer.MAX_VALUE); // cannot exceed array length setStreamFileThreshold((int) sft); -- cgit v1.2.3 From 6185db3d776f1064af1972b4ba2175a917c35ab3 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Thu, 9 Jan 2020 10:23:12 +0100 Subject: Replace usage of ArrayIndexOutOfBoundsException in treewalk Using exceptions during normal operations - for example with the desire of expanding an array in the failure case - can have a severe performance impact. When exceptions are instantiated, a stack trace is collected. Generating stack trace can be expensive. Compared to that, checking an array for length - even if done many times - is cheap since this is a check that can run in just a handful of CPU cycles. Change-Id: Ifaf10623f6a876c9faecfa44654c9296315adfcb Signed-off-by: Patrick Hiesel Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java | 6 ++---- .../src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java | 9 ++++----- 2 files changed, 6 insertions(+), 9 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java index 335abe1b5b..a3c93cf87a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java @@ -239,12 +239,10 @@ public abstract class AbstractTreeIterator { path = p.path; pathOffset = p.pathLen + 1; - try { - path[pathOffset - 1] = '/'; - } catch (ArrayIndexOutOfBoundsException e) { + if (pathOffset > path.length) { growPath(p.pathLen); - path[pathOffset - 1] = '/'; } + path[pathOffset - 1] = '/'; } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java index 019968875d..b2d8fc3aa8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java @@ -387,14 +387,13 @@ public class CanonicalTreeParser extends AbstractTreeIterator { tmp = pathOffset; for (;; tmp++) { c = raw[ptr++]; - if (c == 0) + if (c == 0) { break; - try { - path[tmp] = c; - } catch (ArrayIndexOutOfBoundsException e) { + } + if (tmp >= path.length) { growPath(tmp); - path[tmp] = c; } + path[tmp] = c; } pathLen = tmp; nextPtr = ptr + OBJECT_ID_LENGTH; -- cgit v1.2.3 From 709f83d489b777c21ad5bbeeb3e8b1232b3f0ee5 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 13 Dec 2019 01:18:12 +0100 Subject: WindowCache: add option to use strong refs to reference ByteWindows Java GC evicts all SoftReferences when the used heap size comes close to the maximum heap size. This means peaks in heap memory consumption can flush the complete WindowCache which was observed to have negative impact on performance of upload-pack in Gerrit. Hence add a boolean option core.packedGitUseStrongRefs to allow using strong references to reference packfile pages cached in the WindowCache. If this option is set to true Java gc can no longer flush the WindowCache to free memory if the used heap comes close to the maximum heap size. On the other hand this provides more predictable performance. Bug: 553573 Change-Id: I9de406293087ab0fa61130c8e0829775762ece8d Signed-off-by: Matthias Sohn --- .../internal/storage/file/WindowCacheGetTest.java | 68 +++-- org.eclipse.jgit/.settings/.api_filters | 6 + .../jgit/internal/storage/file/WindowCache.java | 337 ++++++++++++++++++--- .../src/org/eclipse/jgit/lib/ConfigConstants.java | 6 + .../jgit/storage/file/WindowCacheConfig.java | 32 ++ 5 files changed, 379 insertions(+), 70 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java index f1a18b096c..a173532db1 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java @@ -53,6 +53,8 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.List; import org.eclipse.jgit.errors.CorruptObjectException; @@ -66,9 +68,25 @@ import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase; import org.eclipse.jgit.util.MutableInteger; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +@RunWith(Parameterized.class) public class WindowCacheGetTest extends SampleDataRepositoryTestCase { private List toLoad; + private WindowCacheConfig cfg; + private boolean useStrongRefs; + + @Parameters(name = "useStrongRefs={0}") + public static Collection data() { + return Arrays + .asList(new Object[][] { { Boolean.TRUE }, { Boolean.FALSE } }); + } + + public WindowCacheGetTest(Boolean useStrongRef) { + this.useStrongRefs = useStrongRef.booleanValue(); + } @Override @Before @@ -93,11 +111,12 @@ public class WindowCacheGetTest extends SampleDataRepositoryTestCase { } } assertEquals(96, toLoad.size()); + cfg = new WindowCacheConfig(); + cfg.setPackedGitUseStrongRefs(useStrongRefs); } @Test public void testCache_Defaults() throws IOException { - WindowCacheConfig cfg = new WindowCacheConfig(); cfg.install(); doCacheTests(); checkLimits(cfg); @@ -122,7 +141,6 @@ public class WindowCacheGetTest extends SampleDataRepositoryTestCase { @Test public void testCache_TooFewFiles() throws IOException { - final WindowCacheConfig cfg = new WindowCacheConfig(); cfg.setPackedGitOpenFiles(2); cfg.install(); doCacheTests(); @@ -131,7 +149,6 @@ public class WindowCacheGetTest extends SampleDataRepositoryTestCase { @Test public void testCache_TooSmallLimit() throws IOException { - final WindowCacheConfig cfg = new WindowCacheConfig(); cfg.setPackedGitWindowSize(4096); cfg.setPackedGitLimit(4096); cfg.install(); @@ -142,26 +159,31 @@ public class WindowCacheGetTest extends SampleDataRepositoryTestCase { private static void checkLimits(WindowCacheConfig cfg) { final WindowCache cache = WindowCache.getInstance(); WindowCacheStats s = cache.getStats(); - assertTrue(0 < s.getAverageLoadTime()); - assertTrue(0 < s.getOpenByteCount()); - assertTrue(0 < s.getOpenByteCount()); - assertTrue(0.0 < s.getAverageLoadTime()); - assertTrue(0 <= s.getEvictionCount()); - assertTrue(0 < s.getHitCount()); - assertTrue(0 < s.getHitRatio()); - assertTrue(1 > s.getHitRatio()); - assertTrue(0 < s.getLoadCount()); - assertTrue(0 <= s.getLoadFailureCount()); - assertTrue(0.0 <= s.getLoadFailureRatio()); - assertTrue(1 > s.getLoadFailureRatio()); - assertTrue(0 < s.getLoadSuccessCount()); - assertTrue(s.getOpenByteCount() <= cfg.getPackedGitLimit()); - assertTrue(s.getOpenFileCount() <= cfg.getPackedGitOpenFiles()); - assertTrue(0 <= s.getMissCount()); - assertTrue(0 <= s.getMissRatio()); - assertTrue(1 > s.getMissRatio()); - assertTrue(0 < s.getRequestCount()); - assertTrue(0 < s.getTotalLoadTime()); + assertTrue("average load time should be > 0", + 0 < s.getAverageLoadTime()); + assertTrue("open byte count should be > 0", 0 < s.getOpenByteCount()); + assertTrue("eviction count should be >= 0", 0 <= s.getEvictionCount()); + assertTrue("hit count should be > 0", 0 < s.getHitCount()); + assertTrue("hit ratio should be > 0", 0 < s.getHitRatio()); + assertTrue("hit ratio should be < 1", 1 > s.getHitRatio()); + assertTrue("load count should be > 0", 0 < s.getLoadCount()); + assertTrue("load failure count should be >= 0", + 0 <= s.getLoadFailureCount()); + assertTrue("load failure ratio should be >= 0", + 0.0 <= s.getLoadFailureRatio()); + assertTrue("load failure ratio should be < 1", + 1 > s.getLoadFailureRatio()); + assertTrue("load success count should be > 0", + 0 < s.getLoadSuccessCount()); + assertTrue("open byte count should be <= core.packedGitLimit", + s.getOpenByteCount() <= cfg.getPackedGitLimit()); + assertTrue("open file count should be <= core.packedGitOpenFiles", + s.getOpenFileCount() <= cfg.getPackedGitOpenFiles()); + assertTrue("miss success count should be >= 0", 0 <= s.getMissCount()); + assertTrue("miss ratio should be > 0", 0 <= s.getMissRatio()); + assertTrue("miss ratio should be < 1", 1 > s.getMissRatio()); + assertTrue("request count should be > 0", 0 < s.getRequestCount()); + assertTrue("total load time should be > 0", 0 < s.getTotalLoadTime()); } private void doCacheTests() throws IOException { diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 14c22e9372..90341ad9d3 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -67,6 +67,12 @@ + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java index 797507dd11..e8ab73538b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java @@ -48,6 +48,7 @@ import java.io.IOException; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; import java.util.Random; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.concurrent.atomic.LongAdder; @@ -85,9 +86,16 @@ import org.eclipse.jgit.util.Monitoring; * comprised of roughly 10% of the cache, and evicting the oldest accessed entry * within that window. *

- * Entities created by the cache are held under SoftReferences, permitting the + * Entities created by the cache are held under SoftReferences if option + * {@code core.packedGitUseStrongRefs} is set to {@code false} in the git config + * (this is the default) or by calling + * {@link WindowCacheConfig#setPackedGitUseStrongRefs(boolean)}, permitting the * Java runtime's garbage collector to evict entries when heap memory gets low. * Most JREs implement a loose least recently used algorithm for this eviction. + * When this option is set to {@code true} strong references are used which + * means that Java gc cannot evict the WindowCache to reclaim memory. On the + * other hand this provides more predictable performance since the cache isn't + * flushed when used heap comes close to the maximum heap size. *

* The internal hash table does not expand at runtime, instead it is fixed in * size at cache creation time. The internal lock table used to gate load @@ -104,19 +112,19 @@ import org.eclipse.jgit.util.Monitoring; * for a given (PackFile,position) tuple. *

  • For every load() invocation there is exactly one * {@link #createRef(PackFile, long, ByteWindow)} invocation to wrap a - * SoftReference around the cached entity.
  • + * SoftReference or a StrongReference around the cached entity. *
  • For every Reference created by createRef() there will be - * exactly one call to {@link #clear(Ref)} to cleanup any resources associated + * exactly one call to {@link #clear(PageRef)} to cleanup any resources associated * with the (now expired) cached entity.
  • * *

    * Therefore, it is safe to perform resource accounting increments during the * {@link #load(PackFile, long)} or * {@link #createRef(PackFile, long, ByteWindow)} methods, and matching - * decrements during {@link #clear(Ref)}. Implementors may need to override + * decrements during {@link #clear(PageRef)}. Implementors may need to override * {@link #createRef(PackFile, long, ByteWindow)} in order to embed additional * accounting information into an implementation specific - * {@link org.eclipse.jgit.internal.storage.file.WindowCache.Ref} subclass, as + * {@link org.eclipse.jgit.internal.storage.file.WindowCache.PageRef} subclass, as * the cached entity may have already been evicted by the JRE's garbage * collector. *

    @@ -390,8 +398,8 @@ public class WindowCache { cache.removeAll(pack); } - /** ReferenceQueue to cleanup released and garbage collected windows. */ - private final ReferenceQueue queue; + /** cleanup released and/or garbage collected windows. */ + private final CleanupQueue queue; /** Number of entries in {@link #table}. */ private final int tableSize; @@ -425,6 +433,8 @@ public class WindowCache { private final StatsRecorderImpl mbean; + private boolean useStrongRefs; + private WindowCache(WindowCacheConfig cfg) { tableSize = tableSize(cfg); final int lockCount = lockCount(cfg); @@ -433,7 +443,6 @@ public class WindowCache { if (lockCount < 1) throw new IllegalArgumentException(JGitText.get().lockCountMustBeGreaterOrEqual1); - queue = new ReferenceQueue<>(); clock = new AtomicLong(1); table = new AtomicReferenceArray<>(tableSize); locks = new Lock[lockCount]; @@ -455,6 +464,9 @@ public class WindowCache { mmap = cfg.isPackedGitMMAP(); windowSizeShift = bits(cfg.getPackedGitWindowSize()); windowSize = 1 << windowSizeShift; + useStrongRefs = cfg.isPackedGitUseStrongRefs(); + queue = useStrongRefs ? new StrongCleanupQueue(this) + : new SoftCleanupQueue(this); mbean = new StatsRecorderImpl(); statsRecorder = mbean; @@ -503,16 +515,18 @@ public class WindowCache { } } - private Ref createRef(PackFile p, long o, ByteWindow v) { - final Ref ref = new Ref(p, o, v, queue); - statsRecorder.recordOpenBytes(ref.size); + private PageRef createRef(PackFile p, long o, ByteWindow v) { + final PageRef ref = useStrongRefs + ? new StrongRef(p, o, v, queue) + : new SoftRef(p, o, v, (SoftCleanupQueue) queue); + statsRecorder.recordOpenBytes(ref.getSize()); return ref; } - private void clear(Ref ref) { - statsRecorder.recordOpenBytes(-ref.size); + private void clear(PageRef ref) { + statsRecorder.recordOpenBytes(-ref.getSize()); statsRecorder.recordEvictions(1); - close(ref.pack); + close(ref.getPack()); } private void close(PackFile pack) { @@ -577,7 +591,7 @@ public class WindowCache { } v = load(pack, position); - final Ref ref = createRef(pack, position, v); + final PageRef ref = createRef(pack, position, v); hit(ref); for (;;) { final Entry n = new Entry(clean(e2), ref); @@ -601,8 +615,8 @@ public class WindowCache { private ByteWindow scan(Entry n, PackFile pack, long position) { for (; n != null; n = n.next) { - final Ref r = n.ref; - if (r.pack == pack && r.position == position) { + final PageRef r = n.ref; + if (r.getPack() == pack && r.getPosition() == position) { final ByteWindow v = r.get(); if (v != null) { hit(r); @@ -615,7 +629,7 @@ public class WindowCache { return null; } - private void hit(Ref r) { + private void hit(PageRef r) { // We don't need to be 100% accurate here. Its sufficient that at least // one thread performs the increment. Any other concurrent access at // exactly the same time can simply use the same clock value. @@ -625,7 +639,7 @@ public class WindowCache { // final long c = clock.get(); clock.compareAndSet(c, c + 1); - r.lastAccess = c; + r.setLastAccess(c); } private void evict() { @@ -639,7 +653,8 @@ public class WindowCache { for (Entry e = table.get(ptr); e != null; e = e.next) { if (e.dead) continue; - if (old == null || e.ref.lastAccess < old.ref.lastAccess) { + if (old == null || e.ref.getLastAccess() < old.ref + .getLastAccess()) { old = e; slot = ptr; } @@ -659,7 +674,7 @@ public class WindowCache { *

    * This is a last-ditch effort to clear out the cache, such as before it * gets replaced by another cache that is configured differently. This - * method tries to force every cached entry through {@link #clear(Ref)} to + * method tries to force every cached entry through {@link #clear(PageRef)} to * ensure that resources are correctly accounted for and cleaned up by the * subclass. A concurrent reader loading entries while this method is * running may cause resource accounting failures. @@ -692,7 +707,7 @@ public class WindowCache { final Entry e1 = table.get(s); boolean hasDead = false; for (Entry e = e1; e != null; e = e.next) { - if (e.ref.pack == pack) { + if (e.ref.getPack() == pack) { e.kill(); hasDead = true; } else if (e.dead) @@ -705,20 +720,7 @@ public class WindowCache { } private void gc() { - Ref r; - while ((r = (Ref) queue.poll()) != null) { - clear(r); - - final int s = slot(r.pack, r.position); - final Entry e1 = table.get(s); - for (Entry n = e1; n != null; n = n.next) { - if (n.ref == r) { - n.dead = true; - table.compareAndSet(s, e1, clean(e1)); - break; - } - } - } + queue.gc(); } private int slot(PackFile pack, long position) { @@ -731,7 +733,7 @@ public class WindowCache { private static Entry clean(Entry top) { while (top != null && top.dead) { - top.ref.enqueue(); + top.ref.kill(); top = top.next; } if (top == null) @@ -745,7 +747,7 @@ public class WindowCache { final Entry next; /** The referenced object. */ - final Ref ref; + final PageRef ref; /** * Marked true when ref.get() returns null and the ref is dead. @@ -756,34 +758,275 @@ public class WindowCache { */ volatile boolean dead; - Entry(Entry n, Ref r) { + Entry(Entry n, PageRef r) { next = n; ref = r; } final void kill() { dead = true; - ref.enqueue(); + ref.kill(); } } + private static interface PageRef { + /** + * Returns this reference object's referent. If this reference object + * has been cleared, either by the program or by the garbage collector, + * then this method returns null. + * + * @return The object to which this reference refers, or + * null if this reference object has been cleared + */ + T get(); + + /** + * Kill this ref + * + * @return true if this reference object was successfully + * killed; false if it was already killed + */ + boolean kill(); + + /** + * Get the packfile the referenced cache page is allocated for + * + * @return the packfile the referenced cache page is allocated for + */ + PackFile getPack(); + + /** + * Get the position of the referenced cache page in the packfile + * + * @return the position of the referenced cache page in the packfile + */ + long getPosition(); + + /** + * Get size of cache page + * + * @return size of cache page + */ + int getSize(); + + /** + * Get pseudo time of last access to this cache page + * + * @return pseudo time of last access to this cache page + */ + long getLastAccess(); + + /** + * Set pseudo time of last access to this cache page + * + * @param time + * pseudo time of last access to this cache page + */ + void setLastAccess(long time); + + /** + * Whether this is a strong reference. + * @return {@code true} if this is a strong reference + */ + boolean isStrongRef(); + } + /** A soft reference wrapped around a cached object. */ - private static class Ref extends SoftReference { - final PackFile pack; + private static class SoftRef extends SoftReference + implements PageRef { + private final PackFile pack; - final long position; + private final long position; - final int size; + private final int size; - long lastAccess; + private long lastAccess; - protected Ref(final PackFile pack, final long position, - final ByteWindow v, final ReferenceQueue queue) { + protected SoftRef(final PackFile pack, final long position, + final ByteWindow v, final SoftCleanupQueue queue) { super(v, queue); this.pack = pack; this.position = position; this.size = v.size(); } + + @Override + public PackFile getPack() { + return pack; + } + + @Override + public long getPosition() { + return position; + } + + @Override + public int getSize() { + return size; + } + + @Override + public long getLastAccess() { + return lastAccess; + } + + @Override + public void setLastAccess(long time) { + this.lastAccess = time; + } + + @Override + public boolean kill() { + return enqueue(); + } + + @Override + public boolean isStrongRef() { + return false; + } + } + + /** A strong reference wrapped around a cached object. */ + private static class StrongRef implements PageRef { + private ByteWindow referent; + + private final PackFile pack; + + private final long position; + + private final int size; + + private long lastAccess; + + private CleanupQueue queue; + + protected StrongRef(final PackFile pack, final long position, + final ByteWindow v, final CleanupQueue queue) { + this.pack = pack; + this.position = position; + this.referent = v; + this.size = v.size(); + this.queue = queue; + } + + @Override + public PackFile getPack() { + return pack; + } + + @Override + public long getPosition() { + return position; + } + + @Override + public int getSize() { + return size; + } + + @Override + public long getLastAccess() { + return lastAccess; + } + + @Override + public void setLastAccess(long time) { + this.lastAccess = time; + } + + @Override + public ByteWindow get() { + return referent; + } + + @Override + public boolean kill() { + if (referent == null) { + return false; + } + referent = null; + return queue.enqueue(this); + } + + @Override + public boolean isStrongRef() { + return true; + } + } + + private static interface CleanupQueue { + boolean enqueue(PageRef r); + void gc(); + } + + private static class SoftCleanupQueue extends ReferenceQueue + implements CleanupQueue { + private final WindowCache wc; + + SoftCleanupQueue(WindowCache cache) { + this.wc = cache; + } + + @Override + public boolean enqueue(PageRef r) { + // no need to explicitly add soft references which are enqueued by + // the JVM + return false; + } + + @Override + public void gc() { + SoftRef r; + while ((r = (SoftRef) poll()) != null) { + wc.clear(r); + + final int s = wc.slot(r.getPack(), r.getPosition()); + final Entry e1 = wc.table.get(s); + for (Entry n = e1; n != null; n = n.next) { + if (n.ref == r) { + n.dead = true; + wc.table.compareAndSet(s, e1, clean(e1)); + break; + } + } + } + } + } + + private static class StrongCleanupQueue implements CleanupQueue { + private final WindowCache wc; + + private final ConcurrentLinkedQueue> queue = new ConcurrentLinkedQueue<>(); + + StrongCleanupQueue(WindowCache wc) { + this.wc = wc; + } + + @Override + public boolean enqueue(PageRef r) { + if (queue.contains(r)) { + return false; + } + return queue.add(r); + } + + @Override + public void gc() { + PageRef r; + while ((r = queue.poll()) != null) { + wc.clear(r); + + final int s = wc.slot(r.getPack(), r.getPosition()); + final Entry e1 = wc.table.get(s); + for (Entry n = e1; n != null; n = n.next) { + if (n.ref == r) { + n.dead = true; + wc.table.compareAndSet(s, e1, clean(e1)); + break; + } + } + } + } } private static final class Lock { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 9d292f642c..003ce238a0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -224,6 +224,12 @@ public final class ConfigConstants { */ public static final String CONFIG_KEY_PACKED_GIT_OPENFILES = "packedgitopenfiles"; + /** + * The "packedGitUseStrongRefs" key + * @since 5.1.13 + */ + public static final String CONFIG_KEY_PACKED_GIT_USE_STRONGREFS = "packedgitusestrongrefs"; + /** The "remote" key */ public static final String CONFIG_KEY_REMOTE = "remote"; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java index c04b677df1..f066ee18c2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java @@ -50,6 +50,7 @@ import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_MMAP; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_OPENFILES; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_WINDOWSIZE; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_STREAM_FILE_TRESHOLD; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACKED_GIT_USE_STRONGREFS; import org.eclipse.jgit.internal.storage.file.WindowCache; import org.eclipse.jgit.lib.Config; @@ -69,6 +70,8 @@ public class WindowCacheConfig { private long packedGitLimit; + private boolean useStrongRefs; + private int packedGitWindowSize; private boolean packedGitMMAP; @@ -83,6 +86,7 @@ public class WindowCacheConfig { public WindowCacheConfig() { packedGitOpenFiles = 128; packedGitLimit = 10 * MB; + useStrongRefs = false; packedGitWindowSize = 8 * KB; packedGitMMAP = false; deltaBaseCacheLimit = 10 * MB; @@ -133,6 +137,31 @@ public class WindowCacheConfig { packedGitLimit = newLimit; } + /** + * Get whether the window cache should use strong references or + * SoftReferences + * + * @return {@code true} if the window cache should use strong references, + * otherwise it will use {@link java.lang.ref.SoftReference}s + * @since 5.1.13 + */ + public boolean isPackedGitUseStrongRefs() { + return useStrongRefs; + } + + /** + * Set if the cache should use strong refs or soft refs + * + * @param useStrongRefs + * if @{code true} the cache strongly references cache pages + * otherwise it uses {@link java.lang.ref.SoftReference}s which + * can be evicted by the Java gc if heap is almost full + * @since 5.1.13 + */ + public void setPackedGitUseStrongRefs(boolean useStrongRefs) { + this.useStrongRefs = useStrongRefs; + } + /** * Get size in bytes of a single window mapped or read in from the pack * file. @@ -235,6 +264,9 @@ public class WindowCacheConfig { * @since 3.0 */ public WindowCacheConfig fromConfig(Config rc) { + setPackedGitUseStrongRefs(rc.getBoolean(CONFIG_CORE_SECTION, + CONFIG_KEY_PACKED_GIT_USE_STRONGREFS, + isPackedGitUseStrongRefs())); setPackedGitOpenFiles(rc.getInt(CONFIG_CORE_SECTION, null, CONFIG_KEY_PACKED_GIT_OPENFILES, getPackedGitOpenFiles())); setPackedGitLimit(rc.getLong(CONFIG_CORE_SECTION, null, -- cgit v1.2.3 From 1f9c717ff709e26d44090bc1b71be7c4c2b2a0fd Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 3 Jan 2020 18:16:42 +0100 Subject: WindowCache: add metric for cached bytes per repository Since ObjectDatabase and PackFile don't know their repository use the packfile's grand-grand-parent directory as an identifier for the repository the packfile resides in. Remove metric for a repository if the number of cached bytes for the repository drops to 0 in order to ensure the map of cached bytes per repository doesn't contain repositories which have no data cached in the WindowCache. Change-Id: I969ab8029db0a292e7585cbb36ca0baa797da20b Signed-off-by: Matthias Sohn --- .../jgit/internal/storage/file/WindowCache.java | 53 ++++++++++++++++++---- .../jgit/storage/file/WindowCacheStats.java | 9 ++++ 2 files changed, 52 insertions(+), 10 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java index e8ab73538b..3b4f662b90 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java @@ -47,12 +47,16 @@ package org.eclipse.jgit.internal.storage.file; import java.io.IOException; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; +import java.util.Collections; +import java.util.Map; import java.util.Random; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.concurrent.atomic.LongAdder; import java.util.concurrent.locks.ReentrantLock; +import java.util.stream.Collectors; import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.internal.JGitText; @@ -184,18 +188,21 @@ public class WindowCache { /** * Record files opened by cache * - * @param count + * @param delta * delta of number of files opened by cache */ - void recordOpenFiles(int count); + void recordOpenFiles(int delta); /** * Record cached bytes * - * @param count + * @param pack + * pack file the bytes are read from + * + * @param delta * delta of cached bytes */ - void recordOpenBytes(int count); + void recordOpenBytes(PackFile pack, int delta); /** * Returns a snapshot of this recorder's stats. Note that this may be an @@ -217,6 +224,7 @@ public class WindowCache { private final LongAdder evictionCount; private final LongAdder openFileCount; private final LongAdder openByteCount; + private final Map openByteCountPerRepository; /** * Constructs an instance with all counts initialized to zero. @@ -230,6 +238,7 @@ public class WindowCache { evictionCount = new LongAdder(); openFileCount = new LongAdder(); openByteCount = new LongAdder(); + openByteCountPerRepository = new ConcurrentHashMap<>(); } @Override @@ -260,13 +269,28 @@ public class WindowCache { } @Override - public void recordOpenFiles(int count) { - openFileCount.add(count); + public void recordOpenFiles(int delta) { + openFileCount.add(delta); } @Override - public void recordOpenBytes(int count) { - openByteCount.add(count); + public void recordOpenBytes(PackFile pack, int delta) { + openByteCount.add(delta); + String repositoryId = repositoryId(pack); + LongAdder la = openByteCountPerRepository + .computeIfAbsent(repositoryId, k -> new LongAdder()); + la.add(delta); + if (delta < 0) { + openByteCountPerRepository.computeIfPresent(repositoryId, + (k, v) -> v.longValue() == 0 ? null : v); + } + } + + private static String repositoryId(PackFile pack) { + // use repository's gitdir since packfile doesn't know its + // repository + return pack.getPackFile().getParentFile().getParentFile() + .getParent(); } @Override @@ -323,6 +347,15 @@ public class WindowCache { totalLoadTime.reset(); evictionCount.reset(); } + + @Override + public Map getOpenByteCountPerRepository() { + return Collections.unmodifiableMap( + openByteCountPerRepository.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, + e -> Long.valueOf(e.getValue().sum()), + (u, v) -> v))); + } } private static final int bits(int newSize) { @@ -519,12 +552,12 @@ public class WindowCache { final PageRef ref = useStrongRefs ? new StrongRef(p, o, v, queue) : new SoftRef(p, o, v, (SoftCleanupQueue) queue); - statsRecorder.recordOpenBytes(ref.getSize()); + statsRecorder.recordOpenBytes(ref.getPack(), ref.getSize()); return ref; } private void clear(PageRef ref) { - statsRecorder.recordOpenBytes(-ref.getSize()); + statsRecorder.recordOpenBytes(ref.getPack(), -ref.getSize()); statsRecorder.recordEvictions(1); close(ref.getPack()); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java index b7f6394df6..65f8dae342 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java @@ -42,6 +42,8 @@ */ package org.eclipse.jgit.storage.file; +import java.util.Map; + import javax.management.MXBean; import org.eclipse.jgit.internal.storage.file.WindowCache; @@ -225,6 +227,13 @@ public interface WindowCacheStats { */ long getOpenByteCount(); + /** + * Number of bytes cached per repository + * + * @return number of bytes cached per repository + */ + Map getOpenByteCountPerRepository(); + /** * Reset counters. Does not reset open bytes and open files counters. */ -- cgit v1.2.3