8034954: Optimistic iteration in for-in and for-each
authorattila
Wed, 10 Sep 2014 13:08:58 +0200
changeset 26507 9d6e3ec59878
parent 26506 7d55cc7fc301
child 26508 b40ef4386b01
8034954: Optimistic iteration in for-in and for-each Reviewed-by: hannesw, lagergren
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Compiler.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/TypeEvaluator.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java	Wed Sep 10 12:37:44 2014 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java	Wed Sep 10 13:08:58 2014 +0200
@@ -1622,9 +1622,18 @@
 
             @Override
             protected void evaluate() {
-                method.load(ITERATOR_TYPE, iterSlot);
-                // TODO: optimistic for-in iteration
-                method.invoke(interfaceCallNoLookup(ITERATOR_CLASS, "next", Object.class));
+                new OptimisticOperation((Optimistic)forNode.getInit(), TypeBounds.UNBOUNDED) {
+                    @Override
+                    void loadStack() {
+                        method.load(ITERATOR_TYPE, iterSlot);
+                    }
+
+                    @Override
+                    void consumeStack() {
+                        method.invoke(interfaceCallNoLookup(ITERATOR_CLASS, "next", Object.class));
+                        convertOptimisticReturnValue();
+                    }
+                }.emit();
             }
         }.store();
         body.accept(this);
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Compiler.java	Wed Sep 10 12:37:44 2014 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Compiler.java	Wed Sep 10 13:08:58 2014 +0200
@@ -51,6 +51,7 @@
 import java.util.logging.Level;
 import jdk.internal.dynalink.support.NameCodec;
 import jdk.nashorn.internal.codegen.types.Type;
+import jdk.nashorn.internal.ir.Expression;
 import jdk.nashorn.internal.ir.FunctionNode;
 import jdk.nashorn.internal.ir.Optimistic;
 import jdk.nashorn.internal.ir.debug.ClassHistogramElement;
@@ -63,6 +64,7 @@
 import jdk.nashorn.internal.runtime.RecompilableScriptFunctionData;
 import jdk.nashorn.internal.runtime.ScriptEnvironment;
 import jdk.nashorn.internal.runtime.ScriptObject;
+import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.Source;
 import jdk.nashorn.internal.runtime.logging.DebugLogger;
 import jdk.nashorn.internal.runtime.logging.Loggable;
@@ -480,6 +482,19 @@
         return typeEvaluator.getOptimisticType(node);
     }
 
+    /**
+     * Returns true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property names retrieved through
+     * {@link ScriptRuntime#toPropertyIterator(Object)}. It is used to avoid optimistic assumptions about its
+     * property name types.
+     * @param expr the expression to test
+     * @return true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property iterators.
+     */
+    boolean hasStringPropertyIterator(final Expression expr) {
+        return typeEvaluator.hasStringPropertyIterator(expr);
+    }
+
     void addInvalidatedProgramPoint(final int programPoint, final Type type) {
         invalidatedProgramPoints.put(programPoint, type);
     }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java	Wed Sep 10 12:37:44 2014 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java	Wed Sep 10 13:08:58 2014 +0200
@@ -551,13 +551,19 @@
 
         final Expression init = forNode.getInit();
         if(forNode.isForIn()) {
-            forNode.getModify().accept(this);
-            enterTestFirstLoop(forNode, null, init);
+            final JoinPredecessorExpression iterable = forNode.getModify();
+            iterable.accept(this);
+            enterTestFirstLoop(forNode, null, init,
+                    // If we're iterating over property names, and we can discern from the runtime environment
+                    // of the compilation that the object being iterated over must use strings for property
+                    // names (e.g., it is a native JS object or array), then we'll not bother trying to treat
+                    // the property names optimistically.
+                    !forNode.isForEach() && compiler.hasStringPropertyIterator(iterable.getExpression()));
         } else {
             if(init != null) {
                 init.accept(this);
             }
-            enterTestFirstLoop(forNode, forNode.getModify(), null);
+            enterTestFirstLoop(forNode, forNode.getModify(), null, false);
         }
         return false;
     }
@@ -792,7 +798,8 @@
         return false;
     }
 
-    private void enterTestFirstLoop(final LoopNode loopNode, final JoinPredecessorExpression modify, final Expression iteratorValues) {
+    private void enterTestFirstLoop(final LoopNode loopNode, final JoinPredecessorExpression modify,
+            final Expression iteratorValues, final boolean iteratorValuesAreObject) {
         final JoinPredecessorExpression test = loopNode.getTest();
         if(isAlwaysFalse(test)) {
             test.accept(this);
@@ -814,8 +821,12 @@
                 jumpToLabel(test, breakLabel);
             }
             if(iteratorValues instanceof IdentNode) {
-                // Receives iterator values; they're currently all objects (JDK-8034954).
-                onAssignment((IdentNode)iteratorValues, LvarType.OBJECT);
+                final IdentNode ident = (IdentNode)iteratorValues;
+                // Receives iterator values; the optimistic type of the iterator values is tracked on the
+                // identifier, but we override optimism if it's known that the object being iterated over will
+                // never have primitive property names.
+                onAssignment(ident, iteratorValuesAreObject ? LvarType.OBJECT :
+                    toLvarType(compiler.getOptimisticType(ident)));
             }
             final Block body = loopNode.getBody();
             body.accept(this);
@@ -955,7 +966,7 @@
         if(whileNode.isDoWhile()) {
             enterDoWhileLoop(whileNode);
         } else {
-            enterTestFirstLoop(whileNode, null, null);
+            enterTestFirstLoop(whileNode, null, null, false);
         }
         return false;
     }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/TypeEvaluator.java	Wed Sep 10 12:37:44 2014 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/TypeEvaluator.java	Wed Sep 10 13:08:58 2014 +0200
@@ -55,6 +55,19 @@
         this.runtimeScope = runtimeScope;
     }
 
+    /**
+     * Returns true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property names retrieved through
+     * {@link ScriptRuntime#toPropertyIterator(Object)}. It is used to avoid optimistic assumptions about its
+     * property name types.
+     * @param expr the expression to test
+     * @return true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property iterators.
+     */
+    boolean hasStringPropertyIterator(final Expression expr) {
+        return evaluateSafely(expr) instanceof ScriptObject;
+    }
+
     Type getOptimisticType(final Optimistic node) {
         assert compiler.useOptimisticTypes();