Reduce double map iteration of imported modules that need to be looked up in the heap everytime

Author: DavyLandman

src/org/rascalmpl/interpreter/env/GlobalEnvironment.java

@@ -146,7 +146,11 @@ public String toString(){
 	}
 
 	public void removeModule(ModuleEnvironment env) {
-		moduleEnvironment.remove(env.getName());
+		var name = env.getName();
+		moduleEnvironment.remove(name);
+		for (var mod : moduleEnvironment.values()) {
+			mod.deleteImport(name);
+		}
 	}
 
 	public void setModuleURI(String name, URI location) {

src/org/rascalmpl/interpreter/env/ModuleEnvironment.java

@@ -21,10 +21,13 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 
@@ -67,7 +70,8 @@
  */
 public class ModuleEnvironment extends Environment {
 	protected final GlobalEnvironment heap;
-	protected Set<String> importedModules;
+	/** Map of imported modules to resolved ModuleEnvironments, will only be empty in case of a module was in a cycle or got reloaded and failed during the reload. use {@link #importedModulesResolved} to lazily resolve the modules. */
+	protected Map<String, Optional<ModuleEnvironment>> importedModules;
 	protected Set<String> extended;
 	protected TypeStore typeStore;
 	protected Set<IValue> productions;
@@ -87,7 +91,7 @@ public class ModuleEnvironment extends Environment {
 	public ModuleEnvironment(String name, GlobalEnvironment heap) {
 		super(ValueFactoryFactory.getValueFactory().sourceLocation(URIUtil.assumeCorrect("main", name, "")), name);
 		this.heap = heap;
-		this.importedModules = new HashSet<String>();
+		this.importedModules = new HashMap<>();
 		this.concreteSyntaxTypes = new HashMap<String, NonTerminalType>();
 		this.productions = new HashSet<IValue>();
 		this.generalKeywordParameters = new HashMap<Type,List<KeywordFormal>>();
@@ -122,7 +126,7 @@ protected ModuleEnvironment(ModuleEnvironment env) {
 	@Override
 	public void reset() {
 		super.reset();
-		this.importedModules = new HashSet<>();
+		this.importedModules = new HashMap<>();
 		this.concreteSyntaxTypes = new HashMap<>();
 		this.typeStore = new TypeStore();
 		this.productions = new HashSet<IValue>();
@@ -142,9 +146,9 @@ public void extend(ModuleEnvironment other) {
       // so that types become available
 	  if (other.importedModules != null) {
 	    if (this.importedModules == null) {
-	      this.importedModules = new HashSet<String>();
+	      this.importedModules = new HashMap<>();
 	    }
-	    this.importedModules.addAll(other.importedModules);
+	    this.importedModules.putAll(other.importedModules);
 	  }
 
 	  if (other.concreteSyntaxTypes != null) {
@@ -294,7 +298,7 @@ public IMap getSyntaxDefinition() {
 				result.put(VF.string(m), t);
 			}else if(m.equals(getName())) { // This is the root scope.
 				ISetWriter importWriter = VF.setWriter();
-				for(String impname : importedModules){
+				for(String impname : importedModules.keySet()){
 					if(!done.contains(impname)) todo.add(impname);
 
 					importWriter.insert(VF.string(impname));
@@ -327,9 +331,13 @@ public boolean isModuleEnvironment() {
 
 	public void addImport(String name, ModuleEnvironment env) {
 		assert heap.getModule(name).equals(env);
-		importedModules.add(name);
+		importedModules.put(name, Optional.ofNullable(env));
 		typeStore.importStore(env.typeStore);
 	}
+
+	void deleteImport(String name) {
+		importedModules.computeIfPresent(name, (k, v) -> Optional.empty());
+	}
 
 	public void addExtend(String name) {
 		if (extended == null) {
@@ -356,7 +364,7 @@ public List<AbstractFunction> getTests() {
 
 	@Override
 	public Set<String> getImports() {
-		return Collections.unmodifiableSet(importedModules);
+		return Collections.unmodifiableSet(importedModules.keySet());
 	}
 
 	public Set<String> getImportsTransitive() {
@@ -384,11 +392,9 @@ public Set<String> getImportsTransitive() {
 	}
 
 	public void unImport(String moduleName) {
-		if (importedModules.remove(moduleName)) {
-			ModuleEnvironment old = heap.getModule(moduleName);
-			if (old != null) {
-				typeStore.unimportStores(new TypeStore[] { old.getStore() });
-			}
+		var old = importedModules.remove(moduleName);
+		if (old != null && old.isPresent()) {
+			typeStore.unimportStores(old.get().getStore());
 		}
 		cachedGeneralKeywordParameters = null;
 	}
@@ -447,6 +453,7 @@ public Result<IValue> getVariable(QualifiedName name) {
 
 		return getFrameVariable(cons);
 	}
+
 
 	@Override
 	public void storeVariable(String name, Result<IValue> value) {
@@ -461,8 +468,7 @@ public void storeVariable(String name, Result<IValue> value) {
 			super.storeVariable(name, value);
 		}
 		else {
-			for (String i : importedModules) {
-				ModuleEnvironment module = heap.getModule(i);
+			for (ModuleEnvironment module : importedModulesResolved) {
 				result = module.getLocalPublicVariable(name);
 
 				if (result != null) {
@@ -483,8 +489,7 @@ public org.rascalmpl.interpreter.result.Result<IValue> getSimpleVariable(String
 			return var;
 		}
 
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
+		for (ModuleEnvironment mod : importedModulesResolved) {
 
 			if (mod != null) { 
 			  var = mod.getLocalPublicVariable(name);
@@ -511,8 +516,7 @@ protected Map<String,Result<IValue>> getVariableDefiningEnvironment(String name)
 			}
 		}
 
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
+		for (ModuleEnvironment mod : importedModulesResolved) {
 			Result<IValue> r = null;
 			if (mod != null && mod.variableEnvironment != null) 
 				r = mod.variableEnvironment.get(name);
@@ -529,8 +533,7 @@ protected Map<String,Result<IValue>> getVariableDefiningEnvironment(String name)
 	public void getAllFunctions(String name, List<AbstractFunction> collection) {
 		super.getAllFunctions(name, collection);
 
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
+		for (ModuleEnvironment mod : importedModulesResolved) {
 
 			if (mod != null) {
 			  mod.getLocalPublicFunctions(name, collection);
@@ -542,8 +545,7 @@ public void getAllFunctions(String name, List<AbstractFunction> collection) {
 	public void getAllFunctions(Type returnType, String name, List<AbstractFunction> collection) {
 		super.getAllFunctions(returnType, name, collection);
 
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
+		for (ModuleEnvironment mod : importedModulesResolved) {
 
 			if (mod != null) {
 			  mod.getLocalPublicFunctions(returnType, name, collection);
@@ -732,8 +734,7 @@ public Set<GenericKeywordParameters> lookupGenericKeywordParameters(Type adt) {
 			result.add(new GenericKeywordParameters(this, list, getStore().getKeywordParameters(adt)));
 		}
 
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
+		for (ModuleEnvironment mod : importedModulesResolved) {
 
 			list = mod.generalKeywordParameters.get(adt);
 			if (list != null) {
@@ -825,13 +826,33 @@ public String toString() {
 
 	@Override
 	public ModuleEnvironment getImport(String moduleName) {
-		if(importedModules.contains(moduleName)) {
-			return heap.getModule(moduleName);
-		}
-		else {
-			return null;    
+		var result = importedModules.computeIfPresent(moduleName,
+			(m, c) -> c.isPresent() ? c : Optional.ofNullable(heap.getModule(m))
+		);
+		if (result == null || result.isEmpty()) {
+			return null;
 		}
+		return result.get();
 	}
+
+	private Iterable<ModuleEnvironment> importedModulesResolved = 
+		() -> new Iterator<ModuleEnvironment>() {
+			Iterator<Entry<String, Optional<ModuleEnvironment>>> iterator = importedModules.entrySet().iterator();
+			@Override
+			public boolean hasNext() {
+				return iterator.hasNext();
+			}
+			@Override
+			public ModuleEnvironment next() {
+				var entry = iterator.next();
+				var result = entry.getValue();
+				if (result.isEmpty()) {
+					result = Optional.ofNullable(heap.getModule(entry.getKey()));
+					entry.setValue(result);
+				}
+				return result.orElse(null);
+			}
+		};
 
 	@Override
 	public void storeVariable(QualifiedName name, Result<IValue> result) {
@@ -870,8 +891,7 @@ public Type lookupConcreteSyntaxType(String name) {
 		Type type = concreteSyntaxTypes.get(name);
 
 		if (type == null) {
-			for (String i : importedModules) {
-				ModuleEnvironment mod = getImport(i);
+			for (ModuleEnvironment mod : importedModulesResolved) {
 
 				if (mod == null) {
 				  continue;
@@ -989,8 +1009,7 @@ protected Environment getFlagsEnvironment(String name) {
 			return env;
 		}
 
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
+		for (ModuleEnvironment mod : importedModulesResolved) {
 			if(mod == null)	{
 				throw new RuntimeException("getFlagsEnvironment");
 			}