From c0b1443926b001db2620b65103e4526b955a0897 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 27 Mar 2012 15:32:16 -0400 Subject: [PATCH] Index config section and subsection names in one pass Instead of indexing the subsection names on each request for a given section name, index both the section and subsection names in a single scan through the entry list. This should improve lookup time for reading the section names out of the configuration, especially for the url.*.insteadof type of processing performed in RemoteConfig. Change-Id: I7b3269565b1308f69d20dc3f3fe917aea00f8a73 --- .../tst/org/eclipse/jgit/lib/ConfigTest.java | 2 +- .../src/org/eclipse/jgit/lib/Config.java | 158 ++---------------- .../org/eclipse/jgit/lib/ConfigSnapshot.java | 126 ++++++++++++++ 3 files changed, 137 insertions(+), 149 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java index b9f62177c..c76492877 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java @@ -414,9 +414,9 @@ public void test009_readNamesInSection() throws ConfigInvalidException { names.contains("repositoryformatversion")); Iterator itr = names.iterator(); - assertEquals("repositoryFormatVersion", itr.next()); assertEquals("filemode", itr.next()); assertEquals("logAllRefUpdates", itr.next()); + assertEquals("repositoryFormatVersion", itr.next()); assertFalse(itr.hasNext()); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java index 58a03bf8b..1619b589c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java @@ -52,14 +52,9 @@ package org.eclipse.jgit.lib; import java.text.MessageFormat; -import java.util.AbstractSet; import java.util.ArrayList; import java.util.Collections; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; @@ -479,17 +474,22 @@ public String[] getStringList(final String section, String subsection, * section to search for. * @return set of all subsections of specified section within this * configuration and its base configuration; may be empty if no - * subsection exists. + * subsection exists. The set's iterator returns sections in the + * order they are declared by the configuration starting from this + * instance and progressing through the base. */ public Set getSubsections(final String section) { - return get(new SubsectionNames(section)); + return getState().getSubsections(section); } /** - * @return the sections defined in this {@link Config} + * @return the sections defined in this {@link Config}. The set's iterator + * returns sections in the order they are declared by the + * configuration starting from this instance and progressing through + * the base. */ public Set getSections() { - return get(new SectionNames()); + return getState().getSections(); } /** @@ -509,7 +509,7 @@ public Set getNames(String section) { * @return the list of names defined for this subsection */ public Set getNames(String section, String subsection) { - return get(new NamesInSection(section, subsection)); + return getState().getNames(section, subsection); } /** @@ -1243,144 +1243,6 @@ public static interface SectionParser { T parse(Config cfg); } - private static class SubsectionNames implements SectionParser> { - private final String section; - - SubsectionNames(final String sectionName) { - section = sectionName; - } - - public int hashCode() { - return section.hashCode(); - } - - public boolean equals(Object other) { - if (other instanceof SubsectionNames) { - return section.equals(((SubsectionNames) other).section); - } - return false; - } - - public Set parse(Config cfg) { - final Set result = new LinkedHashSet(); - while (cfg != null) { - for (final ConfigLine e : cfg.state.get().entryList) { - if (e.subsection != null && e.name == null - && StringUtils.equalsIgnoreCase(section, e.section)) - result.add(e.subsection); - } - cfg = cfg.baseConfig; - } - return Collections.unmodifiableSet(result); - } - } - - private static class NamesInSection implements SectionParser> { - private final String section; - - private final String subsection; - - NamesInSection(final String sectionName, final String subSectionName) { - section = sectionName; - subsection = subSectionName; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + section.hashCode(); - result = prime * result - + ((subsection == null) ? 0 : subsection.hashCode()); - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - NamesInSection other = (NamesInSection) obj; - if (!section.equals(other.section)) - return false; - if (subsection == null) { - if (other.subsection != null) - return false; - } else if (!subsection.equals(other.subsection)) - return false; - return true; - } - - public Set parse(Config cfg) { - final Map m = new LinkedHashMap(); - while (cfg != null) { - for (final ConfigLine e : cfg.state.get().entryList) { - if (e.name == null) - continue; - if (!StringUtils.equalsIgnoreCase(section, e.section)) - continue; - if ((subsection == null && e.subsection == null) - || (subsection != null && subsection - .equals(e.subsection))) { - String lc = StringUtils.toLowerCase(e.name); - if (!m.containsKey(lc)) - m.put(lc, e.name); - } - } - cfg = cfg.baseConfig; - } - return new CaseFoldingSet(m); - } - } - - private static class SectionNames implements SectionParser> { - public Set parse(Config cfg) { - final Map m = new LinkedHashMap(); - while (cfg != null) { - for (final ConfigLine e : cfg.state.get().entryList) { - if (e.section != null) { - String lc = StringUtils.toLowerCase(e.section); - if (!m.containsKey(lc)) - m.put(lc, e.section); - } - } - cfg = cfg.baseConfig; - } - return new CaseFoldingSet(m); - } - } - - private static class CaseFoldingSet extends AbstractSet { - private final Map names; - - CaseFoldingSet(Map names) { - this.names = Collections.unmodifiableMap(names); - } - - @Override - public boolean contains(Object needle) { - if (!(needle instanceof String)) - return false; - - String n = (String) needle; - return names.containsKey(n) - || names.containsKey(StringUtils.toLowerCase(n)); - } - - @Override - public Iterator iterator() { - return names.values().iterator(); - } - - @Override - public int size() { - return names.size(); - } - } - private static class StringReader { private final char[] buf; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigSnapshot.java index 5527267b8..cc5fe25e5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigSnapshot.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigSnapshot.java @@ -52,19 +52,29 @@ import static org.eclipse.jgit.util.StringUtils.compareIgnoreCase; import static org.eclipse.jgit.util.StringUtils.compareWithCase; +import static org.eclipse.jgit.util.StringUtils.toLowerCase; +import java.util.AbstractSet; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.eclipse.jgit.util.StringUtils; + class ConfigSnapshot { final List entryList; final Map cache; final ConfigSnapshot baseState; volatile List sorted; + volatile SectionNames names; ConfigSnapshot(List entries, ConfigSnapshot base) { entryList = entries; @@ -72,6 +82,40 @@ class ConfigSnapshot { baseState = base; } + Set getSections() { + return names().sections; + } + + Set getSubsections(String section) { + Map> m = names().subsections; + Set r = m.get(section); + if (r == null) + r = m.get(toLowerCase(section)); + if (r == null) + return Collections.emptySet(); + return Collections.unmodifiableSet(r); + } + + Set getNames(String section, String subsection) { + List s = sorted(); + int idx = find(s, section, subsection, ""); + if (idx < 0) + idx = -(idx + 1); + + Map m = new LinkedHashMap(); + while (idx < s.size()) { + ConfigLine e = s.get(idx++); + if (!e.match(section, subsection)) + break; + if (e.name == null) + continue; + String l = toLowerCase(e.name); + if (!m.containsKey(l)) + m.put(l, e.name); + } + return new CaseFoldingSet(m); + } + String[] get(String section, String subsection, String name) { List s = sorted(); int idx = find(s, section, subsection, name); @@ -167,4 +211,86 @@ public int compare(ConfigLine a, ConfigLine b) { b.section, b.subsection, b.name); } } + + private SectionNames names() { + SectionNames n = names; + if (n == null) + names = n = new SectionNames(this); + return n; + } + + private static class SectionNames { + final CaseFoldingSet sections; + final Map> subsections; + + SectionNames(ConfigSnapshot cfg) { + Map sec = new LinkedHashMap(); + Map> sub = new HashMap>(); + while (cfg != null) { + for (ConfigLine e : cfg.entryList) { + if (e.section == null) + continue; + + String l1 = toLowerCase(e.section); + if (!sec.containsKey(l1)) + sec.put(l1, e.section); + + if (e.subsection == null) + continue; + + Set m = sub.get(l1); + if (m == null) { + m = new LinkedHashSet(); + sub.put(l1, m); + } + m.add(e.subsection); + } + cfg = cfg.baseState; + } + + sections = new CaseFoldingSet(sec); + subsections = sub; + } + } + + private static class CaseFoldingSet extends AbstractSet { + private final Map names; + + CaseFoldingSet(Map names) { + this.names = names; + } + + @Override + public boolean contains(Object needle) { + if (needle instanceof String) { + String n = (String) needle; + return names.containsKey(n) + || names.containsKey(StringUtils.toLowerCase(n)); + } + return false; + } + + @Override + public Iterator iterator() { + final Iterator i = names.values().iterator(); + return new Iterator() { + public boolean hasNext() { + return i.hasNext(); + } + + public String next() { + return i.next(); + } + + public void remove() { + throw new UnsupportedOperationException(); + } + }; + } + + @Override + public int size() { + return names.size(); + } + } }