# HG changeset patch
# User jlahoda
# Date 1433401552 -7200
# Node ID 673532e903376adaa342b88e2b6cd7896ccb899c
# Parent  dc33ef3ba6675e2549debb34608beb177903a83e
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

diff -r dc33ef3ba667 -r 673532e90337 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
--- 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>
 
diff -r dc33ef3ba667 -r 673532e90337 langtools/test/tools/javac/types/ScopeListenerTest.java
--- /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")))
+            ;
+    }
+
+}