git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[JGIT PATCH 14/19] Support cached application models in a Config

From
Shawn O. Pearce <spearce@spearce.org>
Date
Jul 25, 2009, 18:52 UTC
Message-ID
<1248547982-4003-15-git-send-email-spearce@spearce.org>
In-Reply-To
<1248547982-4003-14-git-send-email-spearce@spearce.org>

The Config.SectionParser acts as a key into a cache of model object instances, but also knows how to create its model instance if the cache does not contain the corresponding instance.

Because we don't keep track of which keys were accessed by a parser when it created its model, we discard the entire cache anytime any key is modified, forcing all models to reparse on the next access.

Both CoreConfig and TransferConfig are trivial to switch to this method of access, although the typical get path is quite a bit longer as we now need to do a hash lookup instead of a getfield instruction.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Constantine Plotnikov <constantine.plotnikov@gmail.com>
---
 .../src/org/spearce/jgit/lib/Config.java           |   92 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/CoreConfig.java       |   16 +++-
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |   20 +----
 .../src/org/spearce/jgit/lib/TransferConfig.java   |   11 ++-
 4 files changed, 112 insertions(+), 27 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index 4d4b315..6036c4c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -45,7 +45,9 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.spearce.jgit.errors.ConfigInvalidException;
@@ -375,6 +377,43 @@ public String getString(final String section, String subsection,
 		return result;
 	}
 
+	/**
+	 * Obtain a handle to a parsed set of configuration values.
+	 *
+	 * @param <T>
+	 *            type of configuration model to return.
+	 * @param parser
+	 *            parser which can create the model if it is not already
+	 *            available in this configuration file. The parser is also used
+	 *            as the key into a cache and must obey the hashCode and equals
+	 *            contract in order to reuse a parsed model.
+	 * @return the parsed object instance, which is cached inside this config.
+	 */
+	@SuppressWarnings("unchecked")
+	public <T> T get(final SectionParser<T> parser) {
+		final State myState = getState();
+		T obj = (T) myState.cache.get(parser);
+		if (obj == null) {
+			obj = parser.parse(this);
+			myState.cache.put(parser, obj);
+		}
+		return obj;
+	}
+
+	/**
+	 * Remove a cached configuration object.
+	 * <p>
+	 * If the associated configuration object has not yet been cached, this
+	 * method has no effect.
+	 *
+	 * @param parser
+	 *            parser used to obtain the configuration object.
+	 * @see #get(SectionParser)
+	 */
+	public void uncache(final SectionParser<?> parser) {
+		state.get().cache.remove(parser);
+	}
+
 	private String getRawString(final String section, final String subsection,
 			final String name) {
 		final List<String> lst = getRawStringList(section, subsection, name);
@@ -409,6 +448,22 @@ else if (baseConfig != null)
 		return curr;
 	}
 
