From 45101e2a64fb806be7bacaca650104be53702652 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sun, 8 Oct 2017 18:14:02 -0400 Subject: [PATCH] Bug 512297 - Improve caching of type strings in ASTTypeUtil Previously, caching of type strings would only be done during indexing. Now, it is done every time an AST is available, including operations like semantic highlighting and mark occurrences. This is important, because without caching, ASTTypeUtil can end up rebuilding the same type string many times, leading to quadratic performance on some code patterns. Change-Id: I260877f820665cbe0939c0c3065514559592e721 --- .../eclipse/cdt/core/dom/ast/ASTTypeUtil.java | 65 +++++++------------ .../core/dom/parser/ASTTranslationUnit.java | 10 +++ .../core/pdom/AbstractIndexerTask.java | 4 -- 3 files changed, 32 insertions(+), 47 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/ASTTypeUtil.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/ASTTypeUtil.java index 99255858b9b..006b6955adc 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/ASTTypeUtil.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/ASTTypeUtil.java @@ -19,8 +19,8 @@ import java.util.ArrayList; import java.util.BitSet; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; -import java.util.WeakHashMap; import org.eclipse.cdt.core.CCorePlugin; import org.eclipse.cdt.core.dom.ast.IBasicType.Kind; @@ -46,6 +46,7 @@ import org.eclipse.cdt.core.parser.GCCKeywords; import org.eclipse.cdt.core.parser.Keywords; import org.eclipse.cdt.core.parser.util.ArrayUtil; import org.eclipse.cdt.internal.core.dom.parser.ASTInternal; +import org.eclipse.cdt.internal.core.dom.parser.ASTTranslationUnit; import org.eclipse.cdt.internal.core.dom.parser.ITypeContainer; import org.eclipse.cdt.internal.core.dom.parser.IntegralValue; import org.eclipse.cdt.internal.core.dom.parser.c.CASTTypeId; @@ -55,6 +56,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTypeId; import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPDeferredClassInstance; import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPInternalBinding; import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPUnknownMemberClassInstance; +import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.TypeOfDependentExpression; @@ -75,36 +77,6 @@ public class ASTTypeUtil { private static final String SPACE = " "; //$NON-NLS-1$ private static final int DEAULT_ITYPE_SIZE = 2; - private static class ResultCache { - // Keep two separate maps for normalized and unnormalized type representations. - private WeakHashMap normalizedTypes = new WeakHashMap<>(); - private WeakHashMap unnormalizedTypes = new WeakHashMap<>(); - - /** - * Returns the cached string representation for a type. Returns {@code null} - * if the value was not cached or was garbage collected. - */ - public String get(IType type, boolean normalize) { - if (normalize) { - return normalizedTypes.get(type); - } else { - return unnormalizedTypes.get(type); - } - } - - /** - * Stores a string representation of the given type name in the cache. - */ - public String put(IType type, boolean normalize, String result) { - if (normalize) { - return normalizedTypes.put(type, result); - } else { - return unnormalizedTypes.put(type, result); - } - } - } - - private static final ThreadLocal resultCache = new ThreadLocal<>(); private static final ThreadLocal> fSourceFileOnlyCheckInProgress= new ThreadLocal>() { @Override @@ -234,7 +206,7 @@ public class ASTTypeUtil { appendArgumentList(args, normalize, result); return result.toString(); } - + private static void appendArgumentList(ICPPTemplateArgument[] args, boolean normalize, StringBuilder result) { result.append('<'); for (int i = 0; i < args.length; i++) { @@ -577,15 +549,26 @@ public class ASTTypeUtil { return result.toString(); } + private static Map getTypeStringCache(boolean normalize) { + IASTNode lookupPoint = CPPSemantics.getCurrentLookupPoint(); + if (lookupPoint != null) { + IASTTranslationUnit tu = lookupPoint.getTranslationUnit(); + if (tu instanceof ASTTranslationUnit) { + return ((ASTTranslationUnit) tu).getTypeStringCache(normalize); + } + } + return null; + } + /** * Appends the the result of {@link #getType(IType, boolean)} to the given buffer. * @since 5.3 */ public static void appendType(IType type, boolean normalize, StringBuilder result) { // performance: check if type was appended before - ResultCache cache = resultCache.get(); + Map cache = getTypeStringCache(normalize); if (cache != null) { - String cachedResult = cache.get(type, normalize); + String cachedResult = cache.get(type); if (cachedResult != null) { result.append(cachedResult); return; @@ -721,7 +704,7 @@ public class ASTTypeUtil { if (cache != null) { // Store result in the cache. - cache.put(originalType, normalize, result.substring(startOffset)); + cache.put(originalType, result.substring(startOffset)); } } @@ -987,22 +970,18 @@ public class ASTTypeUtil { } /** - * Marks start of processing a translation unit during indexing. - * Enables caching of string representations of types. - * * @noreference This method is not intended to be referenced by clients. + * @deprecated This method is no longer used and is scheduled for removal in 10.0. */ + @Deprecated public static void startTranslationUnit() { - resultCache.set(new ResultCache()); } /** - * Marks the end of processing a translation unit during indexing. - * Disables caching of string representations of types and clears the previously cached results. - * * @noreference This method is not intended to be referenced by clients. + * @deprecated This method is no longer used and is scheduled for removal in 10.0. */ + @Deprecated public static void finishTranslationUnit() { - resultCache.set(null); } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTTranslationUnit.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTTranslationUnit.java index 0e35b180428..f5b1ea5dca5 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTTranslationUnit.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTTranslationUnit.java @@ -14,6 +14,8 @@ package org.eclipse.cdt.internal.core.dom.parser; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; @@ -88,6 +90,10 @@ public abstract class ASTTranslationUnit extends ASTNode implements IASTTranslat private boolean fBasedOnIncompleteIndex; private boolean fNodesOmitted; private IBuiltinBindingsProvider fBuiltinBindingsProvider; + + // Caches + private final WeakHashMap fUnnormalizedTypeStringCache = new WeakHashMap<>(); + private final WeakHashMap fNormalizedTypeStringCache = new WeakHashMap<>(); @Override public final IASTTranslationUnit getTranslationUnit() { @@ -571,4 +577,8 @@ public abstract class ASTTranslationUnit extends ASTNode implements IASTTranslat } return false; } + + public Map getTypeStringCache(boolean normalized) { + return normalized ? fNormalizedTypeStringCache : fUnnormalizedTypeStringCache; + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/AbstractIndexerTask.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/AbstractIndexerTask.java index 436cb6a704d..212e52bc848 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/AbstractIndexerTask.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/AbstractIndexerTask.java @@ -36,7 +36,6 @@ import java.util.regex.Pattern; import org.eclipse.cdt.core.CCorePlugin; import org.eclipse.cdt.core.dom.ILinkage; import org.eclipse.cdt.core.dom.IPDOMIndexerTask; -import org.eclipse.cdt.core.dom.ast.ASTTypeUtil; import org.eclipse.cdt.core.dom.ast.IASTComment; import org.eclipse.cdt.core.dom.ast.IASTFileLocation; import org.eclipse.cdt.core.dom.ast.IASTPreprocessorIncludeStatement; @@ -1088,7 +1087,6 @@ public abstract class AbstractIndexerTask extends PDOMWriter { FileContent codeReader= fResolver.getCodeReader(tu); long start= System.currentTimeMillis(); - ASTTypeUtil.startTranslationUnit(); IASTTranslationUnit ast= createAST(lang, codeReader, scanInfo, fASTOptions, ctx, progress.split(10)); fStatistics.fParsingTime += System.currentTimeMillis() - start; @@ -1114,8 +1112,6 @@ public abstract class AbstractIndexerTask extends PDOMWriter { if (--fSwallowOutOfMemoryError < 0) throw e; th= e; - } finally { - ASTTypeUtil.finishTranslationUnit(); } if (th != null) { swallowError(path, th);