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

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<IType, String> normalizedTypes = new WeakHashMap<>();
-		private WeakHashMap<IType, String> 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> resultCache = new ThreadLocal<>();
 	private static final ThreadLocal<Set<IBinding>> fSourceFileOnlyCheckInProgress=
 			new ThreadLocal<Set<IBinding>>() {
 		@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<IType, String> 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<IType, String> 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<IType, String> fUnnormalizedTypeStringCache = new WeakHashMap<>();
+	private final WeakHashMap<IType, String> fNormalizedTypeStringCache = new WeakHashMap<>();
 
 	@Override
 	public final IASTTranslationUnit getTranslationUnit() {
@@ -571,4 +577,8 @@ public abstract class ASTTranslationUnit extends ASTNode implements IASTTranslat
 		}
 		return false;
 	}
+	
+	public Map<IType, String> 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);