Merge pull request #2441 from usethesource/feat/reduce-module-environment-lookup

Add loopup caches in ModuleEnvironment to improve execution speed

Author: DavyLandman

src/org/rascalmpl/interpreter/Evaluator.java

@@ -372,6 +372,7 @@ public void notifyConstructorDeclaredListeners() {
             }
         }
         constructorDeclaredListeners.clear();
+        getHeap().clearLookupChaches();
     }
 
     @Override

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

@@ -95,6 +95,7 @@ public void registerSourceResolver(String scheme, ICallableValue function) {
 	 */
 	public ModuleEnvironment addModule(ModuleEnvironment mod) {
 		assert mod != null;
+		clearLookupChaches();
 		ModuleEnvironment env = moduleEnvironment.get(mod.getName());
 		if (env == null) {
 			moduleEnvironment.put(mod.getName(), mod);
@@ -111,6 +112,7 @@ else if (env == mod) {
 	public ModuleEnvironment resetModule(String name) {
 		ModuleEnvironment mod = moduleEnvironment.get(name);
 		mod.reset();
+		clearLookupChaches();
 		return mod;
 	}
 
@@ -146,7 +148,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.removeModule(name);
+		}
 	}
 
 	public void setModuleURI(String name, URI location) {
@@ -237,4 +243,8 @@ public boolean isBootstrapper() {
 	public List<String> getExtendCycle() {
         return Collections.unmodifiableList(extendStack.stream().collect(Collectors.toList()));
 	}
+
+	public void clearLookupChaches() {
+		moduleEnvironment.values().forEach(ModuleEnvironment::clearLookupCaches);
+	}
 }

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

@@ -17,14 +17,18 @@
 *******************************************************************************/
 package org.rascalmpl.interpreter.env;
 
+import java.util.ArrayList;
 import java.util.Collection;
 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 +71,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;
@@ -79,6 +84,7 @@ public class ModuleEnvironment extends Environment {
 	private String deprecated;
 	protected Map<String, AbstractFunction> resourceImporters;
 	protected Map<Type, Set<GenericKeywordParameters>> cachedGeneralKeywordParameters;
+	protected Map<String, List<AbstractFunction>> cachedPublicFunctions;
 
 	protected static final TypeFactory TF = TypeFactory.getInstance();
 
@@ -87,7 +93,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>>();
@@ -97,6 +103,7 @@ public ModuleEnvironment(String name, GlobalEnvironment heap) {
 		this.bootstrap = false;
 		this.resourceImporters = new HashMap<String, AbstractFunction>();
 		this.cachedGeneralKeywordParameters = null;
+		this.cachedPublicFunctions = null;
 	}
 
 	/**
@@ -116,13 +123,14 @@ protected ModuleEnvironment(ModuleEnvironment env) {
 		this.bootstrap = env.bootstrap;
 		this.resourceImporters = env.resourceImporters;
 		this.cachedGeneralKeywordParameters = null;
+		this.cachedPublicFunctions = null;
 		this.deprecated = env.deprecated;
 	}
 
 	@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>();
@@ -133,6 +141,13 @@ public void reset() {
 		this.deprecated = null;
 		this.generalKeywordParameters = new HashMap<>();
 		this.cachedGeneralKeywordParameters = null;
+		this.cachedPublicFunctions = null;
+	}
+
+	public void clearLookupCaches() {
+		importedModules.replaceAll((k, v) -> Optional.empty());
+		cachedGeneralKeywordParameters = null;
+		cachedPublicFunctions = null;
 	}
 
 	public void extend(ModuleEnvironment other) {
@@ -142,9 +157,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 +309,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,15 +342,25 @@ 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);
+		this.cachedGeneralKeywordParameters = null;
+		this.cachedPublicFunctions = null;
+	}
+
+	void removeModule(String name) {
+		importedModules.computeIfPresent(name, (k, v) -> Optional.empty());
+		this.cachedGeneralKeywordParameters = null;
+		this.cachedPublicFunctions = null;
 	}
 
 	public void addExtend(String name) {
 		if (extended == null) {
 			extended = new HashSet<String>();
 		}
 		extended.add(name);
+		this.cachedGeneralKeywordParameters = null;
+		this.cachedPublicFunctions = null;
 	}
 
 	public List<AbstractFunction> getTests() {
@@ -356,7 +381,7 @@ public List<AbstractFunction> getTests() {
 
 	@Override
 	public Set<String> getImports() {
-		return Collections.unmodifiableSet(importedModules);
+		return Collections.unmodifiableSet(importedModules.keySet());
 	}
 
 	public Set<String> getImportsTransitive() {
@@ -384,18 +409,18 @@ 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;
+		cachedPublicFunctions = null;
 	}
 
 	public void unExtend(String moduleName) {
 		extended.remove(moduleName);
 		cachedGeneralKeywordParameters = null;
+		cachedPublicFunctions = null;
 	}
 
 	@Override
@@ -447,6 +472,7 @@ public Result<IValue> getVariable(QualifiedName name) {
 
 		return getFrameVariable(cons);
 	}
+
 
 	@Override
 	public void storeVariable(String name, Result<IValue> value) {
@@ -461,8 +487,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 +508,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 +535,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);
@@ -527,32 +550,36 @@ protected Map<String,Result<IValue>> getVariableDefiningEnvironment(String name)
 
 	@Override
 	public void getAllFunctions(String name, List<AbstractFunction> collection) {
-		super.getAllFunctions(name, collection);
-		
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
+		collection.addAll(lookupCachedFunctions(name));
+	}
+
+	private List<AbstractFunction> lookupCachedFunctions(String name) {
+		if (cachedPublicFunctions == null) {
+			cachedPublicFunctions = io.usethesource.capsule.Map.Transient.of();
+		}
+		return cachedPublicFunctions.computeIfAbsent(name, n -> {
+			var result = new ArrayList<AbstractFunction>();
+			super.getAllFunctions(n, result);
 
-			if (mod != null) {
-			  mod.getLocalPublicFunctions(name, collection);
+			for (ModuleEnvironment mod : importedModulesResolved) {
+				
+				if (mod != null) {
+					mod.getLocalPublicFunctions(n, result);
+				}
 			}
-		}
+			return result;
+		});
 	}
 
 	@Override
 	public void getAllFunctions(Type returnType, String name, List<AbstractFunction> collection) {
-		super.getAllFunctions(returnType, name, collection);
-		
-		for (String moduleName : importedModules) {
-			ModuleEnvironment mod = getImport(moduleName);
-			
-			if (mod != null) {
-			  mod.getLocalPublicFunctions(returnType, name, collection);
+		for (var function: lookupCachedFunctions(name)) {
+			if (function.getReturnType().comparable(returnType)) {
+				collection.add(function);
 			}
 		}
 	}
 
-
-	
 
 	private Result<IValue> getLocalPublicVariable(String name) {
 		Result<IValue> var = null;
@@ -578,20 +605,6 @@ private void getLocalPublicFunctions(String name, List<AbstractFunction> collect
 			}
 		}
 	}
-	
-	private void getLocalPublicFunctions(Type returnType, String name, List<AbstractFunction> collection) {
-		if (functionEnvironment != null && !isNamePrivate(name)) {
-			LinkedHashSet<AbstractFunction> lst = functionEnvironment.get(name);
-			
-			if (lst != null) {
-				for (AbstractFunction func : lst) {
-					if (returnType.isSubtypeOf(func.getReturnType())) {
-						collection.add(func);
-					}
-				}
-			}
-		}
-	}
 
 	@Override
 	public Type abstractDataType(String name, Type... parameters) {
@@ -732,8 +745,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 +837,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 +902,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 +1020,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");
 			}
@@ -1081,4 +1111,5 @@ public void resetProductions() {
 	public Set<IValue> getProductions() {
 		return productions;
 	}
+
 }