+	private State getState() {
+		State cur, upd;
+		do {
+			cur = state.get();
+			final State base = getBaseState();
+			if (cur.baseState == base)
+				return cur;
+			upd = new State(cur.entryList, base);
+		} while (!state.compareAndSet(cur, upd));
+		return upd;
+	}
+
+	private State getBaseState() {
+		return baseConfig != null ? baseConfig.getState() : null;
+	}
+
 	/**
 	 * Add or modify a configuration value. The parameters will result in a
 	 * configuration entry like this.
@@ -757,11 +812,11 @@ public void fromText(final String text) throws ConfigInvalidException {
 	}
 
 	private State newState() {
-		return new State(Collections.<Entry> emptyList());
+		return new State(Collections.<Entry> emptyList(), getBaseState());
 	}
 
 	private State newState(final List<Entry> entries) {
-		return new State(Collections.unmodifiableList(entries));
+		return new State(Collections.unmodifiableList(entries), getBaseState());
 	}
 
 	/**
@@ -933,11 +988,42 @@ private static String readValue(final StringReader in, boolean quote,
 		return value.length() > 0 ? value.toString() : null;
 	}
 
+	/**
+	 * Parses a section of the configuration into an application model object.
+	 * <p>
+	 * Instances must implement hashCode and equals such that model objects can
+	 * be cached by using the {@code SectionParser} as a key of a HashMap.
+	 * <p>
+	 * As the {@code SectionParser} itself is used as the key of the internal
+	 * HashMap applications should be careful to ensure the SectionParser key
+	 * does not retain unnecessary application state which may cause memory to
+	 * be held longer than expected.
+	 *
+	 * @param <T>
+	 *            type of the application model created by the parser.
+	 */
+	public static interface SectionParser<T> {
+		/**
+		 * Create a model object from a configuration.
+		 *
+		 * @param cfg
+		 *            the configuration to read values from.
+		 * @return the application model instance.
+		 */
+		T parse(Config cfg);
+	}
+
 	private static class State {
 		final List<Entry> entryList;
 
-		State(List<Entry> entries) {
+		final Map<Object, Object> cache;
+
+		final State baseState;
+
+		State(List<Entry> entries, State base) {
 			entryList = entries;
+			cache = new ConcurrentHashMap<Object, Object>(16, 0.75f, 1);
+			baseState = base;
 		}
 	}
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java
index e98e0bc..ed3827b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java
@@ -38,22 +38,28 @@
 
 package org.spearce.jgit.lib;
 
-import java.util.zip.Deflater;
+import static java.util.zip.Deflater.DEFAULT_COMPRESSION;
+
+import org.spearce.jgit.lib.Config.SectionParser;
 
 /**
  * This class keeps git repository core parameters.
  */
 public class CoreConfig {
-	private static final int DEFAULT_COMPRESSION = Deflater.DEFAULT_COMPRESSION;
-	private static final int DEFAULT_INDEXVERSION = 2;
+	/** Key for {@link Config#get(SectionParser)}. */
+	public static final Config.SectionParser<CoreConfig> KEY = new SectionParser<CoreConfig>() {
+		public CoreConfig parse(final Config cfg) {
+			return new CoreConfig(cfg);
+		}
+	};
 
 	private final int compression;
 
 	private final int packIndexVersion;
 
-	CoreConfig(final RepositoryConfig rc) {
+	private CoreConfig(final Config rc) {
 		compression = rc.getInt("core", "compression", DEFAULT_COMPRESSION);
-		packIndexVersion = rc.getInt("pack", "indexversion", DEFAULT_INDEXVERSION);
+		packIndexVersion = rc.getInt("pack", "indexversion", 2);
 	}
 
 	/**
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index fb97747..c6a13b6 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -42,9 +42,7 @@
 package org.spearce.jgit.lib;
 
 import java.io.File;
-import java.io.IOException;
 
-import org.spearce.jgit.errors.ConfigInvalidException;
 import org.spearce.jgit.util.SystemReader;
 
 /**
@@ -57,10 +55,6 @@
 	/** Section name for a branch configuration. */
 	public static final String BRANCH_SECTION = "branch";
 
-	CoreConfig core;
-
-	TransferConfig transfer;
-
 	/**
 	 * Create a Git configuration file reader/writer/cache for a specific file.
 	 *
@@ -79,14 +73,14 @@ public RepositoryConfig(final Config base, final File cfgLocation) {
 	 * @return Core configuration values
 	 */
 	public CoreConfig getCore() {
-		return core;
+		return get(CoreConfig.KEY);
 	}
 
 	/**
 	 * @return transfer, fetch and receive configuration values
 	 */
 	public TransferConfig getTransfer() {
-		return transfer;
+		return get(TransferConfig.KEY);
 	}
 
 	/**
@@ -175,15 +169,5 @@ public void create() {
 		clear();
 		setInt("core", null, "repositoryformatversion", 0);
 		setBoolean("core", null, "filemode", true);
-
-		core = new CoreConfig(this);
-		transfer = new TransferConfig(this);
-	}
-
-	@Override
-	public void load() throws IOException, ConfigInvalidException {
-		super.load();
-		core = new CoreConfig(this);
-		transfer = new TransferConfig(this);
 	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java
index ff3a5eb..8760103 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java
@@ -37,13 +37,22 @@
 
 package org.spearce.jgit.lib;
 
+import org.spearce.jgit.lib.Config.SectionParser;
+
 /**
  * The standard "transfer", "fetch" and "receive" configuration parameters.
  */
 public class TransferConfig {
+	/** Key for {@link Config#get(SectionParser)}. */
+	public static final Config.SectionParser<TransferConfig> KEY = new SectionParser<TransferConfig>() {
+		public TransferConfig parse(final Config cfg) {
+			return new TransferConfig(cfg);
+		}
+	};
+
 	private final boolean fsckObjects;
 
-	TransferConfig(final RepositoryConfig rc) {
+	private TransferConfig(final Config rc) {
 		fsckObjects = rc.getBoolean("receive", "fsckobjects", false);
 	}
 

-- 
1.6.4.rc2.216.g769fa
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 15 of 26 in “More Config class cleanup work”
  1. 00/19 More Config class cleanup workShawn O. Pearce, Jul 25, 2009
  2. 01/19 Cleanup nonstandard references to encoding strings to bytesShawn O. Pearce, Jul 25, 2009
  3. 02/19 Delete incorrect Javadoc from Config's getRawString methodShawn O. Pearce, Jul 25, 2009
  4. 03/19 Make Config.escapeValue a private methodShawn O. Pearce, Jul 25, 2009
  5. 04/19 Allow a RemoteConfig to use the more generic Config classShawn O. Pearce, Jul 25, 2009
  6. 05/19 Use type specific sets when creating a new RepositoryConfigShawn O. Pearce, Jul 25, 2009
  7. 06/19 Move SystemReader out of RepositoryConfigShawn O. Pearce, Jul 25, 2009
  8. 07/19 Correct user config to be of type FileBasedConfigShawn O. Pearce, Jul 25, 2009
  9. 08/19 Extract the test specific SystemReader out of RepositoryTestCaseShawn O. Pearce, Jul 25, 2009
  10. 09/19 Refactor Config hierarchy to make IO more explicitShawn O. Pearce, Jul 25, 2009
  11. 10/19 Test for the config file when creating a new repositoryShawn O. Pearce, Jul 25, 2009
  12. 11/19 Remove the map lookup for values in ConfigShawn O. Pearce, Jul 25, 2009
  13. 12/19 Return base values first from Config.getStringList()Shawn O. Pearce, Jul 25, 2009
  14. 13/19 Make Config thread safe by using copy-on-write semanticsShawn O. Pearce, Jul 25, 2009
  15. 14/19 Support cached application models in a ConfigShawn O. Pearce, Jul 25, 2009
  16. 15/19 Cache Config subsection names when requested by application codeShawn O. Pearce, Jul 25, 2009
  17. 16/19 Refactor author/committer lookup to use cached dataShawn O. Pearce, Jul 25, 2009
  18. 17/19 Move repository config creation fully into Repository classShawn O. Pearce, Jul 25, 2009
  19. 18/19 Use Config SectionParser cache to store daemon enable statesShawn O. Pearce, Jul 25, 2009
  20. 19/19 Use Config cache for fetch and receive configuration parsingShawn O. Pearce, Jul 25, 2009
  21. Robin RosenbergJul 25, 2009
  22. Shawn O. PearceJul 25, 2009
  23. Robin RosenbergJul 25, 2009
  24. Shawn O. PearceJul 25, 2009
  25. Robin RosenbergJul 25, 2009
  26. Shawn O. PearceJul 25, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.