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
Subject: [JGIT PATCH 13/19] Make Config thread safe by using copy-on-write semantics
Date: Sat, 25 Jul 2009 11:52:56 -0700	[thread overview]
Message-ID: <1248547982-4003-14-git-send-email-spearce@spearce.org> (raw)
In-Reply-To: <1248547982-4003-13-git-send-email-spearce@spearce.org>

It is relatively rare that the Config instance is modified from
application code.  When it is, we usually are working on a small
set of values read from .git/config or ~/.gitconfig.  Making the
class thread-safe through basic copy-on-write semantics for its
inner collection of entries simplifies the locking and permits us
to optimize for the much more common read path.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/Config.java           |   85 +++++++++++++++++---
 1 files changed, 72 insertions(+), 13 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 f4e78f3..4d4b315 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -46,6 +46,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.spearce.jgit.errors.ConfigInvalidException;
 import org.spearce.jgit.util.StringUtils;
@@ -60,9 +61,15 @@
 	private static final long MiB = 1024 * KiB;
 	private static final long GiB = 1024 * MiB;
 
-	private List<Entry> entries;
+	/**
+	 * Immutable current state of the configuration data.
+	 * <p>
+	 * This state is copy-on-write. It should always contain an immutable list
+	 * of the configuration keys/values.
+	 */
+	private final AtomicReference<State> state;
 
-	final Config baseConfig;
+	private final Config baseConfig;
 
 	/**
 	 * Magic value indicating a missing entry.
@@ -87,7 +94,7 @@ public Config() {
 	 */
 	public Config(Config defaultConfig) {
 		baseConfig = defaultConfig;
-		clear();
+		state = new AtomicReference<State>(newState());
 	}
 
 	/**
@@ -358,7 +365,7 @@ public String getString(final String section, String subsection,
 	 */
 	public Set<String> getSubsections(final String section) {
 		final Set<String> result = new HashSet<String>();
-		for (final Entry e : entries) {
+		for (final Entry e : state.get().entryList) {
 			if (StringUtils.equalsIgnoreCase(section, e.section)
 					&& e.subsection != null)
 				result.add(e.subsection);
@@ -382,7 +389,7 @@ else if (baseConfig != null)
 	private List<String> getRawStringList(final String section,
 			final String subsection, final String name) {
 		List<String> r = null;
-		for (final Entry e : entries) {
+		for (final Entry e : state.get().entryList) {
 			if (e.match(section, subsection, name))
 				r = add(r, e.value);
 		}
@@ -541,6 +548,17 @@ public void unset(final String section, final String subsection,
 	 */
 	public void setStringList(final String section, final String subsection,
 			final String name, final List<String> values) {
+		State src, res;
+		do {
+			src = state.get();
+			res = replaceStringList(src, section, subsection, name, values);
+		} while (!state.compareAndSet(src, res));
+	}
+
+	private State replaceStringList(final State srcState,
+			final String section, final String subsection, final String name,
+			final List<String> values) {
+		final List<Entry> entries = copy(srcState, values);
 		int entryIndex = 0;
 		int valueIndex = 0;
 		int insertPosition = -1;
@@ -548,11 +566,12 @@ public void setStringList(final String section, final String subsection,
 		// Reset the first n Entry objects that match this input name.
 		//
 		while (entryIndex < entries.size() && valueIndex < values.size()) {
-			final Entry e = entries.get(entryIndex++);
+			final Entry e = entries.get(entryIndex);
 			if (e.match(section, subsection, name)) {
-				e.value = values.get(valueIndex++);
-				insertPosition = entryIndex;
+				entries.set(entryIndex, e.forValue(values.get(valueIndex++)));
+				insertPosition = entryIndex + 1;
 			}
+			entryIndex++;
 		}
 
 		// Remove any extra Entry objects that we no longer need.
@@ -573,7 +592,7 @@ public void setStringList(final String section, final String subsection,
 				// is already a section available that matches. Insert
 				// after the last key of that section.
 				//
-				insertPosition = findSectionEnd(section, subsection);
+				insertPosition = findSectionEnd(entries, section, subsection);
 			}
 			if (insertPosition < 0) {
 				// We didn't find any matching section header for this key,
@@ -594,9 +613,22 @@ public void setStringList(final String section, final String subsection,
 				entries.add(insertPosition++, e);
 			}
 		}
+
+		return newState(entries);
+	}
+
+	private static List<Entry> copy(final State src, final List<String> values) {
+		// At worst we need to insert 1 line for each value, plus 1 line
+		// for a new section header. Assume that and allocate the space.
+		//
+		final int max = src.entryList.size() + values.size() + 1;
+		final ArrayList<Entry> r = new ArrayList<Entry>(max);
+		r.addAll(src.entryList);
+		return r;
 	}
 
-	private int findSectionEnd(final String section, final String subsection) {
+	private static int findSectionEnd(final List<Entry> entries,
+			final String section, final String subsection) {
 		for (int i = 0; i < entries.size(); i++) {
 			Entry e = entries.get(i);
 			if (e.match(section, subsection, null)) {
@@ -619,7 +651,7 @@ private int findSectionEnd(final String section, final String subsection) {
 	 */
 	public String toText() {
 		final StringBuilder out = new StringBuilder();
-		for (final Entry e : entries) {
+		for (final Entry e : state.get().entryList) {
 			if (e.prefix != null)
 				out.append(e.prefix);			
 			if (e.section != null && e.name == null) {
@@ -721,14 +753,22 @@ public void fromText(final String text) throws ConfigInvalidException {
 				throw new ConfigInvalidException("Invalid line in config file");
 		}
 
-		entries = newEntries;
+		state.set(newState(newEntries));
+	}
+
+	private State newState() {
+		return new State(Collections.<Entry> emptyList());
+	}
+
+	private State newState(final List<Entry> entries) {
+		return new State(Collections.unmodifiableList(entries));
 	}
 
 	/**
 	 * Clear the configuration file
 	 */
 	protected void clear() {
-		entries = new ArrayList<Entry>();
+		state.set(newState());
 	}
 
 	private static String readSectionName(final StringReader in)
@@ -893,6 +933,14 @@ private static String readValue(final StringReader in, boolean quote,
 		return value.length() > 0 ? value.toString() : null;
 	}
 
+	private static class State {
+		final List<Entry> entryList;
+
+		State(List<Entry> entries) {
+			entryList = entries;
+		}
+	}
+
 	/**
 	 * The configuration file entry
 	 */
@@ -927,6 +975,17 @@ private static String readValue(final StringReader in, boolean quote,
 		 */
 		String suffix;
 
+		Entry forValue(final String newValue) {
+			final Entry e = new Entry();
+			e.prefix = prefix;
+			e.section = section;
+			e.subsection = subsection;
+			e.name = name;
+			e.value = newValue;
+			e.suffix = suffix;
+			return e;
+		}
+
 		boolean match(final String aSection, final String aSubsection,
 				final String aKey) {
 			return eqIgnoreCase(section, aSection)
-- 
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                         ` Shawn O. Pearce [this message]
2009-07-25 18:52                           ` [JGIT PATCH 14/19] Support cached application models in a Config Shawn O. Pearce
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-14-git-send-email-spearce@spearce.org \
    --to=spearce@spearce.org \
    --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).