Time based eviction strategy for repository cache

When Repository.close() decrements the useCount to 0 currently the cache
immediately evicts the repository from WindowCache and RepositoryCache.
This leads to I/O overhead on busy repositories because pack files and
references are inserted and deleted from the cache frequently.

This commit defers the eviction of a repository from the caches until
last use of the repository is older than time to live. The eviction is
handled by a background task running periodically.

Add two new configuration parameters:
* core.repositoryCacheExpireAfter: cache entries are evicted if the
cache entry wasn't accessed longer than this time in milliseconds
* core.repositoryCacheCleanupDelay: defines the interval in milliseconds
for running a background task evicting expired cache entries. If set to
-1 the delay is set to min(repositoryCacheExpireAfter, 10 minutes). If
set to 0 the time based eviction is switched off and no background task
is started. If time based eviction is switched off the JVM can still
evict cache entries if heap memory is running low.

Change-Id: I4a0214ad8b4a193985dda6a0ade63b70bdb948d7
Also-by: Matthias Sohn <matthias.sohn@sap.com>
Also-by: Hugo Arès <hugo.ares@ericsson.com>
Also-by: Sasa Zivkov <sasa.zivkov@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2016 Ericsson
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.lib;
+
+import static org.eclipse.jgit.lib.RepositoryCacheConfig.AUTO_CLEANUP_DELAY;
+import static org.eclipse.jgit.lib.RepositoryCacheConfig.NO_CLEANUP;
+import static org.junit.Assert.assertEquals;
+
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.junit.Before;
+import org.junit.Test;
+
+public class RepositoryCacheConfigTest {
+
+	private RepositoryCacheConfig config;
+
+	@Before
+	public void setUp() {
+		config = new RepositoryCacheConfig();
+	}
+
+	@Test
+	public void testDefaultValues() {
+		assertEquals(TimeUnit.HOURS.toMillis(1), config.getExpireAfter());
+		assertEquals(config.getExpireAfter() / 10, config.getCleanupDelay());
+	}
+
+	@Test
+	public void testCleanupDelay() {
+		config.setCleanupDelay(TimeUnit.HOURS.toMillis(1));
+		assertEquals(TimeUnit.HOURS.toMillis(1), config.getCleanupDelay());
+	}
+
+	@Test
+	public void testAutoCleanupDelay() {
+		config.setExpireAfter(TimeUnit.MINUTES.toMillis(20));
+		config.setCleanupDelay(AUTO_CLEANUP_DELAY);
+		assertEquals(TimeUnit.MINUTES.toMillis(20), config.getExpireAfter());
+		assertEquals(config.getExpireAfter() / 10, config.getCleanupDelay());
+	}
+
+	@Test
+	public void testAutoCleanupDelayShouldBeMax10minutes() {
+		config.setExpireAfter(TimeUnit.HOURS.toMillis(10));
+		assertEquals(TimeUnit.HOURS.toMillis(10), config.getExpireAfter());
+		assertEquals(TimeUnit.MINUTES.toMillis(10), config.getCleanupDelay());
+	}
+
+	@Test
+	public void testDisabledCleanupDelay() {
+		config.setCleanupDelay(NO_CLEANUP);
+		assertEquals(NO_CLEANUP, config.getCleanupDelay());
+	}
+
+	@Test
+	public void testFromConfig() throws ConfigInvalidException {
+		Config otherConfig = new Config();
+		otherConfig.fromText("[core]\nrepositoryCacheExpireAfter=1000\n"
+				+ "repositoryCacheCleanupDelay=500");
+		config.fromConfig(otherConfig);
+		assertEquals(1000, config.getExpireAfter());
+		assertEquals(500, config.getCleanupDelay());
+	}
+}

org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java

@@ -195,17 +195,81 @@ public class RepositoryCacheTest extends RepositoryTestCase {
 		assertEquals(0, ((Repository) db).useCnt.get());
 	}
 
