6998063: new Scope impl to fix Scope performance issues

Sat, 06 Nov 2010 13:53:48 -0700

author
jjg
date
Sat, 06 Nov 2010 13:53:48 -0700
changeset 738
9427a3c795fc
parent 737
e406f0645b7e
child 739
a0d9d642f65b

6998063: new Scope impl to fix Scope performance issues
Reviewed-by: jjg
Contributed-by: per.bothner@oracle.com

src/share/classes/com/sun/tools/javac/code/Scope.java file | annotate | diff | comparison | revisions
test/tools/javac/6996626/Main.java file | annotate | diff | comparison | revisions
test/tools/javac/6996626/pack1/Symbol.java file | annotate | diff | comparison | revisions
     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 +

mercurial