8189747: JDK9 javax.lang.model.util.Elements#getTypeElement regressed 1000x in performance

Caching the results of Elements.getTypeElement/getPackageElement

Reviewed-by: darcy

src/jdk.compiler/share/classes/com/sun/tools/javac/model/JavacElements.java

@@ -26,9 +26,11 @@
 package com.sun.tools.javac.model;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -184,7 +186,9 @@ public class JavacElements implements Elements {
     }
 
     private final Set<String> alreadyWarnedDuplicates = new HashSet<>();
+    private final Map<Pair<String, String>, Optional<Symbol>> resultCache = new HashMap<>();
 
+    @SuppressWarnings("unchecked")
     private <S extends Symbol> S unboundNameToSymbol(String methodName,
                                                      String nameStr,
                                                      Class<S> clazz) {
@@ -192,44 +196,46 @@ public class JavacElements implements Elements {
             return nameToSymbol(syms.noModule, nameStr, clazz);
         }
 
-        Set<S> found = new LinkedHashSet<>();
+        return (S) resultCache.computeIfAbsent(Pair.of(methodName, nameStr), p -> {
+            Set<S> found = new LinkedHashSet<>();
 
-        for (ModuleSymbol msym : modules.allModules()) {
+            for (ModuleSymbol msym : modules.allModules()) {
-            S sym = nameToSymbol(msym, nameStr, clazz);
+                S sym = nameToSymbol(msym, nameStr, clazz);
 
-            if (sym == null)
+                if (sym == null)
-                continue;
+                    continue;
 
-            if (clazz == ClassSymbol.class) {
+                if (clazz == ClassSymbol.class) {
-                // Always include classes
+                    // Always include classes
-                found.add(sym);
-            } else if (clazz == PackageSymbol.class) {
-                // In module mode, ignore the "spurious" empty packages that "enclose" module-specific packages.
-                // For example, if a module contains classes or package info in package p.q.r, it will also appear
-                // to have additional packages p.q and p, even though these packages have no content other
-                // than the subpackage.  We don't want those empty packages showing up in searches for p or p.q.
-                if (!sym.members().isEmpty() || ((PackageSymbol) sym).package_info != null) {
                     found.add(sym);
+                } else if (clazz == PackageSymbol.class) {
+                    // In module mode, ignore the "spurious" empty packages that "enclose" module-specific packages.
+                    // For example, if a module contains classes or package info in package p.q.r, it will also appear
+                    // to have additional packages p.q and p, even though these packages have no content other
+                    // than the subpackage.  We don't want those empty packages showing up in searches for p or p.q.
+                    if (!sym.members().isEmpty() || ((PackageSymbol) sym).package_info != null) {
+                        found.add(sym);
+                    }
                 }
             }
-        }
 
-        if (found.size() == 1) {
+            if (found.size() == 1) {
-            return found.iterator().next();
+                return Optional.of(found.iterator().next());
-        } else if (found.size() > 1) {
+            } else if (found.size() > 1) {
-            //more than one element found, produce a note:
+                //more than one element found, produce a note:
-            if (alreadyWarnedDuplicates.add(methodName + ":" + nameStr)) {
+                if (alreadyWarnedDuplicates.add(methodName + ":" + nameStr)) {
-                String moduleNames = found.stream()
+                    String moduleNames = found.stream()
-                                          .map(s -> s.packge().modle)
+                                              .map(s -> s.packge().modle)
-                                          .map(m -> m.toString())
+                                              .map(m -> m.toString())
-                                          .collect(Collectors.joining(", "));
+                                              .collect(Collectors.joining(", "));
-                log.note(Notes.MultipleElements(methodName, nameStr, moduleNames));
+                    log.note(Notes.MultipleElements(methodName, nameStr, moduleNames));
+                }
+                return Optional.empty();
+            } else {
+                //not found:
+                return Optional.empty();
             }
-            return null;
+        }).orElse(null);
-        } else {
-            //not found, or more than one element found:
-            return null;
-        }
     }
 
     /**
@@ -787,4 +793,8 @@ public class JavacElements implements Elements {
             throw new IllegalArgumentException(o.toString());
         return clazz.cast(o);
     }
+
+    public void newRound() {
+        resultCache.clear();
+    }
 }

src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java

@@ -1270,6 +1270,7 @@ public class JavacProcessingEnvironment implements ProcessingEnvironment, Closea
             modules.newRound();
             types.newRound();
             annotate.newRound();
+            elementUtils.newRound();
 
             boolean foundError = false;
 

test/langtools/tools/javac/modules/AnnotationProcessing.java

@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 8133884 8162711 8133896 8172158 8172262 8173636 8175119
+ * @bug 8133884 8162711 8133896 8172158 8172262 8173636 8175119 8189747
  * @summary Verify that annotation processing works.
  * @library /tools/lib
  * @modules
@@ -1389,6 +1389,19 @@ public class AnnotationProcessing extends ModuleTestBase {
             .run()
             .writeAll();
 
+        //from source:
+        new JavacTask(tb)
+            .options("--module-source-path", moduleSrc.toString(),
+                     "--source-path", src.toString(),
+                     "-processorpath", System.getProperty("test.class.path"),
+                     "-processor", UnboundLookupGenerate.class.getName(),
+                     "-XDrawDiagnostics")
+            .outdir(classes)
+            .files(findJavaFiles(moduleSrc))
+            .run()
+            .writeAll()
+            .getOutput(OutputKind.DIRECT);
+
     }
 
     @SupportedAnnotationTypes("*")
@@ -1486,6 +1499,29 @@ public class AnnotationProcessing extends ModuleTestBase {
 
     }
 
+    @SupportedAnnotationTypes("*")
+    public static final class UnboundLookupGenerate extends AbstractProcessor {
+
+        @Override
+        public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
+            if (processingEnv.getElementUtils().getTypeElement("nue.Nue") == null) {
+                try (Writer w = processingEnv.getFiler().createSourceFile("m1x/nue.Nue").openWriter()) {
+                    w.write("package nue; public class Nue {}");
+                } catch (IOException ex) {
+                    throw new IllegalStateException(ex);
+                }
+            }
+
+            return false;
+        }
+
+        @Override
+        public SourceVersion getSupportedSourceVersion() {
+            return SourceVersion.latest();
+        }
+
+    }
+
     @Test
     public void testWrongDefaultTargetModule(Path base) throws Exception {
         Path src = base.resolve("src");