-	public void testRepositoryUnregisteringWhenClosing() throws Exception {
+	@Test
+	public void testRepositoryNotUnregisteringWhenClosing() throws Exception {
 		FileKey loc = FileKey.exact(db.getDirectory(), db.getFS());
 		Repository d2 = RepositoryCache.open(loc);
 		assertEquals(1, d2.useCnt.get());
 		assertThat(RepositoryCache.getRegisteredKeys(),
 				hasItem(FileKey.exact(db.getDirectory(), db.getFS())));
 		assertEquals(1, RepositoryCache.getRegisteredKeys().size());
-
 		d2.close();
-
 		assertEquals(0, d2.useCnt.get());
-		assertEquals(0, RepositoryCache.getRegisteredKeys().size());
+		assertEquals(1, RepositoryCache.getRegisteredKeys().size());
+		assertTrue(RepositoryCache.isCached(d2));
+	}
+
+	@Test
+	public void testRepositoryUnregisteringWhenExpired() throws Exception {
+		Repository repoA = createBareRepository();
+		Repository repoB = createBareRepository();
+		Repository repoC = createBareRepository();
+		RepositoryCache.register(repoA);
+		RepositoryCache.register(repoB);
+		RepositoryCache.register(repoC);
+
+		assertEquals(3, RepositoryCache.getRegisteredKeys().size());
+		assertTrue(RepositoryCache.isCached(repoA));
+		assertTrue(RepositoryCache.isCached(repoB));
+		assertTrue(RepositoryCache.isCached(repoC));
+
+		// fake that repoA was closed more than 1 hour ago (default expiration
+		// time)
+		repoA.close();
+		repoA.closedAt.set(System.currentTimeMillis() - 65 * 60 * 1000);
+		// close repoB but this one will not be expired
+		repoB.close();
+
+		assertEquals(3, RepositoryCache.getRegisteredKeys().size());
+		assertTrue(RepositoryCache.isCached(repoA));
+		assertTrue(RepositoryCache.isCached(repoB));
+		assertTrue(RepositoryCache.isCached(repoC));
+
+		RepositoryCache.clearExpired();
+
+		assertEquals(2, RepositoryCache.getRegisteredKeys().size());
+		assertFalse(RepositoryCache.isCached(repoA));
+		assertTrue(RepositoryCache.isCached(repoB));
+		assertTrue(RepositoryCache.isCached(repoC));
+	}
+
+	@Test
+	public void testReconfigure() throws InterruptedException {
+		RepositoryCache.register(db);
+		assertTrue(RepositoryCache.isCached(db));
+		db.close();
+		assertTrue(RepositoryCache.isCached(db));
+
+		// Actually, we would only need to validate that
+		// WorkQueue.getExecutor().scheduleWithFixedDelay is called with proper
+		// values but since we do not have a mock library, we test
+		// reconfiguration from a black box perspective. I.e. reconfigure
+		// expireAfter and cleanupDelay to 1 ms and wait until the Repository
+		// is evicted to prove that reconfiguration worked.
+		RepositoryCacheConfig config = new RepositoryCacheConfig();
+		config.setExpireAfter(1);
+		config.setCleanupDelay(1);
+		config.install();
+
+		// Instead of using a fixed waiting time, start with small and increase:
+		// sleep 1, 2, 4, 8, 16, ..., 1024 ms
+		// This wait will time out after 2048 ms
+		for (int i = 0; i <= 10; i++) {
+			Thread.sleep(1 << i);
+			if (!RepositoryCache.isCached(db)) {
+				return;
+			}
+		}
+		fail("Repository should have been evicted from cache");
 	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

@@ -63,6 +63,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
 
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.annotations.Nullable;
@@ -113,6 +114,8 @@ public abstract class Repository implements AutoCloseable {
 	/** Use counter */
 	final AtomicInteger useCnt = new AtomicInteger(1);
 
+	final AtomicLong closedAt = new AtomicLong();
+
 	/** Metadata directory holding the repository's critical files. */
 	private final File gitDir;
 
@@ -864,8 +867,11 @@ public abstract class Repository implements AutoCloseable {
 	/** Decrement the use count, and maybe close resources. */
 	public void close() {
 		if (useCnt.decrementAndGet() == 0) {
-			doClose();
+			if (RepositoryCache.isCached(this)) {
-			RepositoryCache.unregister(this);
+				closedAt.set(System.currentTimeMillis());
+			} else {
+				doClose();
+			}
 		}
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java

@@ -52,17 +52,26 @@ import java.util.Collection;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.RawParseUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** Cache of active {@link Repository} instances. */
 public class RepositoryCache {
 	private static final RepositoryCache cache = new RepositoryCache();
 
+	private final static Logger LOG = LoggerFactory
+			.getLogger(RepositoryCache.class);
+
 	/**
 	 * Open an existing repository, reusing a cached instance if possible.
 	 * <p>
@@ -138,10 +147,10 @@ public class RepositoryCache {
 	 * @param db
 	 *            repository to unregister.
 	 */
-	public static void close(final Repository db) {
+	public static void close(@NonNull final Repository db) {
 		if (db.getDirectory() != null) {
 			FileKey key = FileKey.exact(db.getDirectory(), db.getFS());
-			cache.unregisterAndCloseRepository(key);
+			cache.unregisterAndCloseRepository(key, db);
 		}
 	}
 
@@ -187,20 +196,69 @@ public class RepositoryCache {
 		return cache.getKeys();
 	}
 
+	static boolean isCached(@NonNull Repository repo) {
+		File gitDir = repo.getDirectory();
+		if (gitDir == null) {
+			return false;
+		}
+		FileKey key = new FileKey(gitDir, repo.getFS());
+		Reference<Repository> repoRef = cache.cacheMap.get(key);
+		return repoRef != null && repoRef.get() == repo;
+	}
+
 	/** Unregister all repositories from the cache. */
 	public static void clear() {
 		cache.clearAll();
 	}
 
+	static void clearExpired() {
+		cache.clearAllExpired();
+	}
+
+	static void reconfigure(RepositoryCacheConfig repositoryCacheConfig) {
+		cache.configureEviction(repositoryCacheConfig);
+	}
+
 	private final ConcurrentHashMap<Key, Reference<Repository>> cacheMap;
 
 	private final Lock[] openLocks;
 
+	private ScheduledFuture<?> cleanupTask;
+
+	private volatile long expireAfter;
+
 	private RepositoryCache() {
 		cacheMap = new ConcurrentHashMap<Key, Reference<Repository>>();
 		openLocks = new Lock[4];
-		for (int i = 0; i < openLocks.length; i++)
+		for (int i = 0; i < openLocks.length; i++) {
 			openLocks[i] = new Lock();
+		}
+		configureEviction(new RepositoryCacheConfig());
+	}
+
+	private void configureEviction(
+			RepositoryCacheConfig repositoryCacheConfig) {
+		expireAfter = repositoryCacheConfig.getExpireAfter();
+		ScheduledThreadPoolExecutor scheduler = WorkQueue.getExecutor();
+		synchronized (scheduler) {
+			if (cleanupTask != null) {
+				cleanupTask.cancel(false);
+			}
+			long delay = repositoryCacheConfig.getCleanupDelay();
+			if (delay == RepositoryCacheConfig.NO_CLEANUP) {
+				return;
+			}
+			cleanupTask = scheduler.scheduleWithFixedDelay(new Runnable() {
+				@Override
+				public void run() {
+					try {
+						cache.clearAllExpired();
+					} catch (Throwable e) {
+						LOG.error(e.getMessage(), e);
+					}
+				}
+			}, delay, delay, TimeUnit.MILLISECONDS);
+		}
 	}
 
 	@SuppressWarnings("resource")
@@ -239,10 +297,20 @@ public class RepositoryCache {
 		return oldRef != null ? oldRef.get() : null;
 	}
 
-	private void unregisterAndCloseRepository(final Key location) {
+	private boolean isExpired(Repository db) {
-		Repository oldDb = unregisterRepository(location);
+		return db != null && db.useCnt.get() == 0
-		if (oldDb != null) {
+			&& (System.currentTimeMillis() - db.closedAt.get() > expireAfter);
-			oldDb.close();
+	}
+
+	private void unregisterAndCloseRepository(final Key location,
+			Repository db) {
+		synchronized (lockFor(location)) {
+			if (isExpired(db)) {
+				Repository oldDb = unregisterRepository(location);
+				if (oldDb != null) {
+					oldDb.close();
+				}
+			}
 		}
 	}
 
@@ -250,6 +318,15 @@ public class RepositoryCache {
 		return new ArrayList<Key>(cacheMap.keySet());
 	}
 
+	private void clearAllExpired() {
+		for (Reference<Repository> ref : cacheMap.values()) {
+			Repository db = ref.get();
+			if (isExpired(db)) {
+				RepositoryCache.close(db);
+			}
+		}
+	}
+
 	private void clearAll() {
 		for (int stage = 0; stage < 2; stage++) {
 			for (Iterator<Map.Entry<Key, Reference<Repository>>> i = cacheMap

org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright (C) 2016 Ericsson
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.lib;
+
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Configuration parameters for JVM-wide repository cache used by JGit.
+ *
+ * @since 4.4
+ */
+public class RepositoryCacheConfig {
+
+	/**
+	 * Set cleanupDelayMillis to this value in order to switch off time-based
+	 * cache eviction. The JVM can still expire cache entries when heap memory
+	 * runs low.
+	 */
+	public static final long NO_CLEANUP = 0;
+
+	/**
+	 * Set cleanupDelayMillis to this value in order to auto-set it to minimum
+	 * of 1/10 of expireAfterMillis and 10 minutes
+	 */
+	public static final long AUTO_CLEANUP_DELAY = -1;
+
+	private long expireAfterMillis;
+
+	private long cleanupDelayMillis;
+
+	/** Create a default configuration. */
+	public RepositoryCacheConfig() {
+		expireAfterMillis = TimeUnit.HOURS.toMillis(1);
+		cleanupDelayMillis = AUTO_CLEANUP_DELAY;
+	}
+
+	/**
+	 * @return the time an unused repository should expired and be evicted from
+	 *         the RepositoryCache in milliseconds. <b>Default is 1 hour.</b>
+	 */
+	public long getExpireAfter() {
+		return expireAfterMillis;
+	}
+
+	/**
+	 * @param expireAfterMillis
+	 *            the time an unused repository should expired and be evicted
+	 *            from the RepositoryCache in milliseconds.
+	 */
+	public void setExpireAfter(long expireAfterMillis) {
+		this.expireAfterMillis = expireAfterMillis;
+	}
+
+	/**
+	 * @return the delay between the periodic cleanup of expired repository in
+	 *         milliseconds. <b>Default is minimum of 1/10 of expireAfterMillis
+	 *         and 10 minutes</b>
+	 */
+	public long getCleanupDelay() {
+		if (cleanupDelayMillis < 0) {
+			return Math.min(expireAfterMillis / 10,
+					TimeUnit.MINUTES.toMillis(10));
+		}
+		return cleanupDelayMillis;
+	}
+
+	/**
+	 * @param cleanupDelayMillis
+	 *            the delay between the periodic cleanup of expired repository
+	 *            in milliseconds. Set it to {@link #AUTO_CLEANUP_DELAY} to
+	 *            automatically derive cleanup delay from expireAfterMillis.
+	 *            <p>
+	 *            Set it to {@link #NO_CLEANUP} in order to switch off cache
+	 *            expiration.
+	 *            <p>
+	 *            If cache expiration is switched off the JVM still can evict
+	 *            cache entries when the JVM is running low on available heap
+	 *            memory.
+	 */
+	public void setCleanupDelay(long cleanupDelayMillis) {
+		this.cleanupDelayMillis = cleanupDelayMillis;
+	}
+
+	/**
+	 * Update properties by setting fields from the configuration.
+	 * <p>
+	 * If a property is not defined in the configuration, then it is left
+	 * unmodified.
+	 *
+	 * @param config
+	 *            configuration to read properties from.
+	 * @return {@code this}.
+	 */
+	public RepositoryCacheConfig fromConfig(Config config) {
+		setExpireAfter(
+				config.getTimeUnit("core", null, "repositoryCacheExpireAfter", //$NON-NLS-1$//$NON-NLS-2$
+						getExpireAfter(), TimeUnit.MILLISECONDS));
+		setCleanupDelay(
+				config.getTimeUnit("core", null, "repositoryCacheCleanupDelay", //$NON-NLS-1$ //$NON-NLS-2$
+						AUTO_CLEANUP_DELAY, TimeUnit.MILLISECONDS));
+		return this;
+	}
+
+	/**
+	 * Install this configuration as the live settings.
+	 * <p>
+	 * The new configuration is applied immediately.
+	 */
+	public void install() {
+		RepositoryCache.reconfigure(this);
+	}
+}