git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org,
	Constantine Plotnikov <constantine.plotnikov@gmail.com>
Subject: [JGIT PATCH 14/19] Support cached application models in a Config
Date: Sat, 25 Jul 2009 11:52:57 -0700	[thread overview]
Message-ID: <1248547982-4003-15-git-send-email-spearce@spearce.org> (raw)
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

  reply	other threads:[~2009-07-25 18:54 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-25 18:52 [JGIT PATCH 00/19] More Config class cleanup work Shawn O. Pearce
2009-07-25 18:52 ` [JGIT PATCH 01/19] Cleanup nonstandard references to encoding strings to bytes Shawn O. Pearce
2009-07-25 18:52   ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Shawn O. Pearce
2009-07-25 18:52     ` [JGIT PATCH 03/19] Make Config.escapeValue a private method Shawn O. Pearce
2009-07-25 18:52       ` [JGIT PATCH 04/19] Allow a RemoteConfig to use the more generic Config class Shawn O. Pearce
2009-07-25 18:52         ` [JGIT PATCH 05/19] Use type specific sets when creating a new RepositoryConfig Shawn O. Pearce
2009-07-25 18:52           ` [JGIT PATCH 06/19] Move SystemReader out of RepositoryConfig Shawn O. Pearce
2009-07-25 18:52             ` [JGIT PATCH 07/19] Correct user config to be of type FileBasedConfig Shawn O. Pearce
2009-07-25 18:52               ` [JGIT PATCH 08/19] Extract the test specific SystemReader out of RepositoryTestCase Shawn O. Pearce
2009-07-25 18:52                 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Shawn O. Pearce
2009-07-25 18:52                   ` [JGIT PATCH 10/19] Test for the config file when creating a new repository Shawn O. Pearce
2009-07-25 18:52                     ` [JGIT PATCH 11/19] Remove the map lookup for values in Config Shawn O. Pearce
2009-07-25 18:52                       ` [JGIT PATCH 12/19] Return base values first from Config.getStringList() Shawn O. Pearce
2009-07-25 18:52                         ` [JGIT PATCH 13/19] Make Config thread safe by using copy-on-write semantics Shawn O. Pearce
2009-07-25 18:52                           ` Shawn O. Pearce [this message]
2009-07-25 18:52                             ` [JGIT PATCH 15/19] Cache Config subsection names when requested by application code Shawn O. Pearce
2009-07-25 18:52                               ` [JGIT PATCH 16/19] Refactor author/committer lookup to use cached data Shawn O. Pearce
2009-07-25 18:53                                 ` [JGIT PATCH 17/19] Move repository config creation fully into Repository class Shawn O. Pearce
2009-07-25 18:53                                   ` [JGIT PATCH 18/19] Use Config SectionParser cache to store daemon enable states Shawn O. Pearce
2009-07-25 18:53                                     ` [JGIT PATCH 19/19] Use Config cache for fetch and receive configuration parsing Shawn O. Pearce
2009-07-25 22:54                   ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Robin Rosenberg
2009-07-25 22:55                     ` Shawn O. Pearce
2009-07-25 23:34                       ` Robin Rosenberg
2009-07-25 23:38                         ` Shawn O. Pearce
2009-07-25 20:32     ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Robin Rosenberg
2009-07-25 20:33       ` Shawn O. Pearce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1248547982-4003-15-git-send-email-spearce@spearce.org \
    --to=spearce@spearce.org \
    --cc=constantine.plotnikov@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=robin.rosenberg@dewire.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).