BUG-582: Sort modules on initialization

Rather than sorting them each time they are accessed, pay the price
upfront and create a sorted view of modules.

Change-Id: Ib8e02c930f936bf1354d1f7849a72ef8f8698625
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/SchemaContextImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/SchemaContextImpl.java
index c689fa9011725cb241c27d5c86c5a3e18423706f..201ab916185d417cf212a005434fbb1cea191297 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/SchemaContextImpl.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/SchemaContextImpl.java
@@ -39,6 +39,7 @@ import org.opendaylight.yangtools.yang.parser.util.ModuleDependencySort;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Multimaps;
 import com.google.common.collect.SetMultimap;
@@ -56,9 +57,14 @@ final class SchemaContextImpl implements SchemaContext {
     private final Set<Module> modules;
 
     SchemaContextImpl(final Set<Module> modules, final Map<ModuleIdentifier, String> identifiersToSources) {
-        this.modules = modules;
         this.identifiersToSources = identifiersToSources;
 
+        /*
+         * Instead of doing this on each invocation of getModules(), pre-compute
+         * it once and keep it around -- better than the set we got in.
+         */
+        this.modules = ImmutableSet.copyOf(ModuleDependencySort.sort(modules.toArray(new Module[modules.size()])));
+
         /*
          * The most common lookup is from Namespace->Module. Invest some quality time in
          * building that up.
@@ -83,9 +89,7 @@ final class SchemaContextImpl implements SchemaContext {
 
     @Override
     public Set<Module> getModules() {
-        // FIXME: can we pre-compute this in the constructor?
-        List<Module> sorted = ModuleDependencySort.sort(modules.toArray(new Module[modules.size()]));
-        return new LinkedHashSet<Module>(sorted);
+        return modules;
     }
 
     @Override
@@ -139,23 +143,28 @@ final class SchemaContextImpl implements SchemaContext {
 
     @Override
     public Module findModuleByNamespaceAndRevision(final URI namespace, final Date revision) {
-        if (namespace != null) {
-            Set<Module> modules = findModuleByNamespace(namespace);
+        if (namespace == null) {
+            return null;
+        }
+        final Set<Module> modules = findModuleByNamespace(namespace);
+        if (modules.isEmpty()) {
+            return null;
+        }
 
-            if (revision == null) {
-                TreeMap<Date, Module> map = new TreeMap<Date, Module>();
-                for (Module module : modules) {
-                    map.put(module.getRevision(), module);
-                }
-                if (map.isEmpty()) {
-                    return null;
-                }
-                return map.lastEntry().getValue();
-            } else {
-                for (Module module : modules) {
-                    if (module.getRevision().equals(revision)) {
-                        return(module);
-                    }
+        if (revision == null) {
+            // FIXME: The ordering of modules in Multimap could just guarantee this...
+            TreeMap<Date, Module> map = new TreeMap<Date, Module>();
+            for (Module module : modules) {
+                map.put(module.getRevision(), module);
+            }
+            if (map.isEmpty()) {
+                return null;
+            }
+            return map.lastEntry().getValue();
+        } else {
+            for (Module module : modules) {
+                if (module.getRevision().equals(revision)) {
+                    return(module);
                 }
             }
         }