8005976: Break out AccessSpecializer into one pass before CodeGenerator instead of iterative applications from CodeGenerator
authorlagergren
Fri, 11 Jan 2013 10:40:51 +0100
changeset 16168 f0c208287983
parent 16167 d99db3541813
child 16169 45718b3a87eb
8005976: Break out AccessSpecializer into one pass before CodeGenerator instead of iterative applications from CodeGenerator Summary: Now scope and slot information is guaranteed to be fixed AND NOT CHANGE before CodeGeneration. We want to keep it that way to build future type specializations and bring all type work out of CodeGenerator. Reviewed-by: attila, hannesw
nashorn/bin/dump_octane_code.sh
nashorn/bin/verbose_octane.sh
nashorn/docs/DEVELOPER_README
nashorn/src/jdk/nashorn/internal/codegen/AccessSpecializer.java
nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java
nashorn/src/jdk/nashorn/internal/codegen/Compiler.java
nashorn/src/jdk/nashorn/internal/codegen/Lower.java
nashorn/src/jdk/nashorn/internal/codegen/Splitter.java
nashorn/src/jdk/nashorn/internal/codegen/Transform.java
nashorn/src/jdk/nashorn/internal/ir/Block.java
nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java
nashorn/src/jdk/nashorn/internal/ir/Symbol.java
nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/bin/dump_octane_code.sh	Fri Jan 11 10:40:51 2013 +0100
@@ -0,0 +1,53 @@
+#!/bin/bash
+# Copyright (c) 2010, 2013, 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.
+#
+
+#
+# The purpose of this script is to provide a large amount of IR/bytecode from a known
+# application to be diffed against the same output with a different Nashorn version.
+# That way we can quickly detect if a seemingly minute change modifies a lot of code,
+# which it most likely shouldn't. One example of this was when AccessSpecializer was
+# moved into Lower the first time, it worked fine, but as a lot of Scope information
+# at the time was finalized further down the code pipeline it did a lot fewer callsite
+# specializations. This would have been immediately detected with a before and after 
+# diff using the output from this script.
+#
+
+ITERS=$1
+if [ -z $ITERS ]; then 
+    ITERS=7
+fi
+NASHORN_JAR=dist/nashorn.jar
+JVM_FLAGS="-ea -esa -server -jar ${NASHORN_JAR}"
+
+BENCHMARKS=( "box2d.js" "code-load.js" "crypto.js" "deltablue.js" "earley-boyer.js" "gbemu.js" "mandreel.js" "navier-stokes.js" "pdfjs.js" "raytrace.js" "regexp.js" "richards.js" "splay.js" )
+
+for BENCHMARK in "${BENCHMARKS[@]}"
+do     
+    echo "START: ${BENCHMARK}"
+    CMD="${JAVA_HOME}/bin/java ${JVM_FLAGS} -co --print-lower-parse test/script/external/octane/${BENCHMARK}"
+    $CMD
+    echo "END: ${BENCHMARK}"
+    echo ""
+done
+
+echo "Done"
--- a/nashorn/bin/verbose_octane.sh	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/bin/verbose_octane.sh	Fri Jan 11 10:40:51 2013 +0100
@@ -26,18 +26,18 @@
     ITERS=7
 fi
 NASHORN_JAR=dist/nashorn.jar
-JVM_FLAGS="-XX:+UnlockDiagnosticVMOptions -Dnashorn.unstable.relink.threshold=8 -Xms2G -Xmx2G -XX:-TieredCompilation -server -jar ${NASHORN_JAR}"
+JVM_FLAGS="-XX:+UnlockDiagnosticVMOptions -Dnashorn.unstable.relink.threshold=8 -Xms2G -Xmx2G -XX:+TieredCompilation -server -jar ${NASHORN_JAR}"
 JVM_FLAGS7="-Xbootclasspath/p:${NASHORN_JAR} ${JVM_FLAGS}"
 OCTANE_ARGS="--verbose --iterations ${ITERS}"
 
