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

[JGIT PATCH 11/19] Remove the map lookup for values in Config

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

Update operations are already O(N) time in the config file, as we scan through every entry in the file, in order, looking to see if it matches the key we need to mutate. As most configurations are rather small in size the running time is trivial. Doing the same for lookup simplifies our code considerably.

Applications really want a different type of fast lookup strategy. Instead of by key lookups they want something like CoreConfig or RemoteConfig to be cached and quickly obtainable on demand, where the all relevant values have been parsed from their string content representation into the application specific data types. This change does not provide that functionality, but rather assumes we will do so in the near future.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/Config.java           |  129 +++++---------------
 1 files changed, 30 insertions(+), 99 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 d63e926..0f91412 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -43,10 +43,8 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
 import org.spearce.jgit.errors.ConfigInvalidException;
@@ -66,11 +64,6 @@
 	final Config baseConfig;
 
 	/**
-	 * Map from name to values
-	 */
-	private Map<String, Object> byName;
-
-	/**
 	 * Magic value indicating a missing entry.
 	 * <p>
 	 * This value is tested for reference equality in some contexts, so we
@@ -334,19 +327,9 @@ public String getString(final String section, String subsection,
 	 */
 	public String[] getStringList(final String section, String subsection,
 			final String name) {
-		final Object o = getRawEntry(section, subsection, name);
-		if (o instanceof List) {
-			final List lst = (List) o;
-			final String[] r = new String[lst.size()];
-			for (int i = 0; i < r.length; i++)
-				r[i] = ((Entry) lst.get(i)).value;
-			return r;
-		}
-
-		if (o instanceof Entry) {
-			return new String[] { ((Entry) o).value };
-		}
-
+		final List<String> lst = getRawStringList(section, subsection, name);
+		if (lst != null)
+			return lst.toArray(new String[lst.size()]);
 		if (baseConfig != null)
 			return baseConfig.getStringList(section, subsection, name);
 		return new String[0];
@@ -373,43 +356,36 @@ public String getString(final String section, String subsection,
 
 	private String getRawString(final String section, final String subsection,
 			final String name) {
-		final Object o = getRawEntry(section, subsection, name);
-		if (o instanceof List) {
-			return ((Entry) ((List) o).get(0)).value;
-		} else if (o instanceof Entry) {
-			return ((Entry) o).value;
-		} else if (baseConfig != null)
+		final List<String> lst = getRawStringList(section, subsection, name);
+		if (lst != null)
+			return lst.get(0);
+		else if (baseConfig != null)
 			return baseConfig.getRawString(section, subsection, name);
 		else
 			return null;
 	}
 
-	private Object getRawEntry(final String section, final String subsection,
-			final String name) {
-		return byName.get(concatenateKey(section, subsection, name));
+	private List<String> getRawStringList(final String section,
+			final String subsection, final String name) {
+		List<String> r = null;
+		for (final Entry e : entries) {
+			if (e.match(section, subsection, name))
+				r = add(r, e.value);
+		}
+		return r;
 	}
 
-	/**
-	 * Create simple a key name from the key components
-	 *
-	 * @param section
-	 *            the section name
-	 * @param subsection
-	 *            the subsection name
-	 * @param name
-	 *            the key name
-	 * @return a simple key name that have all components concatenated and the
-	 *         case converted
-	 */
-	private static String concatenateKey(final String section,
-			final String subsection, final String name) {
-		String ss;
-		if (subsection != null)
-			ss = "." + subsection;
-		else
-			ss = "";
-		return StringUtils.toLowerCase(section) + ss + "."
-				+ StringUtils.toLowerCase(name);
+	private static List<String> add(final List<String> curr, final String value) {
+		if (curr == null)
+			return Collections.singletonList(value);
+		if (curr.size() == 1) {
+			final List<String> r = new ArrayList<String>(2);
+			r.add(curr.get(0));
+			r.add(value);
+			return r;
+		}
+		curr.add(value);
+		return curr;
 	}
 
 	/**
@@ -551,31 +527,6 @@ 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) {
-		// Update our parsed cache of values for future reference.
-		//
-		String key = concatenateKey(section, subsection, name);
-		if (values.size() == 0)
-			byName.remove(key);
-		else if (values.size() == 1) {
-			final Entry e = new Entry();
-			e.section = section;
-			e.subsection = subsection;
-			e.name = name;
-			e.value = values.get(0);
-			byName.put(key, e);
-		} else {
-			final ArrayList<Entry> eList = new ArrayList<Entry>(values.size());
-			for (final String v : values) {
-				final Entry e = new Entry();
-				e.section = section;
-				e.subsection = subsection;
-				e.name = name;
-				e.value = v;
-				eList.add(e);
-			}
-			byName.put(key, eList);
-		}
-
 		int entryIndex = 0;
 		int valueIndex = 0;
 		int insertPosition = -1;
@@ -697,9 +648,7 @@ public String toText() {
 	 *             made to {@code this}.
 	 */
 	public void fromText(final String text) throws ConfigInvalidException {
-		entries = new ArrayList<Entry>();
-		byName = new HashMap<String, Object>();
-
+		final List<Entry> newEntries = new ArrayList<Entry>();
 		final StringReader in = new StringReader(text);
 		Entry last = null;
 		Entry e = new Entry();
@@ -711,7 +660,7 @@ public void fromText(final String text) throws ConfigInvalidException {
 			final char c = (char) input;
 			if ('\n' == c) {
 				// End of this entry.
-				add(e);
+				newEntries.add(e);
 				if (e.section != null)
 					last = e;
 				e = new Entry();
@@ -757,6 +706,8 @@ public void fromText(final String text) throws ConfigInvalidException {
 			} else
 				throw new ConfigInvalidException("Invalid line in config file");
 		}
+
+		entries = newEntries;
 	}
 
 	/**
@@ -764,26 +715,6 @@ public void fromText(final String text) throws ConfigInvalidException {
 	 */
 	protected void clear() {
 		entries = new ArrayList<Entry>();
-		byName = new HashMap<String, Object>();
-	}
-
-	@SuppressWarnings("unchecked")
-	private void add(final Entry e) {
-		entries.add(e);
-		if (e.section != null && e.name != null) {
-			final String key = concatenateKey(e.section, e.subsection, e.name);
-			final Object o = byName.get(key);
-			if (o == null) {
-				byName.put(key, e);
-			} else if (o instanceof Entry) {
-				final ArrayList<Object> l = new ArrayList<Object>();
-				l.add(o);
-				l.add(e);
-				byName.put(key, l);
-			} else if (o instanceof List) {
-				((List<Entry>) o).add(e);
-			}
-		}
 	}
 
 	private static String readSectionName(final StringReader in)

-- 
1.6.4.rc2.216.g769fa
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 12 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.