8039262: Java compiler performance degradation jdk1.7 vs. jdk1.6 should be amended
Summary: Avoiding Scope listener leak by avoiding cache misses in Types.MembersClosureCache
Reviewed-by: mcimadamore, vromero
Contributed-by: maurizio.cimadamore@oracle.com
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java Mon Jun 01 15:19:54 2015 -0700
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java Thu Jun 04 09:05:52 2015 +0200
@@ -2659,73 +2659,92 @@
// </editor-fold>
// <editor-fold defaultstate="collapsed" desc="compute transitive closure of all members in given site">
- class MembersClosureCache extends SimpleVisitor<CompoundScope, Boolean> {
-
- private WeakHashMap<TypeSymbol, Entry> _map = new WeakHashMap<>();
-
- class Entry {
- final boolean skipInterfaces;
- final CompoundScope compoundScope;
-
- public Entry(boolean skipInterfaces, CompoundScope compoundScope) {
- this.skipInterfaces = skipInterfaces;
- this.compoundScope = compoundScope;
+ class MembersClosureCache extends SimpleVisitor<Scope.CompoundScope, Void> {
+
+ private Map<TypeSymbol, CompoundScope> _map = new HashMap<>();
+
+ Set<TypeSymbol> seenTypes = new HashSet<>();
+
+ class MembersScope extends CompoundScope {
+
+ CompoundScope scope;
+
+ public MembersScope(CompoundScope scope) {
+ super(scope.owner);
+ this.scope = scope;
}
- boolean matches(boolean skipInterfaces) {
- return this.skipInterfaces == skipInterfaces;
+ Filter<Symbol> combine(Filter<Symbol> sf) {
+ return s -> !s.owner.isInterface() && (sf == null || sf.accepts(s));
+ }
+
+ @Override
+ public Iterable<Symbol> getSymbols(Filter<Symbol> sf, LookupKind lookupKind) {
+ return scope.getSymbols(combine(sf), lookupKind);
+ }
+
+ @Override
+ public Iterable<Symbol> getSymbolsByName(Name name, Filter<Symbol> sf, LookupKind lookupKind) {
+ return scope.getSymbolsByName(name, combine(sf), lookupKind);
+ }
+
+ @Override
+ public int getMark() {
+ return scope.getMark();
}
}
- List<TypeSymbol> seenTypes = List.nil();
+ CompoundScope nilScope;
/** members closure visitor methods **/
- public CompoundScope visitType(Type t, Boolean skipInterface) {
- return null;
+ public CompoundScope visitType(Type t, Void _unused) {
+ if (nilScope == null) {
+ nilScope = new CompoundScope(syms.noSymbol);
+ }
+ return nilScope;
}
@Override
- public CompoundScope visitClassType(ClassType t, Boolean skipInterface) {
- if (seenTypes.contains(t.tsym)) {
+ public CompoundScope visitClassType(ClassType t, Void _unused) {
+ if (!seenTypes.add(t.tsym)) {
//this is possible when an interface is implemented in multiple
- //superclasses, or when a classs hierarchy is circular - in such
+ //superclasses, or when a class hierarchy is circular - in such
//cases we don't need to recurse (empty scope is returned)
return new CompoundScope(t.tsym);
}
try {
- seenTypes = seenTypes.prepend(t.tsym);
+ seenTypes.add(t.tsym);
ClassSymbol csym = (ClassSymbol)t.tsym;
- Entry e = _map.get(csym);
- if (e == null || !e.matches(skipInterface)) {
- CompoundScope membersClosure = new CompoundScope(csym);
- if (!skipInterface) {
- for (Type i : interfaces(t)) {
- membersClosure.prependSubScope(visit(i, skipInterface));
- }
+ CompoundScope membersClosure = _map.get(csym);
+ if (membersClosure == null) {
+ membersClosure = new CompoundScope(csym);
+ for (Type i : interfaces(t)) {
+ membersClosure.prependSubScope(visit(i, null));
}
- membersClosure.prependSubScope(visit(supertype(t), skipInterface));
+ membersClosure.prependSubScope(visit(supertype(t), null));
membersClosure.prependSubScope(csym.members());
- e = new Entry(skipInterface, membersClosure);
- _map.put(csym, e);
+ _map.put(csym, membersClosure);
}
- return e.compoundScope;
+ return membersClosure;
}
finally {
- seenTypes = seenTypes.tail;
+ seenTypes.remove(t.tsym);
}
}
@Override
- public CompoundScope visitTypeVar(TypeVar t, Boolean skipInterface) {
- return visit(t.getUpperBound(), skipInterface);
+ public CompoundScope visitTypeVar(TypeVar t, Void _unused) {
+ return visit(t.getUpperBound(), null);
}
}
private MembersClosureCache membersCache = new MembersClosureCache();
public CompoundScope membersClosure(Type site, boolean skipInterface) {
- return membersCache.visit(site, skipInterface);
+ CompoundScope cs = membersCache.visit(site, null);
+ Assert.checkNonNull(cs, () -> "type " + site);
+ return skipInterface ? membersCache.new MembersScope(cs) : cs;
}
// </editor-fold>
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/types/ScopeListenerTest.java Thu Jun 04 09:05:52 2015 +0200
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2015, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8039262
+ * @summary Ensure that using Types.membersClosure does not increase the number of listeners on the
+ * class's members Scope.
+ */
+
+import com.sun.tools.javac.code.Scope;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symtab;
+import com.sun.tools.javac.code.Types;
+import com.sun.tools.javac.file.JavacFileManager;
+import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.util.Names;
+import java.lang.reflect.Field;
+import java.util.Collection;
+
+public class ScopeListenerTest {
+
+ public static void main(String[] args) throws Exception {
+ new ScopeListenerTest().run();
+ }
+
+ void run() throws Exception {
+ Context context = new Context();
+ JavacFileManager.preRegister(context);
+ Types types = Types.instance(context);
+ Symtab syms = Symtab.instance(context);
+ Names names = Names.instance(context);
+ types.membersClosure(syms.stringType, true);
+ types.membersClosure(syms.stringType, false);
+
+ Field listenersField = Scope.class.getDeclaredField("listeners");
+
+ listenersField.setAccessible(true);
+
+ int listenerCount =
+ ((Collection) listenersField.get(syms.stringType.tsym.members())).size();
+
+ for (int i = 0; i < 100; i++) {
+ types.membersClosure(syms.stringType, true);
+ types.membersClosure(syms.stringType, false);
+ }
+
+ int newListenerCount
+ = ((Collection) listenersField.get(syms.stringType.tsym.members())).size();
+
+ if (listenerCount != newListenerCount) {
+ throw new AssertionError("Orig listener count: " + listenerCount +
+ "; new listener count: " + newListenerCount);
+ }
+
+ for (Symbol s : types.membersClosure(syms.stringType, true).getSymbols())
+ ;
+ for (Symbol s : types.membersClosure(syms.stringType, false).getSymbolsByName(names.fromString("substring")))
+ ;
+ }
+
+}