-BENCHMARKS=( "box2d.js" "code-load.js" "crypto.js" "deltablue.js" "earley-boyer.js" "gbemu.js" "navier-stokes.js" "raytrace.js" "regexp.js" "richards.js" "splay.js" )
+BENCHMARKS=( "box2d.js" "code-load.js" "crypto.js" "deltablue.js" "earley-boyer.js" "gbemu.js" "navier-stokes.js" "pdfjs.js" "raytrace.js" "regexp.js" "richards.js" "splay.js" )
 # TODO mandreel.js has metaspace issues
 
 if [ ! -z $JAVA7_HOME ]; then	
     echo "running ${ITERS} iterations with java7 using JAVA_HOME=${JAVA7_HOME}..."
     for BENCHMARK in "${BENCHMARKS[@]}"
     do 
-	CMD="${JAVA8_HOME}/bin/java ${JVM_FLAGS} test/script/basic/run-octane.js -- test/script/external/octane/${BENCHMARK} ${OCTANE_ARGS}"
+	CMD="${JAVA7_HOME}/bin/java ${JVM_FLAGS} test/script/basic/run-octane.js -- test/script/external/octane/${BENCHMARK} ${OCTANE_ARGS}"
 	$CMD
     done
 else
@@ -52,7 +52,7 @@
 	$CMD
     done
 else 
-    echo "no JAVA8_HOME set"
+    echo "no JAVA8_HOME set."
 fi
 
 echo "Done"
--- a/nashorn/docs/DEVELOPER_README	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/docs/DEVELOPER_README	Fri Jan 11 10:40:51 2013 +0100
@@ -23,6 +23,13 @@
 is not explicitly set.
 
 
+SYSTEM PROPERTY: -Dnashorn.compiler.split.threshold=x
+
+This will change the node weight that requires a subgraph of the IR to
+be split into several classes in order not to run out of bytecode space.
+The default value is 0x8000 (32768).
+
+
 SYSTEM PROPERTY: -Dnashorn.callsiteaccess.debug
 
 See the description of the access logger below. This flag is
@@ -451,4 +458,4 @@
 getter and setter in the program, show arguments, return values
 etc. It will also show the internal representation of respective field
 (Object in the normal case, unless running with the dual field
-representation)
\ No newline at end of file
+representation)
--- a/nashorn/src/jdk/nashorn/internal/codegen/AccessSpecializer.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/codegen/AccessSpecializer.java	Fri Jan 11 10:40:51 2013 +0100
@@ -31,7 +31,6 @@
 import jdk.nashorn.internal.ir.Assignment;
 import jdk.nashorn.internal.ir.BinaryNode;
 import jdk.nashorn.internal.ir.CallNode;
-import jdk.nashorn.internal.ir.FunctionNode;
 import jdk.nashorn.internal.ir.IdentNode;
 import jdk.nashorn.internal.ir.IndexNode;
 import jdk.nashorn.internal.ir.Node;
@@ -64,28 +63,13 @@
  * @see TypeOverride
  */
 
