Bug 509898 - IndexFileSet.containsDeclaration is slow and is causing UI

freezes

Added a cache to IndexFileSet and remove a redundant cache from
CPPASTTranslationUnit.

Change-Id: Ifdd6037acf392ad11a4259f1de8cc51d5e153727

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTranslationUnit.java

@@ -32,9 +32,6 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunctionType;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPNamespace;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPNamespaceScope;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPParameter;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPSpecialization;
-import org.eclipse.cdt.core.index.IIndexBinding;
-import org.eclipse.cdt.core.index.IIndexFileSet;
 import org.eclipse.cdt.core.parser.ParserLanguage;
 import org.eclipse.cdt.core.parser.util.ArrayUtil;
 import org.eclipse.cdt.internal.core.dom.Linkage;
@@ -54,9 +51,8 @@ public class CPPASTTranslationUnit extends ASTTranslationUnit implements ICPPAST
 	private final CPPScopeMapper fScopeMapper;
 	private CPPASTAmbiguityResolver fAmbiguityResolver;
 
-	// Caches
+	// Caches.
 	private final Map<ICPPClassType, FinalOverriderMap> fFinalOverriderMapCache = new HashMap<>();
-	private final Map<IBinding, Boolean> fBindingReachabilityCache = new HashMap<>();
 
 	public CPPASTTranslationUnit() {
 		fScopeMapper= new CPPScopeMapper(this);
@@ -239,43 +235,4 @@ public class CPPASTTranslationUnit extends ASTTranslationUnit implements ICPPAST
 	public ICPPClassTemplatePartialSpecialization mapToAST(ICPPClassTemplatePartialSpecialization indexSpec) {
 		return fScopeMapper.mapToAST(indexSpec);
 	}
-
-	/**
-	 * Checks if a binding is an AST binding, or is reachable from the AST through includes.
-	 * The binding is assumed to belong to the AST, if it is not an IIndexBinding and not
-	 * a specialization of an IIndexBinding.
-	 *
-	 * @param binding
-	 * @return {@code true} if the {@code binding} is reachable from the AST.
-	 */
-	public boolean isReachableFromAst(IBinding binding) {
-		Boolean cachedValue = fBindingReachabilityCache.get(binding);
-		if (cachedValue != null)
-			return cachedValue;
-
-		IIndexBinding indexBinding = null;
-		if (binding instanceof IIndexBinding) {
-			indexBinding = (IIndexBinding) binding;
-		}
-		if (binding instanceof ICPPSpecialization) {
-			binding = ((ICPPSpecialization) binding).getSpecializedBinding();
-			if (binding instanceof IIndexBinding) {
-				indexBinding = (IIndexBinding) binding;
-			}
-		}
-		boolean reachable;
-		if (indexBinding == null) {
-			// We don't check if the binding really belongs to this AST assuming that
-			// the caller doesn't deal with two ASTs at a time.
-			reachable = true;
-		} else {
-			IIndexFileSet indexFileSet = getIndexFileSet();
-			IIndexFileSet astFileSet = getASTFileSet();
-			reachable = indexFileSet != null &&
-					(indexFileSet.containsDeclaration(indexBinding) ||
-					 astFileSet.containsDeclaration(indexBinding));
-		}
-		fBindingReachabilityCache.put(binding, reachable);
-		return reachable;
-	}
 }

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java

