From 2647895966e37cc80cb145178425d49c09909c05 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Fri, 27 Jan 2017 19:44:16 -0800 Subject: [PATCH] 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 --- .../dom/parser/cpp/CPPASTTranslationUnit.java | 47 +------------- .../parser/cpp/semantics/CPPSemantics.java | 61 ++++++++++++++----- .../parser/cpp/semantics/FunctionCost.java | 4 +- .../cdt/internal/core/index/IndexFileSet.java | 17 +++++- 4 files changed, 64 insertions(+), 65 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTranslationUnit.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTranslationUnit.java index 0bb7758f146..5e7c40235a1 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTranslationUnit.java +++ b/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 fFinalOverriderMapCache = new HashMap<>(); - private final Map fBindingReachabilityCache = new HashMap<>(); public CPPASTTranslationUnit() { fScopeMapper= new CPPScopeMapper(this); @@ -235,47 +231,8 @@ public class CPPASTTranslationUnit extends ASTTranslationUnit implements ICPPAST ICPPClassTemplatePartialSpecialization astSpec) { fScopeMapper.recordPartialSpecialization(indexSpec, astSpec); } - + 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; - } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java index 6ce4dd118b9..da08043efe4 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java +++ b/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 b1 is less relevant than * b2. */ - 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 b2Reachable= tu.isReachableFromAst(b2); + boolean b1Reachable= isReachableFromAst(tu, b1); + 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 binding - * @return {@code true} if the {@code binding}> is reachable from {@code ast}. + * @param ast the ast to check + * @param binding the binding to check + * @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) { diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/FunctionCost.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/FunctionCost.java index 6f116871bfd..53bd9d323a8 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/FunctionCost.java +++ b/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; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IndexFileSet.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IndexFileSet.java index 64a4b098363..b5cf0b4e2b5 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IndexFileSet.java +++ b/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 fSubSets= new HashMap<>(); + private final HashMap fSubSets= new HashMap<>(); + private final Map fDeclarationContainmentCache = new HashMap<>(); public IndexFileSet() { } @@ -40,12 +41,13 @@ public class IndexFileSet implements IIndexFileSet { public void add(IIndexFile indexFile) { final IIndexFragmentFile fragFile = (IIndexFragmentFile) indexFile; final IIndexFragment frag= fragFile.getIndexFragment(); - IIndexFragmentFileSet subSet= fSubSets.get(frag); + IIndexFragmentFileSet subSet= fSubSets.get(frag); if (subSet == null) { subSet= frag.createFileSet(); 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 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();