-final class AccessSpecializer extends NodeOperatorVisitor implements Transform {
+final class AccessSpecializer extends NodeOperatorVisitor {
     /** Debug logger for access specialization. Enable it with --log=access:level
-        or -Dnashorn.specializations.debug */
+        or -Dnashorn.callsiteaccess.debug */
     private static final DebugLogger LOG   = new DebugLogger("access", "nashorn.callsiteaccess.debug");
     private static final boolean     DEBUG = LOG.isEnabled();
 
     @Override
-    public Node enter(final FunctionNode node) {
-        if (node.isTransformApplied(AccessSpecializer.class)) {
-            return null;
-        }
-
-        return node;
-    }
-
-    @Override
-    public Node leave(final FunctionNode node) {
-        node.registerTransform(AccessSpecializer.class);
-        return node;
-    }
-
-    @Override
     public Node leave(final VarNode varNode) {
         if (varNode.isAssignment()) {
             return leaveAssign(varNode);
@@ -191,7 +175,7 @@
              * getter/setter is selected, which will do the conversion for us
              */
             changeType(rhs, castTo);
-            fine("*** cast: converting " + debugNode(unaryNode) + " to " + debugNode(rhs));
+            LOG.fine("*** cast: converting " + debugNode(unaryNode) + " to " + debugNode(rhs));
 
             return rhs;
         }
@@ -270,17 +254,17 @@
         final Symbol lhsSymbol = lhs.getSymbol();
 
         if (lhsSymbol.hasSlot() && !lhsSymbol.isScope()) {
-            finest(lhs.getSymbol() + " has slot!");
+            LOG.finest(lhs.getSymbol() + " has slot!");
             if (!lhs.getType().isEquivalentTo(rhs.getType())) {
-                finest("\tslot assignment: " +lhs.getType()+ " " +rhs.getType() + " " + debugNode(node));
+                LOG.finest("\tslot assignment: " +lhs.getType()+ " " +rhs.getType() + " " + debugNode(node));
 
                 final Node c = convert(rhs);
                 c.setSymbol(lhsSymbol);
                 ((Assignment<?>)node).setAssignmentSource(c);
 
-                fine("*** slot assignment turned to : " + debugNode(node));
+                LOG.fine("*** slot assignment turned to : " + debugNode(node));
             } else {
-                finest("aborted - type equivalence between lhs and rhs");
+                LOG.finest("aborted - type equivalence between lhs and rhs");
             }
 
             return node;
@@ -310,10 +294,10 @@
 
         // If castTo is wider than widestOperationType, castTo can be further slowed down
         final Type widestOperationType = node.getWidestOperationType();
-        finest("node wants to be " + castTo + " and its widest operation is " + widestOperationType);
+        LOG.finest("node wants to be " + castTo + " and its widest operation is " + widestOperationType);
 
         if (widestOperationType != castTo && Type.widest(castTo, widestOperationType) == castTo) {
-            info("###" + node + " castTo was " + castTo + " but could be downgraded to " + node.getWidestOperationType());
+            LOG.info("###" + node + " castTo was " + castTo + " but could be downgraded to " + node.getWidestOperationType());
             castTo = node.getWidestOperationType();
             if (rhs instanceof TypeOverride) {
                 changeType(rhs, castTo);
@@ -331,16 +315,16 @@
         // We only specialize for numerics, not for booleans.
         if (isSupportedCallSiteType(castTo)) {
             if (rhs.getType() != castTo) {
-                finest("cast was necessary, abort: " + node + " " + rhs.getType() + " != " + castTo);
+                LOG.finest("cast was necessary, abort: " + node + " " + rhs.getType() + " != " + castTo);
                 return node;
             }
 
-            finest("assign: " + debugNode(node));
+            LOG.finest("assign: " + debugNode(node));
 
             changeTypeInAssignment(lhs, castTo);
             ((Assignment<?>)node).setAssignmentSource(rhs);
 
-            info("### modified to " + debugNode(node) + " (given type override " + castTo + ")");
+            LOG.info("### modified to " + debugNode(node) + " (given type override " + castTo + ")");
 
             propagateResultType(node, castTo);
         }
@@ -352,9 +336,9 @@
         //warning! this CANNOT be done for non temporaries as they are used in other computations
         if (isSupportedCallSiteType(type)) {
             if (node.getSymbol().isTemp()) {
-                finest("changing temporary type: " + debugNode(node) + " to " + type);
+                LOG.finest("changing temporary type: " + debugNode(node) + " to " + type);
                 node.getSymbol().setTypeOverride(type);
-                info("### node modified to " + debugNode(node) + " (given type override " + type + ")");
+                LOG.info("### node modified to " + debugNode(node) + " (given type override " + type + ")");
             }
         }
         return node;
@@ -362,7 +346,7 @@
 
     private static void changeTypeInAssignment(final Node dest, final Type newType) {
         if (changeType(dest, newType)) {
-            finest("changed assignment " + dest + " " + dest.getSymbol());
+            LOG.finest("changed assignment " + dest + " " + dest.getSymbol());
             assert !newType.isObject();
 
             final HashSet<Node> exclude = new HashSet<>();
@@ -370,7 +354,7 @@
             dest.accept(new NodeVisitor() {
 
                 private void setCanBePrimitive(final Symbol symbol) {
-                    fine("*** can be primitive symbol " + symbol + " " + Debug.id(symbol));
+                    LOG.fine("*** can be primitive symbol " + symbol + " " + Debug.id(symbol));
                     symbol.setCanBePrimitive(newType);
                 }
 
@@ -412,17 +396,4 @@
         }
         return "";
     }
-
-    private static void info(final String str) {
-        LOG.info(str);
-    }
-
-    private static void fine(final String str) {
-        LOG.fine(str);
-    }
-
-    private static void finest(final String str) {
-        LOG.finest(str);
-    }
-
 }
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Fri Jan 11 10:40:51 2013 +0100
@@ -845,16 +845,12 @@
         return null;
     }
 
-    private void initLocals(final Block block) {
-        initLocals(block, null);
-    }
-
     /**
      * Initialize the slots in a frame to undefined.
      *
      * @param block block with local vars.
      */
-    private void initLocals(final Block block, final AccessSpecializer accessSpecializer) {
+    private void initLocals(final Block block) {
         final FunctionNode function       = block.getFunction();
         final boolean      isFunctionNode = block == function;
 
@@ -885,14 +881,6 @@
             final List<String> nameList = new ArrayList<>();
             final List<Symbol> locals   = new ArrayList<>();
 
-            // note that in the case of a varargs vector the arguments in the vector should NOT have local slots
-            // this was a bug: see NASHORN-21
-            if (isVarArg) {
-                for (final IdentNode param : function.getParameters()) {
-                    param.getSymbol().setNeedsSlot(false);
-                }
-            }
-
             // If there are variable arguments, we need to load them (functions only).
             if (isFunctionNode && isVarArg) {
                 method.loadVarArgs();
@@ -915,8 +903,8 @@
                     nameList.add(symbol.getName());
                     newSymbols.add(symbol);
                     values.add(null);
-                    symbol.setIsScope();
-                    symbol.setNeedsSlot(false);
+                    assert symbol.isScope()   : "scope for " + symbol + " should have been set in Lower already " + function.getName();
+                    assert !symbol.hasSlot()  : "slot for " + symbol + " should have been removed in Lower already" + function.getName();
                 } else if (symbol.isVar()) {
                     assert symbol.hasSlot() : symbol + " should have a slot only, no scope";
                     locals.add(symbol);
@@ -924,10 +912,8 @@
                     nameList.add(symbol.getName());
                     newSymbols.add(symbol);
                     values.add(isVarArg ? null : symbol);
-                    symbol.setIsScope();
-                    if (isVarArg) {
-                        symbol.setNeedsSlot(false);
-                    }
+                    assert symbol.isScope()   : "scope for " + symbol + " should have been set in Lower already " + function.getName() + " varsInScope="+varsInScope+" isVarArg="+isVarArg+" symbol.isScope()=" + symbol.isScope();
+                    assert !(isVarArg && symbol.hasSlot())  : "slot for " + symbol + " should have been removed in Lower already " + function.getName();
                 }
             }
 
@@ -941,10 +927,6 @@
                 initScope();
             }
 
-            if (accessSpecializer != null) {
-                ((FunctionNode)block).accept(accessSpecializer);
-            }
-
             /*
              * Create a new object based on the symbols and values, generate
              * bootstrap code for object
@@ -1001,7 +983,7 @@
         method.begin();
         method.label(functionNode.getEntryLabel());
 
-        initLocals(functionNode, new AccessSpecializer());
+        initLocals(functionNode);
 
         return functionNode;
     }
--- a/nashorn/src/jdk/nashorn/internal/codegen/Compiler.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Compiler.java	Fri Jan 11 10:40:51 2013 +0100
@@ -298,6 +298,8 @@
 
         /** do we need to parse source? */
         if (!state.contains(State.PARSED)) {
+            LOG.info("Parsing '" + source + "'");
+
             assert this.functionNode == null;
             this.functionNode = new Parser(this, strict).parse(RUN_SCRIPT.tag());
 
@@ -338,6 +340,7 @@
 
             if (!state.contains(State.LOWERED)) {
                 debugPrintAST();
+                LOG.info("Lowering '" + functionNode.getName() + "'");
                 functionNode.accept(new Lower(this));
                 state.add(State.LOWERED);
 
@@ -352,6 +355,7 @@
             // and add script code last this class may end up slightly larger than others, but reserving one class
             // just for the main script seems wasteful.
             final CompileUnit scriptCompileUnit = addCompileUnit(firstCompileUnitName(), 0l);
+            LOG.info("Splitting '" + functionNode.getName() + "'");
             new Splitter(this, functionNode, scriptCompileUnit).split();
             assert functionNode.getCompileUnit() == scriptCompileUnit;
 
@@ -361,7 +365,14 @@
                 strict = true;
             }
 
+            LOG.info("Adjusting slot and scope information for symbols for '" + functionNode.getName() + "'");
+            functionNode.accept(new Lower.FinalizeSymbols());
+
+            LOG.info("Specializing callsite types for '" + functionNode.getName() + "'");
+            functionNode.accept(new AccessSpecializer());
+
             try {
+                LOG.info("Emitting bytecode for '" + functionNode.getName() + "'");
                 final CodeGenerator codegen = new CodeGenerator(this);
                 functionNode.accept(codegen);
                 codegen.generateScopeCalls();
@@ -408,6 +419,7 @@
             return true;
         } finally {
             strict = oldStrict;
+            LOG.info("Done with '" + functionNode.getName() + "'");
         }
     }
 
--- a/nashorn/src/jdk/nashorn/internal/codegen/Lower.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Lower.java	Fri Jan 11 10:40:51 2013 +0100
@@ -916,6 +916,13 @@
         } else {
             LOG.info("parameter specialization possible: " + functionNode.getName() + " " + paramSpecializations);
         }
+
+        // parameters should not be slots for a vararg function, make sure this is the case
+        if (functionNode.isVarArg()) {
+            for (final IdentNode param : functionNode.getParameters()) {
+                param.getSymbol().setNeedsSlot(false);
+            }
+        }
     }
 
     private LiteralNode<Undefined> undefined() {
@@ -1086,6 +1093,7 @@
 
     @Override
     public Node enter(final FunctionNode functionNode) {
+        LOG.info("START FunctionNode: " + functionNode.getName());
 
         Node initialEvalResult = undefined();
 
@@ -1255,6 +1263,8 @@
         functionNode.setIsLowered(true);
         functionNode.popFrame();
 
+        LOG.info("END FunctionNode: " + functionNode.getName());
+
         return null;
     }
 
@@ -1482,9 +1492,9 @@
 
     @Override
     public Node enter(final ReferenceNode referenceNode) {
-        final FunctionNode fn = referenceNode.getReference();
-        if (fn != null) {
-            fn.addReferencingParentBlock(getCurrentBlock());
+        final FunctionNode functionNode = referenceNode.getReference();
+        if (functionNode != null) {
+            functionNode.addReferencingParentBlock(getCurrentBlock());
         }
         return referenceNode;
     }
@@ -2869,6 +2879,74 @@
     }
 
     /**
+     * A simple node visitor that ensure that scope and slot information is correct.
+     * This is run as a post pass after we know all scope changing information about
+     * the Lowering. This is also called after potential mutations like splitting
+     * have taken place, as splitting forces scope.
+     *
+     * This was previously done on a per functionNode basis in {@link CodeGenerator},
+     * but this is too late for type information to be used in {@link AccessSpecializer}
+     */
+    static class FinalizeSymbols extends NodeVisitor {
+        @Override
+        public Node leave(final Block block) {
+            return updateSymbols(block);
+        }
+
+        @Override
+        public Node leave(final FunctionNode function) {
+            return updateSymbols(function);
+        }
+
+        private static void updateSymbolsLog(final FunctionNode functionNode, final Symbol symbol, final boolean loseSlot) {
+            if (!symbol.isScope()) {
+                LOG.finest("updateSymbols: " + symbol + " => scope, because all vars in " + functionNode.getName() + " are in scope");
+            }
+            if (loseSlot && symbol.hasSlot()) {
+                LOG.finest("updateSymbols: " + symbol + " => no slot, because all vars in " + functionNode.getName() + " are in scope");
+            }
+        }
+
+        // called after a block or function node (subclass of block) is finished
+        // to correct scope and slot assignment for variables
+        private static Block updateSymbols(final Block block) {
+
+            if (!block.needsScope()) {
+                return block; // nothing to do
+            }
+
+            assert !(block instanceof FunctionNode) || block.getFunction() == block;
+
+            final FunctionNode functionNode   = block.getFunction();
+            final List<Symbol> symbols        = block.getFrame().getSymbols();
+            final boolean      allVarsInScope = functionNode.varsInScope();
+            final boolean      isVarArg       = functionNode.isVarArg();
+
+            for (final Symbol symbol : symbols) {
+                if (symbol.isInternal() || symbol.isThis()) {
+                    continue;
+                }
+
+                if (symbol.isVar()) {
+                    if (allVarsInScope || symbol.isScope()) {
+                        updateSymbolsLog(functionNode, symbol, true);
+                        symbol.setIsScope();
+                        symbol.setNeedsSlot(false);
+                    } else {
+                        assert symbol.hasSlot() : symbol + " should have a slot only, no scope";
+                    }
+                } else if (symbol.isParam() && (allVarsInScope || isVarArg || symbol.isScope())) {
+                    updateSymbolsLog(functionNode, symbol, isVarArg);
+                    symbol.setIsScope();
+                    symbol.setNeedsSlot(!isVarArg);
+                }
+            }
+
+            return block;
+        }
+    }
+
+    /**
      * Helper class to evaluate constant expressions at compile time This is
      * also a simplifier used by BinaryNode visits, UnaryNode visits and
      * conversions.
--- a/nashorn/src/jdk/nashorn/internal/codegen/Splitter.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Splitter.java	Fri Jan 11 10:40:51 2013 +0100
@@ -50,6 +50,7 @@
 import jdk.nashorn.internal.ir.WhileNode;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
+import jdk.nashorn.internal.runtime.options.Options;
 
 /**
  * Split the IR into smaller compile units.
@@ -68,7 +69,7 @@
     private final Map<Node, Long> weightCache = new HashMap<>();
 
     /** Weight threshold for when to start a split. */
-    public static final long SPLIT_THRESHOLD  = 32 * 1024;
+    public static final long SPLIT_THRESHOLD = Options.getIntProperty("nashorn.compiler.splitter.threshold", 32 * 1024);
 
     /**
      * Constructor.
@@ -90,6 +91,7 @@
         long weight = WeighNodes.weigh(functionNode);
 
         if (weight >= SPLIT_THRESHOLD) {
+            Compiler.LOG.info("Splitting '" + functionNode.getName() + "' as its weight " + weight + " exceeds split threshold " + SPLIT_THRESHOLD);
 
             functionNode.accept(this);
 
--- a/nashorn/src/jdk/nashorn/internal/codegen/Transform.java	Thu Jan 10 15:28:05 2013 +0100
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,33 +0,0 @@
-/*
- * Copyright (c) 2010, 2013, 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.  Oracle designates this
- * particular file as subject to the "Classpath" exception as provided
- * by Oracle in the LICENSE file that accompanied this code.
- *
- * 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.
- */
-
-package jdk.nashorn.internal.codegen;
-
-/**
- * Code generation transform
- */
-public interface Transform {
-    //empty
-}
--- a/nashorn/src/jdk/nashorn/internal/ir/Block.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/ir/Block.java	Fri Jan 11 10:40:51 2013 +0100
@@ -582,12 +582,10 @@
     }
 
     /**
-     * Reset the needs scope flag.
-     *
-     * @param needsScope  new needs scope flag
+     * Set the needs scope flag.
      */
-    public void setNeedsScope(final boolean needsScope) {
-        this.needsScope = needsScope;
+    public void setNeedsScope() {
+        this.needsScope = true;
     }
 
 }
--- a/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java	Fri Jan 11 10:40:51 2013 +0100
@@ -43,7 +43,6 @@
 import jdk.nashorn.internal.codegen.MethodEmitter;
 import jdk.nashorn.internal.codegen.Namespace;
 import jdk.nashorn.internal.codegen.Splitter;
-import jdk.nashorn.internal.codegen.Transform;
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.annotations.Ignore;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
@@ -189,14 +188,9 @@
     /** Does this function need a scope object? */
     private static final int NEEDS_SCOPE           = HAS_ALL_VARS_IN_SCOPE | IS_VAR_ARG;
 
-
     /** What is the return type of this function? */
     private Type returnType = Type.OBJECT;
 
-    /** Transforms that have been applied to this function, a list as some transforms conceivably can run many times */
-    @Ignore
-    private final List<Class<? extends Transform>> appliedTransforms;
-
     /**
      * Used to keep track of a function's parent blocks.
      * This is needed when a (finally body) block is cloned than contains inner functions.
@@ -232,7 +226,6 @@
         this.labelStack        = new Stack<>();
         this.controlStack      = new Stack<>();
         this.declarations      = new ArrayList<>();
-        this.appliedTransforms = new ArrayList<>();
         // my block -> function is this. We added @SuppressWarnings("LeakingThisInConstructor") as NetBeans identifies
         // it as such a leak - this is a false positive as we're setting this into a field of the object being
         // constructed, so it can't be seen from other threads.
@@ -266,7 +259,6 @@
         this.labelStack        = new Stack<>();
         this.controlStack      = new Stack<>();
         this.declarations      = new ArrayList<>();
-        this.appliedTransforms = new ArrayList<>();
 
         for (final VarNode decl : functionNode.getDeclarations()) {
             declarations.add((VarNode) cs.existingOrCopy(decl)); //TODO same?
@@ -793,23 +785,6 @@
     }
 
     /**
-     * Check if a {@link Transform} has been taken place to this method.
-     * @param transform to check for
-     * @return true if transform has been applied
-     */
-    public boolean isTransformApplied(final Class<? extends Transform> transform) {
-        return appliedTransforms.contains(transform);
-    }
-
-    /**
-     * Tag this function with an applied transform
-     * @param transform the transform
-     */
-    public void registerTransform(final Class<? extends Transform> transform) {
-        appliedTransforms.add(transform);
-    }
-
-    /**
      * Checks if this function is a sub-function generated by splitting a larger one
      * @see Splitter
      *
--- a/nashorn/src/jdk/nashorn/internal/ir/Symbol.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/ir/Symbol.java	Fri Jan 11 10:40:51 2013 +0100
@@ -331,8 +331,11 @@
      * Flag this symbol as scope as described in {@link Symbol#isScope()}
      */
     public void setIsScope() {
+        if (!isScope()) {
+            trace("SET IS SCOPE");
+        }
         flags |= IS_SCOPE;
-        getBlock().setNeedsScope(true);
+        getBlock().setNeedsScope();
     }
 
     /**
@@ -550,8 +553,10 @@
      * @param slot valid bytecode slot, or -1 if not available
      */
     public void setSlot(final int slot) {
-        trace("SET SLOT " + slot);
-        this.slot = slot;
+        if (slot != this.slot) {
+            trace("SET SLOT " + slot);
+            this.slot = slot;
+        }
     }
 
     /**
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java	Thu Jan 10 15:28:05 2013 +0100
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java	Fri Jan 11 10:40:51 2013 +0100
@@ -1794,14 +1794,14 @@
                typeError(Context.getGlobal(), strictErrorMessage, name, ScriptRuntime.safeToString((this)));
            }
            assert canBeFastScope || !NashornCallSiteDescriptor.isFastScope(desc);
-           final PropertyMap map = getMap();
-           return new GuardedInvocation(Lookup.EMPTY_SETTER, map.getProtoGetSwitchPoint(name), NashornGuards.getMapGuard(map));
+           final PropertyMap myMap = getMap();
+           return new GuardedInvocation(Lookup.EMPTY_SETTER, myMap.getProtoGetSwitchPoint(name), NashornGuards.getMapGuard(myMap));
     }
 
     @SuppressWarnings("unused")
     private static void setEmbed(final CallSiteDescriptor desc, final PropertyMap oldMap, final PropertyMap newMap, final MethodHandle setter, final int i, final Object self, final Object value) throws Throwable {
         final ScriptObject obj = (ScriptObject)self;
-        if(obj.trySetEmbedOrSpill(desc, oldMap, newMap, value)) {
+        if (obj.trySetEmbedOrSpill(desc, oldMap, newMap, value)) {
             obj.useEmbed(i);
             setter.invokeExact(self, value);
         }
@@ -1810,7 +1810,7 @@
     @SuppressWarnings("unused")
     private static void setSpill(final CallSiteDescriptor desc, final PropertyMap oldMap, final PropertyMap newMap, final int index, final Object self, final Object value) {
         final ScriptObject obj = (ScriptObject)self;
-        if(obj.trySetEmbedOrSpill(desc, oldMap, newMap, value)) {
+        if (obj.trySetEmbedOrSpill(desc, oldMap, newMap, value)) {
             obj.spill[index] = value;
         }
     }