@@ -1115,7 +1115,7 @@ public class CPPSemantics {
 				if (item instanceof IBinding) {
 					IBinding binding = (IBinding) item;
 					CPPASTTranslationUnit tu = data.getTranslationUnit();
-					if (!isFromIndex(binding) || tu == null || tu.isReachableFromAst(binding)) {
+					if (!isFromIndex(binding) || tu == null || isReachableFromAst(tu, binding)) {
 						return true;
 					}
 				}
@@ -2289,7 +2289,7 @@ public class CPPSemantics {
 	 * the two bindings have the same relevance; -1 if <code>b1</code> is less relevant than
 	 * <code>b2</code>.
 	 */
-	static int compareByRelevance(CPPASTTranslationUnit tu, IBinding b1, IBinding b2) {
+	static int compareByRelevance(IASTTranslationUnit tu, IBinding b1, IBinding b2) {
 		boolean b1FromIndex= isFromIndex(b1);
 		boolean b2FromIndex= isFromIndex(b2);
 		if (b1FromIndex != b2FromIndex) {
@@ -2297,8 +2297,8 @@ public class CPPSemantics {
 		} else if (b1FromIndex) {
 			// Both are from index.
 			if (tu != null) {
-				boolean b1Reachable= tu.isReachableFromAst(b1);
+				boolean b1Reachable= isReachableFromAst(tu, b1);
-				boolean b2Reachable= tu.isReachableFromAst(b2);
+				boolean b2Reachable= isReachableFromAst(tu, b2);
 				if (b1Reachable != b2Reachable) {
 					return b1Reachable ? 1 : -1;
 				}
@@ -2317,7 +2317,7 @@ public class CPPSemantics {
 			return false;
 		final CPPASTTranslationUnit tu = data.getTranslationUnit();
 		if (tu != null) {
-			return !tu.isReachableFromAst(b2) && tu.isReachableFromAst(type);
+			return !isReachableFromAst(tu, b2) && isReachableFromAst(tu, type);
 		}
 		return false;
 	}
@@ -2338,12 +2338,12 @@ public class CPPSemantics {
 			}
 		}
 
-		if (!tu.isReachableFromAst(type)) {
+		if (!isReachableFromAst(tu, type)) {
 			return false;
 		}
 
 		for (IFunction fn : fns) {
-			if (tu.isReachableFromAst(fn)) {
+			if (isReachableFromAst(tu, fn)) {
 				return false;	// function from ast
 			}
 		}
@@ -2400,12 +2400,12 @@ public class CPPSemantics {
 			}
 			// Everything is from the index
 			final CPPASTTranslationUnit tu = data.getTranslationUnit();
-			if (!tu.isReachableFromAst(obj)) {
+			if (!isReachableFromAst(tu, obj)) {
 				return -1; // obj not reachable
 			}
 
 			for (IFunction fn : fns) {
-				if (tu.isReachableFromAst(fn)) {
+				if (isReachableFromAst(tu, fn)) {
 					return 0; // obj reachable, 1 function reachable
 				}
 			}
@@ -2433,12 +2433,43 @@ public class CPPSemantics {
 
 	/**
 	 * Checks if a binding is an AST binding, or is reachable from the AST through includes.
-	 * The binding is assumed to belong to the AST, if it is not an IIndexBinding and not
+	 * The binding is assumed to belong to the AST, if it is not an {@link IIndexBinding} and not
 	 * a specialization of an IIndexBinding.
 	 *
-	 * @param ast
+	 * @param ast the ast to check
-	 * @param binding
+	 * @param binding the binding to check
-	 * @return {@code true} if the {@code binding}> is reachable from {@code ast}.
+	 * @return {@code true} if the {@code binding}> is reachable from the {@code ast}
+	 */
+	private static boolean isReachableFromAst(IASTTranslationUnit ast, IBinding binding) {
+		IIndexBinding indexBinding = null;
+		if (binding instanceof IIndexBinding) {
+			indexBinding = (IIndexBinding) binding;
+		}
+		if (binding instanceof ICPPSpecialization) {
+			binding = ((ICPPSpecialization) binding).getSpecializedBinding();
+			if (binding instanceof IIndexBinding) {
+				indexBinding = (IIndexBinding) binding;
+			}
+		}
+		if (indexBinding == null) {
+			// We don't check if the binding really belongs to the AST specified by the ast
+			// parameter assuming that the caller doesn't deal with two ASTs at a time.
+			return true;
+		}
+		IIndexFileSet indexFileSet = ast.getIndexFileSet();
+		IIndexFileSet astFileSet = ast.getASTFileSet();
+		return indexFileSet != null &&
+				(indexFileSet.containsDeclaration(indexBinding) ||
+				 astFileSet.containsDeclaration(indexBinding));
+	}
+
+	/**
+	 * Checks if a name is an AST name, or is reachable from the AST through includes.
+	 * The name is assumed to belong to the AST, if it is not an {@link IIndexName}.
+	 *
+	 * @param ast the ast to check
+	 * @param name the name to check
+	 * @return {@code true} if the {@code name}> is reachable from the {@code ast}
 	 */
 	private static boolean isReachableFromAst(IASTTranslationUnit ast, IName name) {
 		if (!(name instanceof IIndexName)) {
@@ -2752,7 +2783,7 @@ public class CPPSemantics {
 		}
 
 		// Try to instantiate a template
-		CPPASTTranslationUnit tu= data.getTranslationUnit();
+		IASTTranslationUnit tu= data.getTranslationUnit();
 		ICPPTemplateArgument[] tmplArgs= ICPPTemplateArgument.EMPTY_ARGUMENTS;
 		if (templateID instanceof ICPPASTTemplateId) {
 			tmplArgs = CPPTemplates.createTemplateArgumentArray((ICPPASTTemplateId) templateID);
@@ -3325,7 +3356,7 @@ public class CPPSemantics {
 		ICPPFunction result= null;
 		ICPPFunctionTemplate resultTemplate= null;
 		boolean isAmbiguous= false;
-		final CPPASTTranslationUnit tu= (CPPASTTranslationUnit) point.getTranslationUnit();
+		final IASTTranslationUnit tu= point.getTranslationUnit();
 		for (IFunction fn : fns) {
 			try {
 				if (fn instanceof ICPPFunctionTemplate) {

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/FunctionCost.java

@@ -22,6 +22,7 @@ import java.util.Arrays;
 import org.eclipse.cdt.core.dom.ast.DOMException;
 import org.eclipse.cdt.core.dom.ast.IASTExpression.ValueCategory;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.dom.ast.IBinding;
 import org.eclipse.cdt.core.dom.ast.IFunction;
 import org.eclipse.cdt.core.dom.ast.IType;
@@ -31,7 +32,6 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunctionTemplate;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunctionType;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPSpecialization;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTranslationUnit;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.OverloadableOperator;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPTemplates.TypeSelection;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.Cost.DeferredUDC;
@@ -141,7 +141,7 @@ class FunctionCost {
 	/**
 	 * Compares this function call cost to another one.
 	 */
-	public int compareTo(CPPASTTranslationUnit tu, FunctionCost other) throws DOMException {
+	public int compareTo(IASTTranslationUnit tu, FunctionCost other) throws DOMException {
 		if (other == null)
 			return -1;
 

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IndexFileSet.java

@@ -31,7 +31,8 @@ import org.eclipse.core.runtime.CoreException;
 
 public class IndexFileSet implements IIndexFileSet {
 	private IIndexFileSet fInverse;
-	private HashMap<IIndexFragment, IIndexFragmentFileSet> fSubSets= new HashMap<>();
+	private final HashMap<IIndexFragment, IIndexFragmentFileSet> fSubSets= new HashMap<>();
+	private final Map<IBinding, Boolean> fDeclarationContainmentCache = new HashMap<>();
 
 	public IndexFileSet() {
 	}
@@ -46,6 +47,7 @@ public class IndexFileSet implements IIndexFileSet {
 			fSubSets.put(frag, subSet);
 		}
 		subSet.add(fragFile);
+		fDeclarationContainmentCache.clear();
 	}
 
 	@Override
@@ -55,11 +57,16 @@ public class IndexFileSet implements IIndexFileSet {
 		IIndexFragmentFileSet subSet = fSubSets.get(fragment);		
 		if (subSet != null) {
 			subSet.remove(fragmentFile);
+			fDeclarationContainmentCache.clear();
 		}
 	}
 
 	@Override
 	public boolean containsDeclaration(IIndexBinding binding) {
+		Boolean cachedValue = fDeclarationContainmentCache.get(binding);
+		if (cachedValue != null)
+			return cachedValue;
+
 		for (Map.Entry<IIndexFragment, IIndexFragmentFileSet> entry : fSubSets.entrySet()) {
 			IIndexFragment fragment = entry.getKey();
 			IIndexFragmentFileSet fragmentFileSet = entry.getValue();
@@ -72,14 +79,17 @@ public class IndexFileSet implements IIndexFileSet {
 					long nameRecord;
 					while ((nameRecord = nameIterator.next()) != 0) {
 						long fileRecord = PDOMName.getFileRecord(db, nameRecord);
-						if (pdomFileSet.containsFile(fileRecord))
+						if (pdomFileSet.containsFile(fileRecord)) {
+							fDeclarationContainmentCache.put(binding, true);
 							return true;
 						}
 					}
+				}
 			} catch (CoreException e) {
 				CCorePlugin.log(e);
 			}
 		}
+		fDeclarationContainmentCache.put(binding, false);
 		return false;
 	}
 
@@ -223,6 +233,7 @@ public class IndexFileSet implements IIndexFileSet {
 				public boolean containsDeclaration(IIndexBinding binding) {
 					throw new UnsupportedOperationException();
 				}
+
 				@Override
 				public boolean containsNonLocalDeclaration(IBinding binding, IIndexFragment ignore) {
 					throw new UnsupportedOperationException();