Sat, 06 Nov 2010 13:53:48 -0700
6998063: new Scope impl to fix Scope performance issues
Reviewed-by: jjg
Contributed-by: per.bothner@oracle.com
1.1 --- a/src/share/classes/com/sun/tools/javac/code/Scope.java Thu Nov 04 15:39:43 2010 -0700 1.2 +++ b/src/share/classes/com/sun/tools/javac/code/Scope.java Sat Nov 06 13:53:48 2010 -0700 1.3 @@ -1,5 +1,5 @@ 1.4 /* 1.5 - * Copyright (c) 1999, 2008, Oracle and/or its affiliates. All rights reserved. 1.6 + * Copyright (c) 1999, 2010, Oracle and/or its affiliates. All rights reserved. 1.7 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 1.8 * 1.9 * This code is free software; you can redistribute it and/or modify it 1.10 @@ -31,7 +31,8 @@ 1.11 /** A scope represents an area of visibility in a Java program. The 1.12 * Scope class is a container for symbols which provides 1.13 * efficient access to symbols given their names. Scopes are implemented 1.14 - * as hash tables. Scopes can be nested; the next field of a scope points 1.15 + * as hash tables with "open addressing" and "double hashing". 1.16 + * Scopes can be nested; the next field of a scope points 1.17 * to its next outer scope. Nested scopes can share their hash tables. 1.18 * 1.19 * <p><b>This is NOT part of any supported API. 1.20 @@ -55,7 +56,7 @@ 1.21 1.22 /** A hash table for the scope's entries. 1.23 */ 1.24 - public Entry[] table; 1.25 + Entry[] table; 1.26 1.27 /** Mask for hash codes, always equal to (table.length - 1). 1.28 */ 1.29 @@ -67,8 +68,9 @@ 1.30 public Entry elems; 1.31 1.32 /** The number of elements in this scope. 1.33 + * This includes deleted elements, whose value is the sentinel. 1.34 */ 1.35 - public int nelems = 0; 1.36 + int nelems = 0; 1.37 1.38 /** A timestamp - useful to quickly check whether a scope has changed or not 1.39 */ 1.40 @@ -109,7 +111,8 @@ 1.41 } 1.42 } 1.43 1.44 - /** Every hash bucket is a list of Entry's which ends in sentinel. 1.45 + /** Use as a "not-found" result for lookup. 1.46 + * Also used to mark deleted entries in the table. 1.47 */ 1.48 private static final Entry sentinel = new Entry(null, null, null, null); 1.49 1.50 @@ -130,12 +133,15 @@ 1.51 this.owner = owner; 1.52 this.table = table; 1.53 this.hashMask = table.length - 1; 1.54 - this.elems = null; 1.55 - this.nelems = 0; 1.56 - this.shared = 0; 1.57 this.scopeCounter = scopeCounter; 1.58 } 1.59 1.60 + /** Convenience constructor used for dup and dupUnshared. */ 1.61 + private Scope(Scope next, Symbol owner, Entry[] table) { 1.62 + this(next, owner, table, next.scopeCounter); 1.63 + this.nelems = next.nelems; 1.64 + } 1.65 + 1.66 /** Construct a new scope, within scope next, with given owner, 1.67 * using a fresh table of length INITIAL_SIZE. 1.68 */ 1.69 @@ -145,7 +151,6 @@ 1.70 1.71 protected Scope(Symbol owner, ScopeCounter scopeCounter) { 1.72 this(null, owner, new Entry[INITIAL_SIZE], scopeCounter); 1.73 - for (int i = 0; i < INITIAL_SIZE; i++) table[i] = sentinel; 1.74 } 1.75 1.76 /** Construct a fresh scope within this scope, with same owner, 1.77 @@ -154,11 +159,7 @@ 1.78 * of fresh tables. 1.79 */ 1.80 public Scope dup() { 1.81 - Scope result = new Scope(this, this.owner, this.table, scopeCounter); 1.82 - shared++; 1.83 - // System.out.println("====> duping scope " + this.hashCode() + " owned by " + this.owner + " to " + result.hashCode()); 1.84 - // new Error().printStackTrace(System.out); 1.85 - return result; 1.86 + return dup(this.owner); 1.87 } 1.88 1.89 /** Construct a fresh scope within this scope, with new owner, 1.90 @@ -167,7 +168,7 @@ 1.91 * of fresh tables. 1.92 */ 1.93 public Scope dup(Symbol newOwner) { 1.94 - Scope result = new Scope(this, newOwner, this.table, scopeCounter); 1.95 + Scope result = new Scope(this, newOwner, this.table); 1.96 shared++; 1.97 // System.out.println("====> duping scope " + this.hashCode() + " owned by " + newOwner + " to " + result.hashCode()); 1.98 // new Error().printStackTrace(System.out); 1.99 @@ -179,7 +180,7 @@ 1.100 * the table of its outer scope. 1.101 */ 1.102 public Scope dupUnshared() { 1.103 - return new Scope(this, this.owner, this.table.clone(), scopeCounter); 1.104 + return new Scope(this, this.owner, this.table.clone()); 1.105 } 1.106 1.107 /** Remove all entries of this scope from its table, if shared 1.108 @@ -189,7 +190,7 @@ 1.109 assert shared == 0; 1.110 if (table != next.table) return next; 1.111 while (elems != null) { 1.112 - int hash = elems.sym.name.hashCode() & hashMask; 1.113 + int hash = getIndex(elems.sym.name); 1.114 Entry e = table[hash]; 1.115 assert e == elems : elems.sym; 1.116 table[hash] = elems.shadowed; 1.117 @@ -197,6 +198,7 @@ 1.118 } 1.119 assert next.shared > 0; 1.120 next.shared--; 1.121 + next.nelems = nelems; 1.122 // System.out.println("====> leaving scope " + this.hashCode() + " owned by " + this.owner + " to " + next.hashCode()); 1.123 // new Error().printStackTrace(System.out); 1.124 return next; 1.125 @@ -215,19 +217,17 @@ 1.126 s.hashMask = newtable.length - 1; 1.127 } 1.128 } 1.129 - for (int i = 0; i < newtable.length; i++) newtable[i] = sentinel; 1.130 - for (int i = 0; i < oldtable.length; i++) copy(oldtable[i]); 1.131 - } 1.132 - 1.133 - /** Copy the given entry and all entries shadowed by it to table 1.134 - */ 1.135 - private void copy(Entry e) { 1.136 - if (e.sym != null) { 1.137 - copy(e.shadowed); 1.138 - int hash = e.sym.name.hashCode() & hashMask; 1.139 - e.shadowed = table[hash]; 1.140 - table[hash] = e; 1.141 + int n = 0; 1.142 + for (int i = oldtable.length; --i >= 0; ) { 1.143 + Entry e = oldtable[i]; 1.144 + if (e != null && e != sentinel && ! e.isBogus()) { 1.145 + table[getIndex(e.sym.name)] = e; 1.146 + n++; 1.147 + } 1.148 } 1.149 + // We don't need to update nelems for shared inherited scopes, 1.150 + // since that gets handled by leave(). 1.151 + nelems = n; 1.152 } 1.153 1.154 /** Enter symbol sym in this scope. 1.155 @@ -248,13 +248,17 @@ 1.156 */ 1.157 public void enter(Symbol sym, Scope s, Scope origin) { 1.158 assert shared == 0; 1.159 - // Temporarily disabled (bug 6460352): 1.160 - // if (nelems * 3 >= hashMask * 2) dble(); 1.161 - int hash = sym.name.hashCode() & hashMask; 1.162 - Entry e = makeEntry(sym, table[hash], elems, s, origin); 1.163 + if (nelems * 3 >= hashMask * 2) 1.164 + dble(); 1.165 + int hash = getIndex(sym.name); 1.166 + Entry old = table[hash]; 1.167 + if (old == null) { 1.168 + old = sentinel; 1.169 + nelems++; 1.170 + } 1.171 + Entry e = makeEntry(sym, old, elems, s, origin); 1.172 table[hash] = e; 1.173 elems = e; 1.174 - nelems++; 1.175 scopeCounter.inc(); 1.176 } 1.177 1.178 @@ -268,15 +272,15 @@ 1.179 public void remove(Symbol sym) { 1.180 assert shared == 0; 1.181 Entry e = lookup(sym.name); 1.182 - while (e.scope == this && e.sym != sym) e = e.next(); 1.183 if (e.scope == null) return; 1.184 1.185 scopeCounter.inc(); 1.186 1.187 // remove e from table and shadowed list; 1.188 - Entry te = table[sym.name.hashCode() & hashMask]; 1.189 + int i = getIndex(sym.name); 1.190 + Entry te = table[i]; 1.191 if (te == e) 1.192 - table[sym.name.hashCode() & hashMask] = e.shadowed; 1.193 + table[i] = e.shadowed; 1.194 else while (true) { 1.195 if (te.shadowed == e) { 1.196 te.shadowed = e.shadowed; 1.197 @@ -335,12 +339,50 @@ 1.198 return lookup(name, noFilter); 1.199 } 1.200 public Entry lookup(Name name, Filter<Symbol> sf) { 1.201 - Entry e = table[name.hashCode() & hashMask]; 1.202 + Entry e = table[getIndex(name)]; 1.203 + if (e == null || e == sentinel) 1.204 + return sentinel; 1.205 while (e.scope != null && (e.sym.name != name || !sf.accepts(e.sym))) 1.206 e = e.shadowed; 1.207 return e; 1.208 } 1.209 1.210 + /*void dump (java.io.PrintStream out) { 1.211 + out.println(this); 1.212 + for (int l=0; l < table.length; l++) { 1.213 + Entry le = table[l]; 1.214 + out.print("#"+l+": "); 1.215 + if (le==sentinel) out.println("sentinel"); 1.216 + else if(le == null) out.println("null"); 1.217 + else out.println(""+le+" s:"+le.sym); 1.218 + } 1.219 + }*/ 1.220 + 1.221 + /** Look for slot in the table. 1.222 + * We use open addressing with double hashing. 1.223 + */ 1.224 + int getIndex (Name name) { 1.225 + int h = name.hashCode(); 1.226 + int i = h & hashMask; 1.227 + // The expression below is always odd, so it is guaranteed 1.228 + // be be mutually prime with table.length, a power of 2. 1.229 + int x = hashMask - ((h + (h >> 16)) << 1); 1.230 + int d = -1; // Index of a deleted item. 1.231 + for (;;) { 1.232 + Entry e = table[i]; 1.233 + if (e == null) 1.234 + return d >= 0 ? d : i; 1.235 + if (e == sentinel) { 1.236 + // We have to keep searching even if we see a deleted item. 1.237 + // However, remember the index in case we fail to find the name. 1.238 + if (d < 0) 1.239 + d = i; 1.240 + } else if (e.sym.name == name) 1.241 + return i; 1.242 + i = (i + x) & hashMask; 1.243 + } 1.244 + } 1.245 + 1.246 public Iterable<Symbol> getElements() { 1.247 return getElements(noFilter); 1.248 } 1.249 @@ -441,10 +483,7 @@ 1.250 * outwards if not found in this scope. 1.251 */ 1.252 public Entry next() { 1.253 - Entry e = shadowed; 1.254 - while (e.scope != null && e.sym.name != sym.name) 1.255 - e = e.shadowed; 1.256 - return e; 1.257 + return shadowed; 1.258 } 1.259 1.260 public Scope getOrigin() { 1.261 @@ -456,6 +495,8 @@ 1.262 // in many cases. 1.263 return scope; 1.264 } 1.265 + 1.266 + protected boolean isBogus () { return false; } 1.267 } 1.268 1.269 public static class ImportScope extends Scope { 1.270 @@ -470,22 +511,10 @@ 1.271 } 1.272 1.273 public Entry lookup(Name name) { 1.274 - Entry e = table[name.hashCode() & hashMask]; 1.275 - while (e.scope != null && 1.276 - (e.sym.name != name || 1.277 - /* Since an inner class will show up in package and 1.278 - * import scopes until its inner class attribute has 1.279 - * been processed, we have to weed it out here. This 1.280 - * is done by comparing the owners of the entry's 1.281 - * scope and symbol fields. The scope field's owner 1.282 - * points to where the class originally was imported 1.283 - * from. The symbol field's owner points to where the 1.284 - * class is situated now. This can change when an 1.285 - * inner class is read (see ClassReader.enterClass). 1.286 - * By comparing the two fields we make sure that we do 1.287 - * not accidentally import an inner class that started 1.288 - * life as a flat class in a package. */ 1.289 - e.sym.owner != e.scope.owner)) 1.290 + Entry e = table[getIndex(name)]; 1.291 + if (e == null) 1.292 + return sentinel; 1.293 + while (e.isBogus()) 1.294 e = e.shadowed; 1.295 return e; 1.296 } 1.297 @@ -499,15 +528,33 @@ 1.298 } 1.299 public Entry next() { 1.300 Entry e = super.shadowed; 1.301 - while (e.scope != null && 1.302 - (e.sym.name != sym.name || 1.303 - e.sym.owner != e.scope.owner)) // see lookup() 1.304 + while (isBogus()) 1.305 e = e.shadowed; 1.306 return e; 1.307 } 1.308 1.309 @Override 1.310 public Scope getOrigin() { return origin; } 1.311 + 1.312 + /** 1.313 + * Is this a bogus inner-class import? 1.314 + * An inner class {@code Outer$Inner.class} read from a class file 1.315 + * starts out in a package scope under the name {@code Outer$Inner}, 1.316 + * which (if star-imported) gets copied to the import scope. 1.317 + * When the InnerClasses attribute is processed, the ClassSymbol 1.318 + * is renamed in place (to {@code Inner}), and the owner changed 1.319 + * to the {@code Outer} class. The ImportScope still has the old 1.320 + * Entry that was created and hashed as {@code "Outer$Inner"}, 1.321 + * but whose name was changed to {@code "Inner"}. This violates 1.322 + * the invariants for the Scope hash table, and so is pretty bogus. 1.323 + * When the symbol was renamed, it should have been removed from 1.324 + * the import scope (and not just the package scope); however, 1.325 + * doing so is difficult. A better fix would be to change 1.326 + * import scopes to indirectly reference package symbols, rather 1.327 + * than copy from them. 1.328 + * Until then, we detect and skip the bogus entries using this test. 1.329 + */ 1.330 + protected boolean isBogus () { return sym.owner != scope.owner; } 1.331 } 1.332 } 1.333
2.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 2.2 +++ b/test/tools/javac/6996626/Main.java Sat Nov 06 13:53:48 2010 -0700 2.3 @@ -0,0 +1,45 @@ 2.4 +/* 2.5 + * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. 2.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 2.7 + * 2.8 + * This code is free software; you can redistribute it and/or modify it 2.9 + * under the terms of the GNU General Public License version 2 only, as 2.10 + * published by the Free Software Foundation. 2.11 + * 2.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 2.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 2.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 2.15 + * version 2 for more details (a copy is included in the LICENSE file that 2.16 + * accompanied this code). 2.17 + * 2.18 + * You should have received a copy of the GNU General Public License version 2.19 + * 2 along with this work; if not, write to the Free Software Foundation, 2.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 2.21 + * 2.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 2.23 + * or visit www.oracle.com if you need additional information or have any 2.24 + * questions. 2.25 + */ 2.26 + 2.27 +/* @test 2.28 + * @bug 6996626 2.29 + * @summary Scope fix issues for ImportScope 2.30 + * @compile pack1/Symbol.java 2.31 + * @compile Main.java 2.32 + */ 2.33 + 2.34 +import pack1.*; 2.35 +import pack1.Symbol.*; 2.36 + 2.37 +// The following imports are just to trigger re-hashing (in 2.38 +// com.sun.tools.javac.code.Scope.dble()) of the star-import scope. 2.39 +import java.io.*; 2.40 +import java.net.*; 2.41 +import java.util.*; 2.42 + 2.43 +public class Main { 2.44 + public void main (String[] args) { 2.45 + throw new CompletionFailure(); 2.46 + } 2.47 +} 2.48 +
3.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 3.2 +++ b/test/tools/javac/6996626/pack1/Symbol.java Sat Nov 06 13:53:48 2010 -0700 3.3 @@ -0,0 +1,31 @@ 3.4 +/* 3.5 + * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. 3.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 3.7 + * 3.8 + * This code is free software; you can redistribute it and/or modify it 3.9 + * under the terms of the GNU General Public License version 2 only, as 3.10 + * published by the Free Software Foundation. 3.11 + * 3.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 3.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 3.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 3.15 + * version 2 for more details (a copy is included in the LICENSE file that 3.16 + * accompanied this code). 3.17 + * 3.18 + * You should have received a copy of the GNU General Public License version 3.19 + * 2 along with this work; if not, write to the Free Software Foundation, 3.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 3.21 + * 3.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 3.23 + * or visit www.oracle.com if you need additional information or have any 3.24 + * questions. 3.25 + */ 3.26 + 3.27 +package pack1; 3.28 + 3.29 +public class Symbol { 3.30 + public static class CompletionFailure extends RuntimeException { } 3.31 +} 3.32 + 3.33 + 3.34 +