Browse Source

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 <matthias.sohn@sap.com>
stable-5.1
Matthias Sohn 5 years ago
parent
commit
709f83d489
  1. 68
      org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java
  2. 6
      org.eclipse.jgit/.settings/.api_filters
  3. 337
      org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java
  4. 6
      org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
  5. 32
      org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheConfig.java

68
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<TestObject> toLoad;
private WindowCacheConfig cfg;
private boolean useStrongRefs;
@Parameters(name = "useStrongRefs={0}")
public static Collection<Object[]> 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 {

6
org.eclipse.jgit/.settings/.api_filters

@ -67,6 +67,12 @@
<message_argument value="CONFIG_KEY_PACKED_GIT_OPENFILES"/>
</message_arguments>
</filter>
<filter id="1142947843">
<message_arguments>
<message_argument value="5.1.13"/>
<message_argument value="CONFIG_KEY_PACKED_GIT_USE_STRONGREFS"/>
</message_arguments>
</filter>
<filter id="1142947843">
<message_arguments>
<message_argument value="5.1.13"/>

337
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.
* <p>
* 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.
* <p>
* 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 <code>(PackFile,position)</code> tuple.</li>
* <li>For every <code>load()</code> invocation there is exactly one
* {@link #createRef(PackFile, long, ByteWindow)} invocation to wrap a
* SoftReference around the cached entity.</li>
* SoftReference or a StrongReference around the cached entity.</li>
* <li>For every Reference created by <code>createRef()</code> 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.</li>
* </ul>
* <p>
* 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.
* <p>
@ -390,8 +398,8 @@ public class WindowCache {
cache.removeAll(pack);
}
/** ReferenceQueue to cleanup released and garbage collected windows. */
private final ReferenceQueue<ByteWindow> 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<ByteWindow> createRef(PackFile p, long o, ByteWindow v) {
final PageRef<ByteWindow> 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<ByteWindow> 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<ByteWindow> 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<ByteWindow> 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 {
* <p>
* 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<ByteWindow> 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<ByteWindow> r) {
next = n;
ref = r;
}
final void kill() {
dead = true;
ref.enqueue();
ref.kill();
}
}
private static interface PageRef<T> {
/**
* 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 <code>null</code>.
*
* @return The object to which this reference refers, or
* <code>null</code> if this reference object has been cleared
*/
T get();
/**
* Kill this ref
*
* @return <code>true</code> if this reference object was successfully
* killed; <code>false</code> 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<ByteWindow> {
final PackFile pack;
private static class SoftRef extends SoftReference<ByteWindow>
implements PageRef<ByteWindow> {
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<ByteWindow> 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<ByteWindow> {
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<ByteWindow> r);
void gc();
}
private static class SoftCleanupQueue extends ReferenceQueue<ByteWindow>
implements CleanupQueue {
private final WindowCache wc;
SoftCleanupQueue(WindowCache cache) {
this.wc = cache;
}
@Override
public boolean enqueue(PageRef<ByteWindow> 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<PageRef<ByteWindow>> queue = new ConcurrentLinkedQueue<>();
StrongCleanupQueue(WindowCache wc) {
this.wc = wc;
}
@Override
public boolean enqueue(PageRef<ByteWindow> r) {
if (queue.contains(r)) {
return false;
}
return queue.add(r);
}
@Override
public void gc() {
PageRef<ByteWindow> 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 {

6
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";

32
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,

Loading…
Cancel
Save