changeset 3043:37b60162a2db

8137269: Add better support for local caching in ArgumentAttr Summary: ArgumentAttr should support local caches when results of speculative attribution might be thrown away Reviewed-by: jlahoda
author mcimadamore
date Fri, 02 Oct 2015 13:27:57 +0100
parents e6fcc24b6d14
children d034f4347b09
files src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
diffstat 3 files changed, 37 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java	Thu Oct 01 19:47:06 2015 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java	Fri Oct 02 13:27:57 2015 +0100
@@ -29,6 +29,7 @@
 import com.sun.tools.javac.code.Source;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
+import com.sun.tools.javac.comp.ArgumentAttr.LocalCacheContext;
 import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.JCTree.JCBlock;
 import com.sun.tools.javac.tree.JCTree.JCClassDecl;
@@ -54,7 +55,6 @@
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.DefinedBy;
 import com.sun.tools.javac.util.DefinedBy.Api;
-import com.sun.tools.javac.util.Filter;
 import com.sun.tools.javac.util.JCDiagnostic;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticType;
 import com.sun.tools.javac.util.List;
@@ -72,7 +72,6 @@
 import static com.sun.tools.javac.code.Flags.SYNTHETIC;
 import static com.sun.tools.javac.code.TypeTag.CLASS;
 import static com.sun.tools.javac.tree.JCTree.Tag.APPLY;
-import static com.sun.tools.javac.tree.JCTree.Tag.CLASSDEF;
 import static com.sun.tools.javac.tree.JCTree.Tag.METHODDEF;
 import static com.sun.tools.javac.tree.JCTree.Tag.NEWCLASS;
 import static com.sun.tools.javac.tree.JCTree.Tag.TYPEAPPLY;
@@ -88,6 +87,7 @@
     final Log log;
     final Attr attr;
     final DeferredAttr deferredAttr;
+    final ArgumentAttr argumentAttr;
     final TreeMaker make;
     final Names names;
     private final boolean allowDiamondWithAnonymousClassCreation;
@@ -107,6 +107,7 @@
         log = Log.instance(context);
         attr = Attr.instance(context);
         deferredAttr = DeferredAttr.instance(context);
+        argumentAttr = ArgumentAttr.instance(context);
         make = TreeMaker.instance(context);
         names = Names.instance(context);
         Options options = Options.instance(context);
@@ -363,8 +364,13 @@
 
             TreeMapper treeMapper = new TreeMapper(context);
             //TODO: to further refine the analysis, try all rewriting combinations
-            deferredAttr.attribSpeculative(fakeBlock, env, attr.statInfo, treeMapper,
-                    t -> new AnalyzeDeferredDiagHandler(context));
+            LocalCacheContext localCacheContext = argumentAttr.withLocalCacheContext();
+            try {
+                deferredAttr.attribSpeculative(fakeBlock, env, attr.statInfo, treeMapper,
+                        t -> new AnalyzeDeferredDiagHandler(context));
+            } finally {
+                localCacheContext.leave();
+            }
 
             context.treeMap.entrySet().forEach(e -> {
                 context.treesToAnalyzer.get(e.getKey())
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java	Thu Oct 01 19:47:06 2015 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java	Fri Oct 02 13:27:57 2015 +0100
@@ -109,9 +109,6 @@
     /** Cache for argument types; behavior is influences by the currrently selected cache policy. */
     Map<UniquePos, ArgumentType<?>> argumentTypeCache = new LinkedHashMap<>();
 
-    /** Cache policy: should argument types be cached? */
-    private CachePolicy cachePolicy = CachePolicy.CACHE;
-
     public static ArgumentAttr instance(Context context) {
         ArgumentAttr instance = context.get(methodAttrKey);
         if (instance == null)
@@ -160,12 +157,29 @@
     }
 
     /**
-     * Sets given ache policy and returns current policy.
+     * Returns a local caching context in which argument types can safely be cached without
+     * the risk of polluting enclosing contexts. This is useful when attempting speculative
+     * attribution of potentially erroneous expressions, which could end up polluting the cache.
      */
-    CachePolicy withCachePolicy(CachePolicy newPolicy) {
-        CachePolicy oldPolicy = this.cachePolicy;
-        this.cachePolicy = newPolicy;
-        return oldPolicy;
+    LocalCacheContext withLocalCacheContext() {
+        return new LocalCacheContext();
+    }
+
+    /**
+     * Local cache context; this class keeps track of the previous cache and reverts to it
+     * when the {@link LocalCacheContext#leave()} method is called.
+     */
+    class LocalCacheContext {
+        Map<UniquePos, ArgumentType<?>> prevCache;
+
+        public LocalCacheContext() {
+            this.prevCache = argumentTypeCache;
+            argumentTypeCache = new HashMap<>();
+        }
+
+        public void leave() {
+            argumentTypeCache = prevCache;
+        }
     }
 
     /**
@@ -226,9 +240,7 @@
             setResult(that, cached.dup(that, env));
         } else {
             Z res = argumentTypeFactory.get();
-            if (cachePolicy == CachePolicy.CACHE) {
-                argumentTypeCache.put(pos, res);
-            }
+            argumentTypeCache.put(pos, res);
             setResult(that, res);
         }
     }
@@ -341,7 +353,7 @@
                 speculativeTypes.put(resultInfo, t);
                 return t;
             } else {
-                if (!env.info.isSpeculative && cachePolicy == CachePolicy.CACHE) {
+                if (!env.info.isSpeculative) {
                     argumentTypeCache.remove(new UniquePos(dt.tree));
                 }
                 return deferredAttr.basicCompleter.complete(dt, resultInfo, deferredAttrContext);
@@ -663,17 +675,4 @@
             return source.getFile().getName() + " @ " + source.getLineNumber(pos);
         }
     }
-
-    /**
-     * Argument type caching policy.
-     */
-    enum CachePolicy {
-        /** Cache argument types. */
-        CACHE,
-        /**
-         * Don't cache argument types. This is useful when performing speculative attribution on
-         * a tree that is known to contain erroneous info.
-         */
-        NO_CACHE;
-    }
 }
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java	Thu Oct 01 19:47:06 2015 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java	Fri Oct 02 13:27:57 2015 +0100
@@ -29,7 +29,7 @@
 import com.sun.source.tree.NewClassTree;
 import com.sun.tools.javac.code.*;
 import com.sun.tools.javac.code.Type.TypeMapping;
-import com.sun.tools.javac.comp.ArgumentAttr.CachePolicy;
+import com.sun.tools.javac.comp.ArgumentAttr.LocalCacheContext;
 import com.sun.tools.javac.comp.Resolve.ResolveError;
 import com.sun.tools.javac.resources.CompilerProperties.Fragments;
 import com.sun.tools.javac.tree.*;
@@ -777,14 +777,14 @@
 
             boolean canLambdaBodyCompleteNormally(JCLambda tree) {
                 List<JCVariableDecl> oldParams = tree.params;
-                CachePolicy prevPolicy = argumentAttr.withCachePolicy(CachePolicy.NO_CACHE);
+                LocalCacheContext localCacheContext = argumentAttr.withLocalCacheContext();
                 try {
                     tree.params = tree.params.stream()
                             .map(vd -> make.VarDef(vd.mods, vd.name, make.Erroneous(), null))
                             .collect(List.collector());
                     return attribSpeculativeLambda(tree, env, attr.unknownExprInfo).canCompleteNormally;
                 } finally {
-                    argumentAttr.withCachePolicy(prevPolicy);
+                    localCacheContext.leave();
                     tree.params = oldParams;
                 }
             }