[JGIT PATCH 0/4] Avoid using 1280 file descriptors

[JGIT PATCH 0/4] Avoid using 1280 file descriptors

[Shawn O. Pearce]

This is a small series to teach WindowCache not to use 1280 file
descriptors by default.  Most JVMs won't have a ulimit that large,
resulting in out of file errors if the WindowCache actually tries
to fully populate itself.

Shawn O. Pearce (4):
  Refactor WindowCache.reconfigure() to take a configuration object
  Update EGit plugin to use WindowCacheConfig
  Cap the number of open files in the WindowCache
  Teach WindowCacheConfig to read core.packedgit* settings from config

 .../spearce/egit/core/project/GitProjectData.java  |   12 +-
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |    8 +-
 .../org/spearce/jgit/lib/UnpackedObjectCache.java  |    7 +-
 .../src/org/spearce/jgit/lib/WindowCache.java      |  119 ++++++++++----
 .../org/spearce/jgit/lib/WindowCacheConfig.java    |  170 ++++++++++++++++++++
 5 files changed, 271 insertions(+), 45 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java

[JGIT PATCH 1/4] Refactor WindowCache.reconfigure() to take a configuration object

[Shawn O. Pearce]

This makes it safer for applications to be calling reconfigure()
should we ever add more controllable parameters to cache.  It may
also leave the door open for more common option handling, such as
reading from ~/.gitconfig in a standard way across applications
built upon JGit.  We could also add sanity check functions to the
new configuration object, allowing applications to inquire about
the validity before installing it.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |    8 +-
 .../org/spearce/jgit/lib/UnpackedObjectCache.java  |    7 +-
 .../src/org/spearce/jgit/lib/WindowCache.java      |   54 +++++---
 .../org/spearce/jgit/lib/WindowCacheConfig.java    |  134 ++++++++++++++++++++
 4 files changed, 178 insertions(+), 25 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 4e56b38..5d8c056 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -113,8 +113,12 @@ public String getProperty(String key) {
 	 * Configure JGit before setting up test repositories.
 	 */
 	protected void configure() {
-		packedGitMMAP = "true".equals(System.getProperty("jgit.junit.usemmmap"));
-		WindowCache.reconfigure(128*1024, 8192, packedGitMMAP, 8192);
+		final WindowCacheConfig c = new WindowCacheConfig();
+		c.setPackedGitLimit(128 * WindowCacheConfig.KB);
+		c.setPackedGitWindowSize(8 * WindowCacheConfig.KB);
+		c.setPackedGitMMAP("true".equals(System.getProperty("jgit.junit.usemmmap")));
+		c.setDeltaBaseCacheLimit(8 * WindowCacheConfig.KB);
+		WindowCache.reconfigure(c);
 	}
 
 	/**
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
index 677b3a7..13861bf 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
@@ -42,8 +42,6 @@
 class UnpackedObjectCache {
 	private static final int CACHE_SZ = 1024;
 
-	private static final int MB = 1024 * 1024;
-
 	private static final SoftReference<Entry> DEAD;
 
 	private static int hash(final long position) {
@@ -62,14 +60,15 @@ private static int hash(final long position) {
 
 	static {
 		DEAD = new SoftReference<Entry>(null);
-		maxByteCount = 10 * MB;
+		maxByteCount = new WindowCacheConfig().getDeltaBaseCacheLimit();
 
 		cache = new Slot[CACHE_SZ];
 		for (int i = 0; i < CACHE_SZ; i++)
 			cache[i] = new Slot();
 	}
 
-	static synchronized void reconfigure(final int dbLimit) {
+	static synchronized void reconfigure(final WindowCacheConfig cfg) {
+		final int dbLimit = cfg.getDeltaBaseCacheLimit();
 		if (maxByteCount != dbLimit) {
 			maxByteCount = dbLimit;
 			releaseMemory();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
index 6a650cb..ba1124a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
@@ -46,10 +46,6 @@
  * the other windowed file access classes.
  */
 public class WindowCache {
-	private static final int KB = 1024;
-
-	private static final int MB = 1024 * KB;
-
 	private static final int bits(int newSize) {
 		if (newSize < 4096)
 			throw new IllegalArgumentException("Invalid window size");
@@ -77,10 +73,11 @@ private static final int bits(int newSize) {
 	private static int openByteCount;
 
 	static {
-		maxByteCount = 10 * MB;
-		windowSizeShift = bits(8 * KB);
+		final WindowCacheConfig c = new WindowCacheConfig();
+		maxByteCount = c.getPackedGitLimit();
+		windowSizeShift = bits(c.getPackedGitWindowSize());
 		windowSize = 1 << windowSizeShift;
-		mmap = false;
+		mmap = c.isPackedGitMMAP();
 		cache = new ByteWindow[cacheTableSize()];
 		clearedWindowQueue = new ReferenceQueue<Object>();
 	}
@@ -104,34 +101,53 @@ private static int cacheTableSize() {
 	 *            true to enable use of mmap when creating windows.
 	 * @param deltaBaseCacheLimit
 	 *            number of bytes to hold in the delta base cache.
+	 * @deprecated Use {@link WindowCacheConfig} instead.
 	 */
 	public static void reconfigure(final int packedGitLimit,
 			final int packedGitWindowSize, final boolean packedGitMMAP,
 			final int deltaBaseCacheLimit) {
-		reconfigureImpl(packedGitLimit, packedGitWindowSize, packedGitMMAP);
-		UnpackedObjectCache.reconfigure(deltaBaseCacheLimit);
+		final WindowCacheConfig c = new WindowCacheConfig();
+		c.setPackedGitLimit(packedGitLimit);
+		c.setPackedGitWindowSize(packedGitWindowSize);
+		c.setPackedGitMMAP(packedGitMMAP);
+		c.setDeltaBaseCacheLimit(deltaBaseCacheLimit);
+		reconfigure(c);
+	}
+
+	/**
+	 * Modify the configuration of the window cache.
+	 * <p>
+	 * The new configuration is applied immediately. If the new limits are
+	 * smaller than what what is currently cached, older entries will be purged
+	 * as soon as possible to allow the cache to meet the new limit.
+	 *
+	 * @param cfg
+	 *            the new window cache configuration.
+	 */
+	public static void reconfigure(final WindowCacheConfig cfg) {
+		reconfigureImpl(cfg);
+		UnpackedObjectCache.reconfigure(cfg);
 	}
 
-	private static synchronized void reconfigureImpl(final int packedGitLimit,
-			final int packedGitWindowSize, final boolean packedGitMMAP) {
+	private static synchronized void reconfigureImpl(final WindowCacheConfig cfg) {
 		boolean prune = false;
 		boolean evictAll = false;
 
-		if (maxByteCount < packedGitLimit) {
-			maxByteCount = packedGitLimit;
-		} else if (maxByteCount > packedGitLimit) {
-			maxByteCount = packedGitLimit;
+		if (maxByteCount < cfg.getPackedGitLimit()) {
+			maxByteCount = cfg.getPackedGitLimit();
+		} else if (maxByteCount > cfg.getPackedGitLimit()) {
+			maxByteCount = cfg.getPackedGitLimit();
 			prune = true;
 		}
 
-		if (bits(packedGitWindowSize) != windowSizeShift) {
-			windowSizeShift = bits(packedGitWindowSize);
+		if (bits(cfg.getPackedGitWindowSize()) != windowSizeShift) {
+			windowSizeShift = bits(cfg.getPackedGitWindowSize());
 			windowSize = 1 << windowSizeShift;
 			evictAll = true;
 		}
 
-		if (mmap != packedGitMMAP) {
-			mmap = packedGitMMAP;
+		if (mmap != cfg.isPackedGitMMAP()) {
+			mmap = cfg.isPackedGitMMAP();
 			evictAll = true;
 		}
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
new file mode 100644
index 0000000..b4c4638
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
@@ -0,0 +1,134 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * 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 Git Development Community 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.spearce.jgit.lib;
+
+/** Configuration parameters for {@link WindowCache}. */
+public class WindowCacheConfig {
+	/** 1024 (number of bytes in one kibibyte/kilobyte) */
+	public static final int KB = 1024;
+
+	/** 1024 {@link #KB} (number of bytes in one mebibyte/megabyte) */
+	public static final int MB = 1024 * KB;
+
+	private int packedGitLimit;
+
+	private int packedGitWindowSize;
+
+	private boolean packedGitMMAP;
+
+	private int deltaBaseCacheLimit;
+
+	/** Create a default configuration. */
+	public WindowCacheConfig() {
+		packedGitLimit = 10 * MB;
+		packedGitWindowSize = 8 * KB;
+		packedGitMMAP = false;
+		deltaBaseCacheLimit = 10 * MB;
+	}
+
+	/**
+	 * @return maximum number bytes of heap memory to dedicate to caching pack
+	 *         file data. <b>Default is 10 MB.</b>
+	 */
+	public int getPackedGitLimit() {
+		return packedGitLimit;
+	}
+
+	/**
+	 * @param newLimit
+	 *            maximum number bytes of heap memory to dedicate to caching
+	 *            pack file data.
+	 */
+	public void setPackedGitLimit(final int newLimit) {
+		packedGitLimit = newLimit;
+	}
+
+	/**
+	 * @return size in bytes of a single window mapped or read in from the pack
+	 *         file. <b>Default is 8 KB.</b>
+	 */
+	public int getPackedGitWindowSize() {
+		return packedGitWindowSize;
+	}
+
+	/**
+	 * @param newSize
+	 *            size in bytes of a single window read in from the pack file.
+	 */
+	public void setPackedGitWindowSize(final int newSize) {
+		packedGitWindowSize = newSize;
+	}
+
+	/**
+	 * @return true enables use of Java NIO virtual memory mapping for windows;
+	 *         false reads entire window into a byte[] with standard read calls.
+	 *         <b>Default false.</b>
+	 */
+	public boolean isPackedGitMMAP() {
+		return packedGitMMAP;
+	}
+
+	/**
+	 * @param usemmap
+	 *            true enables use of Java NIO virtual memory mapping for
+	 *            windows; false reads entire window into a byte[] with standard
+	 *            read calls.
+	 */
+	public void setPackedGitMMAP(final boolean usemmap) {
+		packedGitMMAP = usemmap;
+	}
+
+	/**
+	 * @return maximum number of bytes to cache in {@link UnpackedObjectCache}
+	 *         for inflated, recently accessed objects, without delta chains.
+	 *         <b>Default 10 MB.</b>
+	 */
+	public int getDeltaBaseCacheLimit() {
+		return deltaBaseCacheLimit;
+	}
+
+	/**
+	 * @param newLimit
+	 *            maximum number of bytes to cache in
+	 *            {@link UnpackedObjectCache} for inflated, recently accessed
+	 *            objects, without delta chains.
+	 */
+	public void setDeltaBaseCacheLimit(final int newLimit) {
+		deltaBaseCacheLimit = newLimit;
+	}
+}
-- 
1.6.2.1.286.g8173

[JGIT PATCH 2/4] Update EGit plugin to use WindowCacheConfig

[Shawn O. Pearce]

Its more flexible for future additions to the parameters.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/egit/core/project/GitProjectData.java  |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java b/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java
index 8b72818..31d5483 100644
--- a/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java
+++ b/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java
@@ -42,6 +42,7 @@
 import org.spearce.egit.core.GitProvider;
 import org.spearce.jgit.lib.Repository;
 import org.spearce.jgit.lib.WindowCache;
+import org.spearce.jgit.lib.WindowCacheConfig;
 
 /**
  * This class keeps information about how a project is mapped to
@@ -229,12 +230,13 @@ private synchronized static Repository lookupRepository(final File gitDir)
 	 * Update the settings for the global window cache of the workspace.
 	 */
 	public static void reconfigureWindowCache() {
+		final WindowCacheConfig c = new WindowCacheConfig();
 		Preferences p = Activator.getDefault().getPluginPreferences();
-		int wLimit = p.getInt(GitCorePreferences.core_packedGitLimit);
-		int wSize = p.getInt(GitCorePreferences.core_packedGitWindowSize);
-		boolean mmap = p.getBoolean(GitCorePreferences.core_packedGitMMAP);
-		int dbLimit = p.getInt(GitCorePreferences.core_deltaBaseCacheLimit);
-		WindowCache.reconfigure(wLimit, wSize, mmap, dbLimit);
+		c.setPackedGitLimit(p.getInt(GitCorePreferences.core_packedGitLimit));
+		c.setPackedGitWindowSize(p.getInt(GitCorePreferences.core_packedGitWindowSize));
+		c.setPackedGitMMAP(p.getBoolean(GitCorePreferences.core_packedGitMMAP));
+		c.setDeltaBaseCacheLimit(p.getInt(GitCorePreferences.core_deltaBaseCacheLimit));
+		WindowCache.reconfigure(c);
 	}
 
 	private final IProject project;
-- 
1.6.2.1.286.g8173

[JGIT PATCH 3/4] Cap the number of open files in the WindowCache

[Shawn O. Pearce]

The default WindowCache configuration is:

  packedGitLimit:      10 MB
  packedGitWindowSize:  8 KB

10 MB / 8 KB allows up to 1280 windows permitted in the WindowCache
at any given time.  If every window came from a unique pack file, we
need 1280 file descriptors just for the resources in the WindowCache.
For most applications this is way beyond the hard limit configured
for the host JVM, causing java.io.IOException("Too many open files")
from possibly any part of JGit or the host application.

Specifically, I ran into this problem in Gerrit Code Review, where we
commonly see hundreds of very small pack files spread over hundreds
of Git repositories, all accessed from a persistent JVM that is also
hosting an SSH daemon and a web server.  The aggressive caching of
windows in the WindowCache and of Repository objects in Gerrit's
own RepositoryCache caused us to retain far too many tiny pack files.

We now set the limit at 128 open files, assuming this is a reasonable
limit for most applications using the library.  Git repositories tend
to be in the handful of packs/repository (e.g. <10 packs/repository)
and applications using JGit tend to access only a handful of Git
repositories at a time (e.g. <10 repositories/JVM).

If we detect a file open failure while opening a pack we halve
the number of permitted open files and try again, until we reach
a lower bound of 16 open files.  Needing to go lower may indicate
a file descriptor leak in the host application.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/WindowCache.java      |   65 +++++++++++++++-----
 .../org/spearce/jgit/lib/WindowCacheConfig.java    |   20 ++++++
 2 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
index ba1124a..13912a7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
@@ -54,6 +54,8 @@ private static final int bits(int newSize) {
 		return Integer.numberOfTrailingZeros(newSize);
 	}
 
+	private static int maxFileCount;
+
 	private static int maxByteCount;
 
 	private static int windowSize;
@@ -70,10 +72,13 @@ private static final int bits(int newSize) {
 
 	private static ByteWindow lruTail;
 
+	private static int openFileCount;
+
 	private static int openByteCount;
 
 	static {
 		final WindowCacheConfig c = new WindowCacheConfig();
+		maxFileCount = c.getPackedGitOpenFiles();
 		maxByteCount = c.getPackedGitLimit();
 		windowSizeShift = bits(c.getPackedGitWindowSize());
 		windowSize = 1 << windowSizeShift;
@@ -133,6 +138,13 @@ private static synchronized void reconfigureImpl(final WindowCacheConfig cfg) {
 		boolean prune = false;
 		boolean evictAll = false;
 
+		if (maxFileCount < cfg.getPackedGitOpenFiles())
+			maxFileCount = cfg.getPackedGitOpenFiles();
+		else if (maxFileCount > cfg.getPackedGitOpenFiles()) {
+			maxFileCount = cfg.getPackedGitOpenFiles();
+			prune = true;
+		}
+
 		if (maxByteCount < cfg.getPackedGitLimit()) {
 			maxByteCount = cfg.getPackedGitLimit();
 		} else if (maxByteCount > cfg.getPackedGitLimit()) {
@@ -229,20 +241,37 @@ private static synchronized final void getImpl(final WindowCursor curs,
 		}
 
 		if (wp.openCount == 0) {
-			try {
-				wp.openCount = 1;
-				wp.cacheOpen();
-			} catch (IOException ioe) {
-				wp.openCount = 0;
-				throw ioe;
-			} catch (RuntimeException ioe) {
-				wp.openCount = 0;
-				throw ioe;
-			} catch (Error ioe) {
-				wp.openCount = 0;
-				throw ioe;
-			} finally {
-				wp.openCount--;
+			TRY_OPEN: for (;;) {
+				try {
+					openFileCount++;
+					releaseMemory();
+					runClearedWindowQueue();
+					wp.openCount = 1;
+					wp.cacheOpen();
+					break;
+				} catch (IOException ioe) {
+					openFileCount--;
+					if ("Too many open files".equals(ioe.getMessage())
+							&& maxFileCount > 16) {
+						// We may be able to recover by halving our limit
+						// and trying again.
+						//
+						maxFileCount = Math.max(16, maxFileCount >> 2);
+						continue TRY_OPEN;
+					}
+					wp.openCount = 0;
+					throw ioe;
+				} catch (RuntimeException ioe) {
+					openFileCount--;
+					wp.openCount = 0;
+					throw ioe;
+				} catch (Error ioe) {
+					openFileCount--;
+					wp.openCount = 0;
+					throw ioe;
+				} finally {
+					wp.openCount--;
+				}
 			}
 
 			// The cacheOpen may have mapped the window we are trying to
@@ -278,6 +307,7 @@ private static synchronized final void getImpl(final WindowCursor curs,
 
 	static synchronized void markLoaded(final ByteWindow w) {
 		if (--w.provider.openCount == 0) {
+			openFileCount--;
 			w.provider.cacheClose();
 		}
 	}
@@ -291,13 +321,17 @@ private static void makeMostRecent(ByteWindow<?> e) {
 
 	private static void releaseMemory() {
 		ByteWindow<?> e = lruTail;
-		while (openByteCount > maxByteCount && e != null) {
+		while (isOverLimit() && e != null) {
 			final ByteWindow<?> p = e.lruPrev;
 			clear(e);
 			e = p;
 		}
 	}
 
+	private static boolean isOverLimit() {
+		return openByteCount > maxByteCount || openFileCount > maxFileCount;
+	}
+
 	/**
 	 * Remove all windows associated with a specific provider.
 	 * <p>
@@ -341,6 +375,7 @@ private static void clear(final ByteWindow<?> e) {
 	private static void unlinkSize(final ByteWindow<?> e) {
 		if (e.sizeActive) {
 			if (--e.provider.openCount == 0) {
+				openFileCount--;
 				e.provider.cacheClose();
 			}
 			openByteCount -= e.size;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
index b4c4638..d906a7c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
@@ -45,6 +45,8 @@
 	/** 1024 {@link #KB} (number of bytes in one mebibyte/megabyte) */
 	public static final int MB = 1024 * KB;
 
+	private int packedGitOpenFiles;
+
 	private int packedGitLimit;
 
 	private int packedGitWindowSize;
@@ -55,6 +57,7 @@
 
 	/** Create a default configuration. */
 	public WindowCacheConfig() {
+		packedGitOpenFiles = 128;
 		packedGitLimit = 10 * MB;
 		packedGitWindowSize = 8 * KB;
 		packedGitMMAP = false;
@@ -62,6 +65,23 @@ public WindowCacheConfig() {
 	}
 
 	/**
+	 * @return maximum number of streams to open at a time. Open packs count
+	 *         against the process limits. <b>Default is 128.</b>
+	 */
+	public int getPackedGitOpenFiles() {
+		return packedGitOpenFiles;
+	}
+
+	/**
+	 * @param fdLimit
+	 *            maximum number of streams to open at a time. Open packs count
+	 *            against the process limits
+	 */
+	public void setPackedGitOpenFiles(final int fdLimit) {
+		packedGitOpenFiles = fdLimit;
+	}
+
+	/**
 	 * @return maximum number bytes of heap memory to dedicate to caching pack
 	 *         file data. <b>Default is 10 MB.</b>
 	 */
-- 
1.6.2.1.286.g8173

[JGIT PATCH 4/4] Teach WindowCacheConfig to read core.packedgit* settings from config

[Shawn O. Pearce]

This may help applications which want to read configuration from
the current repository, or from ~/.gitconfig when picking the access
strategy for the current process.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/lib/WindowCacheConfig.java    |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
index d906a7c..ea28164 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
@@ -151,4 +151,20 @@ public int getDeltaBaseCacheLimit() {
 	public void setDeltaBaseCacheLimit(final int newLimit) {
 		deltaBaseCacheLimit = newLimit;
 	}
+
+	/**
+	 * Update properties by setting fields from the configuration.
+	 * <p>
+	 * If a property is not defined in the configuration, then it is left
+	 * unmodified.
+	 *
+	 * @param rc configuration to read properties from.
+	 */
+	public void fromConfig(final RepositoryConfig rc) {
+		setPackedGitOpenFiles(rc.getInt("core", null, "packedgitopenfiles", getPackedGitOpenFiles()));
+		setPackedGitLimit(rc.getInt("core", null, "packedgitlimit", getPackedGitLimit()));
+		setPackedGitWindowSize(rc.getInt("core", null, "packedgitwindowsize", getPackedGitWindowSize()));
+		setPackedGitMMAP(rc.getBoolean("core", null, "packedgitmmap", isPackedGitMMAP()));
+		setDeltaBaseCacheLimit(rc.getInt("core", null, "deltabasecachelimit", getDeltaBaseCacheLimit()));
+	}
 }
-- 
1.6.2.1.286.g8173

Re: [JGIT PATCH 3/4] Cap the number of open files in the WindowCache

[Robin Rosenberg]

tisdag 17 mars 2009 02:16:09 skrev "Shawn O. Pearce" <spearce@spearce.org>:

> If we detect a file open failure while opening a pack we halve
> the number of permitted open files and try again, until we reach
> a lower bound of 16 open files.  Needing to go lower may indicate
> a file descriptor leak in the host application.
...
> +			TRY_OPEN: for (;;) {
> +				try {
> +					openFileCount++;
> +					releaseMemory();
> +					runClearedWindowQueue();
> +					wp.openCount = 1;
> +					wp.cacheOpen();
> +					break;
> +				} catch (IOException ioe) {
> +					openFileCount--;
> +					if ("Too many open files".equals(ioe.getMessage())
> +							&& maxFileCount > 16) {

The output of getMessage isn't that simple to interpret. Here it is filename+" (Too many files open)",
and on other platforms it is probably something else. This goes for the message part of most exceptions
thrown from platform specific code like file i/o socket i/o etc. The type of exception is a FileNotFoundException,
btw.

I wonder whether  your code works on any platform.

Re: [JGIT PATCH 3/4] Cap the number of open files in the WindowCache

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> tisdag 17 mars 2009 02:16:09 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> 
> > If we detect a file open failure while opening a pack we halve
> > the number of permitted open files and try again, [...]
> 
> The output of getMessage isn't that simple to interpret. Here it is filename+" (Too many files open)",
> and on other platforms it is probably something else. This goes for the message part of most exceptions
> thrown from platform specific code like file i/o socket i/o etc. The type of exception is a FileNotFoundException,
> btw.
> 
> I wonder whether  your code works on any platform.

Arrrgh.
 
OK.  Maybe scrap that part of the patch then?

Its too bad they don't have a specific type of exception for this,
nor do they have a way to hold onto file descriptors under a type
like a SoftReference where the runtime can whack them if you have
too many.

I guess that's why Hadoop HBase just tells you to up your fd ulimit
to 32767.  :-)

-- 
Shawn.

Re: [JGIT PATCH 3/4] Cap the number of open files in the WindowCache

[Robin Rosenberg]

onsdag 18 mars 2009 00:08:03 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > tisdag 17 mars 2009 02:16:09 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> > 
> > > If we detect a file open failure while opening a pack we halve
> > > the number of permitted open files and try again, [...]
> > 
> > The output of getMessage isn't that simple to interpret. Here it is filename+" (Too many files open)",
> > and on other platforms it is probably something else. This goes for the message part of most exceptions
> > thrown from platform specific code like file i/o socket i/o etc. The type of exception is a FileNotFoundException,
> > btw.
> > 
> > I wonder whether  your code works on any platform.
> 
> Arrrgh.
>  
> OK.  Maybe scrap that part of the patch then?
Yes, I think so, inless you want to try something as ugly as getMessage().toLower().indexof("(too many")? Not
sure what it looks like in Windows or OSX. We know from the JDK source it's filename + "(" + reason +")" and
the problem here is the reason part.

> Its too bad they don't have a specific type of exception for this,
FileNotFoundException is a little more specific. Maybe in combinatikon with a file.exists() and file.canRead test...
(thinking loud now)

> nor do they have a way to hold onto file descriptors under a type
> like a SoftReference where the runtime can whack them if you have
> too many.

The problem is that it's not connected to file descriptors but to memory. Doing a GC on filenotfoundexception
(here) could help here if one uses soft reference, or one could prune the cache manually. The parameter
could also be a 

> 
> I guess that's why Hadoop HBase just tells you to up your fd ulimit
> to 32767.  :-)

Yeah, with gigabytes of memory that might not consume too much resources anyway.

-- robin

[JGIT PATCH 3/4 v2] Cap the number of open files in the WindowCache

[Shawn O. Pearce]

The default WindowCache configuration is:

  packedGitLimit:      10 MB
  packedGitWindowSize:  8 KB

10 MB / 8 KB allows up to 1280 windows permitted in the WindowCache
at any given time.  If every window came from a unique pack file, we
need 1280 file descriptors just for the resources in the WindowCache.
For most applications this is way beyond the hard limit configured
for the host JVM, causing java.io.IOException("Too many open files")
from possibly any part of JGit or the host application.

Specifically, I ran into this problem in Gerrit Code Review, where we
commonly see hundreds of very small pack files spread over hundreds
of Git repositories, all accessed from a persistent JVM that is also
hosting an SSH daemon and a web server.  The aggressive caching of
windows in the WindowCache and of Repository objects in Gerrit's
own RepositoryCache caused us to retain far too many tiny pack files.

We now set the limit at 128 open files, assuming this is a reasonable
limit for most applications using the library.  Git repositories tend
to be in the handful of packs/repository (e.g. <10 packs/repository)
and applications using JGit tend to access only a handful of Git
repositories at a time (e.g. <10 repositories/JVM).

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
 > onsdag 18 mars 2009 00:08:03 skrev "Shawn O. Pearce" <spearce@spearce.org>:
 > > Arrrgh.
 > >  
 > > OK.  Maybe scrap that part of the patch then?
 > Yes, I think so

 Scrapped the ugly test for getMessage() when the open fails.
 So we no longer try to reduce fd usage on failure.

 .../src/org/spearce/jgit/lib/WindowCache.java      |   26 +++++++++++++++++++-
 .../org/spearce/jgit/lib/WindowCacheConfig.java    |   20 +++++++++++++++
 2 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
index ba1124a..f755dad 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
@@ -54,6 +54,8 @@ private static final int bits(int newSize) {
 		return Integer.numberOfTrailingZeros(newSize);
 	}
 
+	private static int maxFileCount;
+
 	private static int maxByteCount;
 
 	private static int windowSize;
@@ -70,10 +72,13 @@ private static final int bits(int newSize) {
 
 	private static ByteWindow lruTail;
 
+	private static int openFileCount;
+
 	private static int openByteCount;
 
 	static {
 		final WindowCacheConfig c = new WindowCacheConfig();
+		maxFileCount = c.getPackedGitOpenFiles();
 		maxByteCount = c.getPackedGitLimit();
 		windowSizeShift = bits(c.getPackedGitWindowSize());
 		windowSize = 1 << windowSizeShift;
@@ -133,6 +138,13 @@ private static synchronized void reconfigureImpl(final WindowCacheConfig cfg) {
 		boolean prune = false;
 		boolean evictAll = false;
 
+		if (maxFileCount < cfg.getPackedGitOpenFiles())
+			maxFileCount = cfg.getPackedGitOpenFiles();
+		else if (maxFileCount > cfg.getPackedGitOpenFiles()) {
+			maxFileCount = cfg.getPackedGitOpenFiles();
+			prune = true;
+		}
+
 		if (maxByteCount < cfg.getPackedGitLimit()) {
 			maxByteCount = cfg.getPackedGitLimit();
 		} else if (maxByteCount > cfg.getPackedGitLimit()) {
@@ -230,15 +242,21 @@ private static synchronized final void getImpl(final WindowCursor curs,
 
 		if (wp.openCount == 0) {
 			try {
+				openFileCount++;
+				releaseMemory();
+				runClearedWindowQueue();
 				wp.openCount = 1;
 				wp.cacheOpen();
 			} catch (IOException ioe) {
+				openFileCount--;
 				wp.openCount = 0;
 				throw ioe;
 			} catch (RuntimeException ioe) {
+				openFileCount--;
 				wp.openCount = 0;
 				throw ioe;
 			} catch (Error ioe) {
+				openFileCount--;
 				wp.openCount = 0;
 				throw ioe;
 			} finally {
@@ -278,6 +296,7 @@ private static synchronized final void getImpl(final WindowCursor curs,
 
 	static synchronized void markLoaded(final ByteWindow w) {
 		if (--w.provider.openCount == 0) {
+			openFileCount--;
 			w.provider.cacheClose();
 		}
 	}
@@ -291,13 +310,17 @@ private static void makeMostRecent(ByteWindow<?> e) {
 
 	private static void releaseMemory() {
 		ByteWindow<?> e = lruTail;
-		while (openByteCount > maxByteCount && e != null) {
+		while (isOverLimit() && e != null) {
 			final ByteWindow<?> p = e.lruPrev;
 			clear(e);
 			e = p;
 		}
 	}
 
+	private static boolean isOverLimit() {
+		return openByteCount > maxByteCount || openFileCount > maxFileCount;
+	}
+
 	/**
 	 * Remove all windows associated with a specific provider.
 	 * <p>
@@ -341,6 +364,7 @@ private static void clear(final ByteWindow<?> e) {
 	private static void unlinkSize(final ByteWindow<?> e) {
 		if (e.sizeActive) {
 			if (--e.provider.openCount == 0) {
+				openFileCount--;
 				e.provider.cacheClose();
 			}
 			openByteCount -= e.size;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
index b4c4638..d906a7c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
@@ -45,6 +45,8 @@
 	/** 1024 {@link #KB} (number of bytes in one mebibyte/megabyte) */
 	public static final int MB = 1024 * KB;
 
+	private int packedGitOpenFiles;
+
 	private int packedGitLimit;
 
 	private int packedGitWindowSize;
@@ -55,6 +57,7 @@
 
 	/** Create a default configuration. */
 	public WindowCacheConfig() {
+		packedGitOpenFiles = 128;
 		packedGitLimit = 10 * MB;
 		packedGitWindowSize = 8 * KB;
 		packedGitMMAP = false;
@@ -62,6 +65,23 @@ public WindowCacheConfig() {
 	}
 
 	/**
+	 * @return maximum number of streams to open at a time. Open packs count
+	 *         against the process limits. <b>Default is 128.</b>
+	 */
+	public int getPackedGitOpenFiles() {
+		return packedGitOpenFiles;
+	}
+
+	/**
+	 * @param fdLimit
+	 *            maximum number of streams to open at a time. Open packs count
+	 *            against the process limits
+	 */
+	public void setPackedGitOpenFiles(final int fdLimit) {
+		packedGitOpenFiles = fdLimit;
+	}
+
+	/**
 	 * @return maximum number bytes of heap memory to dedicate to caching pack
 	 *         file data. <b>Default is 10 MB.</b>
 	 */
-- 
1.6.2.1.286.g8173