8173117: Compilation significantly slower after JDK-8169197

Only using recovery search when an error is inevitable.

Reviewed-by: jjg, mcimadamore
This commit is contained in:
Jan Lahoda 2017-01-20 15:32:07 +01:00
parent 3e7e4c275b
commit 721001933d
7 changed files with 285 additions and 95 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -818,4 +818,12 @@ public class Symtab {
public Collection<ModuleSymbol> getAllModules() {
return modules.values();
}
public Iterable<ClassSymbol> getClassesForName(Name candidate) {
return classes.getOrDefault(candidate, Collections.emptyMap()).values();
}
public Iterable<PackageSymbol> getPackagesForName(Name candidate) {
return packages.getOrDefault(candidate, Collections.emptyMap()).values();
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -41,6 +41,7 @@ import com.sun.tools.javac.comp.Resolve.MethodResolutionDiagHelper.Template;
import com.sun.tools.javac.comp.Resolve.ReferenceLookupResult.StaticKind;
import com.sun.tools.javac.jvm.*;
import com.sun.tools.javac.main.Option;
import com.sun.tools.javac.resources.CompilerProperties.Fragments;
import com.sun.tools.javac.tree.*;
import com.sun.tools.javac.tree.JCTree.*;
import com.sun.tools.javac.tree.JCTree.JCMemberReference.ReferenceKind;
@ -61,12 +62,12 @@ import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Stream;
import javax.lang.model.element.ElementVisitor;
import com.sun.tools.javac.code.Directive.ExportsDirective;
import static com.sun.tools.javac.code.Flags.*;
import static com.sun.tools.javac.code.Flags.BLOCK;
import static com.sun.tools.javac.code.Flags.STATIC;
@ -74,9 +75,8 @@ import static com.sun.tools.javac.code.Kinds.*;
import static com.sun.tools.javac.code.Kinds.Kind.*;
import static com.sun.tools.javac.code.TypeTag.*;
import static com.sun.tools.javac.comp.Resolve.MethodResolutionPhase.*;
import com.sun.tools.javac.resources.CompilerProperties.Errors;
import com.sun.tools.javac.resources.CompilerProperties.Fragments;
import static com.sun.tools.javac.tree.JCTree.Tag.*;
import static com.sun.tools.javac.util.Iterators.createCompoundIterator;
/** Helper class for name resolution, used mostly by the attribution phase.
*
@ -1970,7 +1970,7 @@ public class Resolve {
* @param env The current environment.
* @param name The fully qualified name of the class to be loaded.
*/
Symbol loadClass(Env<AttrContext> env, Name name) {
Symbol loadClass(Env<AttrContext> env, Name name, RecoveryLoadClass recoveryLoadClass) {
try {
ClassSymbol c = finder.loadClass(env.toplevel.modle, name);
return isAccessible(env, c) ? c : new AccessError(env, null, c);
@ -1987,40 +1987,60 @@ public class Resolve {
}
}
public static interface RecoveryLoadClass {
public interface RecoveryLoadClass {
Symbol loadClass(Env<AttrContext> env, Name name);
}
private RecoveryLoadClass recoveryLoadClass = new RecoveryLoadClass() {
@Override
public Symbol loadClass(Env<AttrContext> env, Name name) {
if (allowModules) {
Scope importScope = env.toplevel.namedImportScope;
Symbol existing = importScope.findFirst(Convert.shortName(name),
sym -> sym.kind == TYP && sym.flatName() == name);
private final RecoveryLoadClass noRecovery = (env, name) -> null;
if (existing != null) {
return new InvisibleSymbolError(env, true, existing);
}
return lookupInvisibleSymbol(env, name, syms::getClass, (ms, n) -> {
private final RecoveryLoadClass doRecoveryLoadClass = new RecoveryLoadClass() {
@Override public Symbol loadClass(Env<AttrContext> env, Name name) {
List<Name> candidates = Convert.classCandidates(name);
return lookupInvisibleSymbol(env, name,
n -> () -> createCompoundIterator(candidates,
c -> syms.getClassesForName(c)
.iterator()),
(ms, n) -> {
for (Name candidate : candidates) {
try {
return finder.loadClass(ms, n);
return finder.loadClass(ms, candidate);
} catch (CompletionFailure cf) {
//ignore
return null;
}
}, sym -> sym.kind == Kind.TYP, false, typeNotFound);
}
return null;
}
return null;
}, sym -> sym.kind == Kind.TYP, false, typeNotFound);
}
};
public RecoveryLoadClass setRecoveryLoadClass(RecoveryLoadClass recovery) {
RecoveryLoadClass prev = recoveryLoadClass;
recoveryLoadClass = recovery;
return prev;
}
private final RecoveryLoadClass namedImportScopeRecovery = (env, name) -> {
Scope importScope = env.toplevel.namedImportScope;
Symbol existing = importScope.findFirst(Convert.shortName(name),
sym -> sym.kind == TYP && sym.flatName() == name);
if (existing != null) {
return new InvisibleSymbolError(env, true, existing);
}
return null;
};
private final RecoveryLoadClass starImportScopeRecovery = (env, name) -> {
Scope importScope = env.toplevel.starImportScope;
Symbol existing = importScope.findFirst(Convert.shortName(name),
sym -> sym.kind == TYP && sym.flatName() == name);
if (existing != null) {
try {
existing = finder.loadClass(existing.packge().modle, name);
return new InvisibleSymbolError(env, true, existing);
} catch (CompletionFailure cf) {
//ignore
}
}
return null;
};
Symbol lookupPackage(Env<AttrContext> env, Name name) {
PackageSymbol pack = syms.lookupPackage(env.toplevel.modle, name);
@ -2034,7 +2054,7 @@ public class Resolve {
.stream()
.anyMatch(p -> p.fullname.startsWith(nameAndDot));
return lookupInvisibleSymbol(env, name, syms::getPackage, syms::enterPackage, sym -> {
return lookupInvisibleSymbol(env, name, syms::getPackagesForName, syms::enterPackage, sym -> {
sym.complete();
return sym.exists();
}, prefixOfKnown, pack);
@ -2059,42 +2079,44 @@ public class Resolve {
return TreeInfo.fullName(((JCFieldAccess) qualid).selected) == name;
}
private Symbol lookupInvisibleSymbol(Env<AttrContext> env,
Name name,
BiFunction<ModuleSymbol, Name, Symbol> get,
BiFunction<ModuleSymbol, Name, Symbol> load,
Predicate<Symbol> validate,
boolean suppressError,
Symbol defaultResult) {
private <S extends Symbol> Symbol lookupInvisibleSymbol(Env<AttrContext> env,
Name name,
Function<Name, Iterable<S>> get,
BiFunction<ModuleSymbol, Name, S> load,
Predicate<S> validate,
boolean suppressError,
Symbol defaultResult) {
//even if a class/package cannot be found in the current module and among packages in modules
//it depends on that are exported for any or this module, the class/package may exist internally
//in some of these modules, or may exist in a module on which this module does not depend.
//Provide better diagnostic in such cases by looking for the class in any module:
Iterable<? extends S> candidates = get.apply(name);
for (S sym : candidates) {
if (validate.test(sym))
return new InvisibleSymbolError(env, suppressError, sym);
}
Set<ModuleSymbol> recoverableModules = new HashSet<>(syms.getAllModules());
recoverableModules.remove(env.toplevel.modle);
for (ModuleSymbol ms : recoverableModules) {
Symbol sym = get.apply(ms, name);
//avoid overly eager completing classes from source-based modules, as those
//may not be completable with the current compiler settings:
if (sym == null && (ms.sourceLocation == null)) {
if (ms.sourceLocation == null) {
if (ms.classLocation == null) {
ms = moduleFinder.findModule(ms);
}
if (ms.kind != ERR) {
sym = load.apply(ms, name);
S sym = load.apply(ms, name);
if (sym != null && validate.test(sym)) {
return new InvisibleSymbolError(env, suppressError, sym);
}
}
}
if (sym == null)
continue;
if (validate.test(sym)) {
return new InvisibleSymbolError(env, suppressError, sym);
}
}
return defaultResult;
@ -2186,10 +2208,10 @@ public class Resolve {
* @param scope The scope in which to look for the type.
* @param name The type's name.
*/
Symbol findGlobalType(Env<AttrContext> env, Scope scope, Name name) {
Symbol findGlobalType(Env<AttrContext> env, Scope scope, Name name, RecoveryLoadClass recoveryLoadClass) {
Symbol bestSoFar = typeNotFound;
for (Symbol s : scope.getSymbolsByName(name)) {
Symbol sym = loadClass(env, s.flatName());
Symbol sym = loadClass(env, s.flatName(), recoveryLoadClass);
if (bestSoFar.kind == TYP && sym.kind == TYP &&
bestSoFar != sym)
return new AmbiguityError(bestSoFar, sym);
@ -2260,15 +2282,15 @@ public class Resolve {
}
if (!env.tree.hasTag(IMPORT)) {
sym = findGlobalType(env, env.toplevel.namedImportScope, name);
sym = findGlobalType(env, env.toplevel.namedImportScope, name, namedImportScopeRecovery);
if (sym.exists()) return sym;
else bestSoFar = bestOf(bestSoFar, sym);
sym = findGlobalType(env, env.toplevel.packge.members(), name);
sym = findGlobalType(env, env.toplevel.packge.members(), name, noRecovery);
if (sym.exists()) return sym;
else bestSoFar = bestOf(bestSoFar, sym);
sym = findGlobalType(env, env.toplevel.starImportScope, name);
sym = findGlobalType(env, env.toplevel.starImportScope, name, starImportScopeRecovery);
if (sym.exists()) return sym;
else bestSoFar = bestOf(bestSoFar, sym);
}
@ -2315,7 +2337,11 @@ public class Resolve {
Name fullname = TypeSymbol.formFullName(name, pck);
Symbol bestSoFar = typeNotFound;
if (kind.contains(KindSelector.TYP)) {
Symbol sym = loadClass(env, fullname);
RecoveryLoadClass recoveryLoadClass =
allowModules && !kind.contains(KindSelector.PCK) &&
!pck.exists() && !env.info.isSpeculative ?
doRecoveryLoadClass : noRecovery;
Symbol sym = loadClass(env, fullname, recoveryLoadClass);
if (sym.exists()) {
// don't allow programs to use flatnames
if (name == sym.name) return sym;
@ -4136,11 +4162,21 @@ public class Resolve {
JCDiagnostic details = inaccessiblePackageReason(env, sym.packge());
if (pos.getTree() != null && pos.getTree().hasTag(SELECT) && sym.owner.kind == PCK) {
pos = ((JCFieldAccess) pos.getTree()).selected.pos();
if (pos.getTree() != null) {
Symbol o = sym;
JCTree tree = pos.getTree();
return diags.create(dkind, log.currentSource(),
pos, "package.not.visible", sym.packge(), details);
while (o.kind != PCK && tree.hasTag(SELECT)) {
o = o.owner;
tree = ((JCFieldAccess) tree).selected;
}
if (o.kind == PCK) {
pos = tree.pos();
return diags.create(dkind, log.currentSource(),
pos, "package.not.visible", o, details);
}
}
return diags.create(dkind, log.currentSource(),

View File

@ -184,39 +184,34 @@ public class JavacElements implements Elements {
return nameToSymbol(syms.noModule, nameStr, clazz);
}
RecoveryLoadClass prevRecoveryLoadClass = resolve.setRecoveryLoadClass((env, name) -> null);
try {
Set<S> found = new LinkedHashSet<>();
Set<S> found = new LinkedHashSet<>();
for (ModuleSymbol msym : modules.allModules()) {
S sym = nameToSymbol(msym, nameStr, clazz);
for (ModuleSymbol msym : modules.allModules()) {
S sym = nameToSymbol(msym, nameStr, clazz);
if (sym != null) {
if (!allowModules || clazz == ClassSymbol.class || !sym.members().isEmpty()) {
//do not add packages without members:
found.add(sym);
}
if (sym != null) {
if (!allowModules || clazz == ClassSymbol.class || !sym.members().isEmpty()) {
//do not add packages without members:
found.add(sym);
}
}
}
if (found.size() == 1) {
return found.iterator().next();
} else if (found.size() > 1) {
//more than one element found, produce a note:
if (alreadyWarnedDuplicates.add(methodName + ":" + nameStr)) {
String moduleNames = found.stream()
.map(s -> s.packge().modle)
.map(m -> m.toString())
.collect(Collectors.joining(", "));
log.note(Notes.MultipleElements(methodName, nameStr, moduleNames));
}
return null;
} else {
//not found, or more than one element found:
return null;
if (found.size() == 1) {
return found.iterator().next();
} else if (found.size() > 1) {
//more than one element found, produce a note:
if (alreadyWarnedDuplicates.add(methodName + ":" + nameStr)) {
String moduleNames = found.stream()
.map(s -> s.packge().modle)
.map(m -> m.toString())
.collect(Collectors.joining(", "));
log.note(Notes.MultipleElements(methodName, nameStr, moduleNames));
}
} finally {
resolve.setRecoveryLoadClass(prevRecoveryLoadClass);
return null;
} else {
//not found, or more than one element found:
return null;
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -335,4 +335,16 @@ public class Convert {
}
return names;
}
public static List<Name> classCandidates(Name name) {
List<Name> names = List.nil();
String nameStr = name.toString();
int index = -1;
while ((index = nameStr.indexOf('.', index + 1)) > 0) {
String pack = nameStr.substring(0, index + 1);
String clz = nameStr.substring(index + 1).replace('.', '$');
names = names.prepend(name.table.names.fromString(pack + clz));
}
return names.reverse();
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -23,11 +23,12 @@
/*
* @test
* @bug 8169197 8172668
* @bug 8169197 8172668 8173117
* @summary Check convenient errors are produced for inaccessible classes.
* @library /tools/lib
* @modules jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.main
* jdk.compiler/com.sun.tools.javac.util
* @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask ModuleTestBase
* @run main ConvenientAccessErrorsTest
*/
@ -37,6 +38,10 @@ import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.Convert;
import com.sun.tools.javac.util.Name;
import com.sun.tools.javac.util.Names;
import toolbox.JarTask;
import toolbox.JavacTask;
import toolbox.Task;
@ -78,6 +83,37 @@ public class ConvenientAccessErrorsTest extends ModuleTestBase {
throw new Exception("expected output not found; actual: " + log);
}
@Test
public void testNoDepNested(Path base) throws Exception {
Path src = base.resolve("src");
Path src_m1 = src.resolve("m1x");
tb.writeJavaFiles(src_m1,
"module m1x { exports api; }",
"package api; public class Api { public static class Nested {} }");
Path src_m2 = src.resolve("m2x");
tb.writeJavaFiles(src_m2,
"module m2x { }",
"package test; public class Test { api.Api.Nested nested; }");
Path classes = base.resolve("classes");
tb.createDirectories(classes);
List<String> log = new JavacTask(tb)
.options("-XDrawDiagnostics",
"--module-source-path", src.toString())
.outdir(classes)
.files(findJavaFiles(src))
.run(Task.Expect.FAIL)
.writeAll()
.getOutputLines(Task.OutputKind.DIRECT);
List<String> expected = Arrays.asList(
"Test.java:1:35: compiler.err.package.not.visible: api, (compiler.misc.not.def.access.does.not.read: m2x, api, m1x)",
"1 error");
if (!expected.equals(log))
throw new Exception("expected output not found; actual: " + log);
}
@Test
public void testNotExported(Path base) throws Exception {
Path src = base.resolve("src");
@ -390,8 +426,7 @@ public class ConvenientAccessErrorsTest extends ModuleTestBase {
List<String> expected = Arrays.asList(
"Test.java:1:22: compiler.err.package.not.visible: api, (compiler.misc.not.def.access.not.exported: api, m1x)",
"Test.java:1:49: compiler.err.not.def.access.package.cant.access: api.Api, api, (compiler.misc.not.def.access.not.exported: api, m1x)",
"2 errors");
"1 error");
if (!expected.equals(log))
throw new Exception("expected output not found; actual: " + log);
@ -593,4 +628,80 @@ public class ConvenientAccessErrorsTest extends ModuleTestBase {
throw new Exception("expected output not found; actual: " + log);
}
@Test
public void testInaccessibleInSourceModuleViaBinaryModule(Path base) throws Exception {
Path src = base.resolve("src");
Path src_m1 = src.resolve("m1x");
tb.writeJavaFiles(src_m1,
"@Deprecated module m1x { }");
Path src_m2 = src.resolve("m2x");
tb.writeJavaFiles(src_m2,
"module m2x { requires transitive m1x; }");
Path src_m3 = src.resolve("m3x");
tb.writeJavaFiles(src_m3,
"module m3x { requires transitive m2x; exports api; }",
"package api; class Api { }");
Path classes = base.resolve("classes");
tb.createDirectories(classes);
new JavacTask(tb)
.options("-XDrawDiagnostics",
"--module-source-path", src.toString())
.outdir(classes)
.files(findJavaFiles(src))
.run()
.writeAll();
tb.cleanDirectory(classes.resolve("m2x")); //force completion from source if needed
Files.delete(classes.resolve("m2x"));
tb.cleanDirectory(src_m3); //binary only module
Files.delete(src_m3);
//m4x does not depend on m1x/m2x/m3x, so cannot access api.Api
//but the recovery search should not complete m2x, as that would cause a deprecation warning:
Path src_m4 = src.resolve("m4x");
tb.writeJavaFiles(src_m4,
"module m4x { }",
"package m4x; public class Test extends api.Api { }");
List<String> log = new JavacTask(tb)
.options("-XDrawDiagnostics",
"--module-source-path", src.toString(),
"--module-path", classes.toString(),
"-Xlint:deprecation")
.outdir(classes)
.files(findJavaFiles(src_m4))
.run(Task.Expect.FAIL)
.writeAll()
.getOutputLines(Task.OutputKind.DIRECT);
List<String> expected = Arrays.asList(
"Test.java:1:40: compiler.err.package.not.visible: api, (compiler.misc.not.def.access.does.not.read: m4x, api, m3x)",
"1 error");
if (!expected.equals(log))
throw new Exception("expected output not found; actual: " + log);
}
@Test
public void testConvertNameCandidates(Path base) throws Exception {
Context ctx = new Context();
Names names = Names.instance(ctx);
Name name = names.fromString("com.sun.tools.javac.Attr.BreakAttr");
com.sun.tools.javac.util.List<String> actual =
Convert.classCandidates(name).map(n -> n.toString());
List<String> expected = Arrays.asList(
"com.sun$tools$javac$Attr$BreakAttr",
"com.sun.tools$javac$Attr$BreakAttr",
"com.sun.tools.javac$Attr$BreakAttr",
"com.sun.tools.javac.Attr$BreakAttr",
"com.sun.tools.javac.Attr.BreakAttr"
);
if (!expected.equals(actual)) {
throw new Exception("Expected names not generated: " + actual);
}
}
}

View File

@ -23,7 +23,7 @@
/*
* @test
* @bug 8154283 8167320 8171098 8172809
* @bug 8154283 8167320 8171098 8172809 8173117
* @summary tests for multi-module mode compilation
* @library /tools/lib
* @modules
@ -495,6 +495,33 @@ public class EdgeCases extends ModuleTestBase {
}
}
@Test
public void testInvisibleClassVisiblePackageClash(Path base) throws Exception {
Path src = base.resolve("src");
Path src_m1 = src.resolve("m1x");
tb.writeJavaFiles(src_m1,
"module m1x { }",
"package m1x;\n" +
"import m1x.a.*; public class Test { A a; }\n",
"package m1x.a;\n" +
"public class A { }\n");
Path src_m2 = src.resolve("m2x");
tb.writeJavaFiles(src_m2,
"module m2x { }",
"package m1x;\n" +
"public class a { public static class A { } }\n");
Path classes = base.resolve("classes");
tb.createDirectories(classes);
new JavacTask(tb)
.options("--module-source-path", src.toString(),
"-XDrawDiagnostics")
.outdir(classes)
.files(findJavaFiles(src))
.run()
.writeAll();
}
@Test
public void testStripUnknownRequired(Path base) throws Exception {
Path src = base.resolve("src");

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -70,9 +70,10 @@ public class PackageMultipleModules extends ModuleTestBase {
.writeAll()
.getOutputLines(Task.OutputKind.DIRECT);
List<String> expected = Arrays.asList("A.java:1:22: compiler.err.package.not.visible: test, (compiler.misc.not.def.access.does.not.read: m1x, test, m2x)",
"B.java:1:22: compiler.err.package.not.visible: test, (compiler.misc.not.def.access.does.not.read: m2x, test, m1x)",
"2 errors");
List<String> expected = Arrays.asList(
"A.java:1:26: compiler.err.cant.resolve.location: kindname.class, B, , , (compiler.misc.location: kindname.package, test, null)",
"B.java:1:26: compiler.err.cant.resolve.location: kindname.class, A, , , (compiler.misc.location: kindname.package, test, null)",
"2 errors");
if (!log.equals(expected))
throw new Exception("expected output not found");
}