Merge pull request #105 in G/fastr from ~ADAM.WELC_ORACLE.COM/fastr:feature/promise_opt to master

* commit 'bb30b492':
  Fixes related to making EagerPromise a final class again.
  Modified option controlling inline caching for promises to allow specifying cache size.
  The EagerPromise class made final again
  Enforce guarantee that if no eager promise evaluation optimizations are used then no eager promises are created.
  A change to enable option-controlled cache-less evaluation of promises.
  Fixed a problem with evaluation of default promises.
  Made a "promised" promise a subclass of "eager" promise to enable "promised" promise stats.

com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/PromiseHelperNode.java

@@ -38,6 +38,7 @@ import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import com.oracle.truffle.api.profiles.ValueProfile;
 import com.oracle.truffle.r.nodes.InlineCacheNode;
+import com.oracle.truffle.r.runtime.FastROptions;
 import com.oracle.truffle.r.runtime.RArguments;
 import com.oracle.truffle.r.runtime.RCaller;
 import com.oracle.truffle.r.runtime.RError;
@@ -45,6 +46,7 @@ import com.oracle.truffle.r.runtime.RInternalError;
 import com.oracle.truffle.r.runtime.VirtualEvalFrame;
 import com.oracle.truffle.r.runtime.data.RPromise;
 import com.oracle.truffle.r.runtime.data.RPromise.EagerPromise;
+import com.oracle.truffle.r.runtime.data.RPromise.EagerPromiseBase;
 import com.oracle.truffle.r.runtime.data.RPromise.PromiseState;
 import com.oracle.truffle.r.runtime.nodes.RBaseNode;
 
@@ -116,7 +118,7 @@ public class PromiseHelperNode extends RBaseNode {
         private boolean deoptimize(RPromise promise) {
             if (!promise.getState().isDefaultOpt()) {
                 deoptimizeProfile.enter();
-                EagerPromise eager = (EagerPromise) promise;
+                EagerPromiseBase eager = (EagerPromiseBase) promise;
                 return eager.deoptimize();
             }
 
@@ -156,7 +158,7 @@ public class PromiseHelperNode extends RBaseNode {
         if (state.isDefaultOpt()) {
             obj = generateValueDefault(frame, state, promise);
         } else {
-            obj = generateValueEager(frame, state, (EagerPromise) promise);
+            obj = generateValueEager(frame, state, (EagerPromiseBase) promise);
         }
         if (isEvaluated(promise)) {
             // TODO: this only happens if compilation is in play and, as such, is difficult to track
@@ -174,22 +176,23 @@ public class PromiseHelperNode extends RBaseNode {
             throw RError.error(RError.SHOW_CALLER, RError.Message.PROMISE_CYCLE);
         }
         try {
-            // Evaluate guarded by underEvaluation
-            promise.setState(PromiseState.UnderEvaluation);
-
             if (isInOriginFrame(frame, promise)) {
+                // state change must happen inside of conditional as isInOriginalFrame checks the
+                // state
+                promise.setState(PromiseState.UnderEvaluation);
                 if (expressionInlineCache == null) {
                     CompilerDirectives.transferToInterpreterAndInvalidate();
-                    expressionInlineCache = insert(InlineCacheNode.createExpression(3));
+                    expressionInlineCache = insert(InlineCacheNode.createExpression(FastROptions.PromiseCacheSize.getNonNegativeIntValue()));
                 }
                 return expressionInlineCache.execute(frame, promise.getRep());
             } else {
+                promise.setState(PromiseState.UnderEvaluation);
                 Frame promiseFrame = promiseFrameProfile.profile(promise.getFrame());
                 assert promiseFrame != null;
 
                 if (promiseClosureCache == null) {
                     CompilerDirectives.transferToInterpreterAndInvalidate();
-                    promiseClosureCache = insert(InlineCacheNode.createPromise(3));
+                    promiseClosureCache = insert(InlineCacheNode.createPromise(FastROptions.PromiseCacheSize.getNonNegativeIntValue()));
                 }
                 promiseFrame = wrapPromiseFrame(frame, promiseFrame);
                 return promiseClosureCache.execute(promiseFrame, promise.getClosure());
@@ -199,7 +202,7 @@ public class PromiseHelperNode extends RBaseNode {
         }
     }
 
-    private Object generateValueEager(VirtualFrame frame, PromiseState state, EagerPromise promise) {
+    private Object generateValueEager(VirtualFrame frame, PromiseState state, EagerPromiseBase promise) {
         assert state.isEager() || state == PromiseState.Promised;
         if (!isDeoptimized(promise)) {
             Assumption eagerAssumption = isValidAssumptionProfile.profile(promise.getIsValidAssumption());
@@ -209,7 +212,7 @@ public class PromiseHelperNode extends RBaseNode {
                     return checkNextNode().evaluate(frame, nextPromise);
                 } else {
                     assert state.isEager();
-                    return getEagerValue(frame, promise);
+                    return getEagerValue(frame, (EagerPromise) promise);
                 }
             } else {
                 CompilerDirectives.transferToInterpreter();
@@ -235,7 +238,7 @@ public class PromiseHelperNode extends RBaseNode {
             // Evaluate guarded by underEvaluation
             obj = generateValueDefaultSlowPath(frame, state, promise);
         } else {
-            obj = generateValueEagerSlowPath(frame, state, (EagerPromise) promise);
+            obj = generateValueEagerSlowPath(frame, state, (EagerPromiseBase) promise);
         }
         promise.setValue(obj);
         return obj;
@@ -268,7 +271,7 @@ public class PromiseHelperNode extends RBaseNode {
                         RCaller.createForPromise(RArguments.getCall(promiseFrame), frame == null ? 0 : RArguments.getDepth(frame)));
     }
 
-    private static Object generateValueEagerSlowPath(VirtualFrame frame, PromiseState state, EagerPromise promise) {
+    private static Object generateValueEagerSlowPath(VirtualFrame frame, PromiseState state, EagerPromiseBase promise) {
         assert state.isEager() || state == PromiseState.Promised;
         if (!promise.isDeoptimized()) {
             Assumption eagerAssumption = promise.getIsValidAssumption();
@@ -301,7 +304,7 @@ public class PromiseHelperNode extends RBaseNode {
      */
     public void materialize(RPromise promise) {
         if (isOptEagerProfile.profile(promise.getState().isEager()) || isOptPromisedProfile.profile(promise.getState() == PromiseState.Promised)) {
-            EagerPromise eager = (EagerPromise) promise;
+            EagerPromiseBase eager = (EagerPromiseBase) promise;
             eager.materialize();
         }
         // otherwise: already the generic and slow RPromise
@@ -319,7 +322,7 @@ public class PromiseHelperNode extends RBaseNode {
         return isNullFrameProfile.profile(promise.isNullFrame());
     }
 
-    private boolean isDeoptimized(EagerPromise promise) {
+    private boolean isDeoptimized(EagerPromiseBase promise) {
         return isDeoptimizedProfile.profile(promise.isDeoptimized());
     }
 

com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RMissingHelper.java

@@ -31,7 +31,7 @@ import com.oracle.truffle.r.runtime.data.RArgsValuesAndNames;
 import com.oracle.truffle.r.runtime.data.REmpty;
 import com.oracle.truffle.r.runtime.data.RMissing;
 import com.oracle.truffle.r.runtime.data.RPromise;
-import com.oracle.truffle.r.runtime.data.RPromise.EagerPromise;
+import com.oracle.truffle.r.runtime.data.RPromise.EagerPromiseBase;
 import com.oracle.truffle.r.runtime.data.RPromise.PromiseState;
 import com.oracle.truffle.r.runtime.nodes.RNode;
 
@@ -158,8 +158,8 @@ public class RMissingHelper {
                 }
                 promise.setState(PromiseState.UnderEvaluation);
                 // TODO Profile necessary here???
-                if (promise instanceof EagerPromise) {
-                    EagerPromise eagerPromise = (EagerPromise) promise;
+                if (promise instanceof EagerPromiseBase) {
+                    EagerPromiseBase eagerPromise = (EagerPromiseBase) promise;
                     if (!eagerPromise.isDeoptimized()) {
                         Object eagerValue = eagerPromise.getEagerValue();
                         if (eagerValue instanceof RPromise) {

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/FastROptions.java

@@ -60,7 +60,8 @@ public enum FastROptions {
     EagerEvalConstants("Unconditionally evaluates constants before creating Promises", true),
     EagerEvalVariables("Enables optimistic eager evaluation of single variables reads", true),
     EagerEvalDefault("Enables optimistic eager evaluation of single variables reads (for default parameters)", false),
-    EagerEvalExpressions("Enables optimistic eager evaluation of trivial expressions", false);
+    EagerEvalExpressions("Enables optimistic eager evaluation of trivial expressions", false),
+    PromiseCacheSize("Enables inline caches for promises evaluation", "3", true);
 
     private final String help;
     private final boolean isBoolean;
@@ -101,6 +102,25 @@ public enum FastROptions {
         }
     }
 
+    public int getNonNegativeIntValue() {
+        assert !isBoolean;
+        if (value instanceof String) {
+            try {
+                int res = Integer.decode((String) value);
+                if (res >= 0) {
+                    return res;
+                } // else fall through to error message
+            } catch (NumberFormatException x) {
+                // fall through to error message
+            }
+        }
+
+        System.out.println("non negative integer option value expected with " + name());
+        System.exit(2);
+        return -1;
+
+    }
+
     private static FastROptions[] VALUES = values();
 
     static void setValue(String name, Object value) {
@@ -221,4 +241,9 @@ public enum FastROptions {
         }
         FastROptions.Debug.value = s;
     }
+
+    public static boolean noEagerEval() {
+        return !(EagerEval.getBooleanValue() || EagerEvalConstants.getBooleanValue() || EagerEvalVariables.getBooleanValue() || EagerEvalDefault.getBooleanValue() ||
+                        EagerEvalExpressions.getBooleanValue());
+    }
 }

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/FastPathFactory.java

@@ -24,6 +24,7 @@ package com.oracle.truffle.r.runtime.data;
 
 import java.util.function.Supplier;
 
+import com.oracle.truffle.r.runtime.FastROptions;
 import com.oracle.truffle.r.runtime.RBuiltin;
 import com.oracle.truffle.r.runtime.RVisibility;
 import com.oracle.truffle.r.runtime.nodes.RFastPathNode;
@@ -57,7 +58,7 @@ public interface FastPathFactory {
 
         @Override
         public boolean forcedEagerPromise(int index) {
-            return true;
+            return FastROptions.noEagerEval() ? false : true;
         }
     };
 

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java

@@ -33,7 +33,9 @@ import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.RootCallTarget;
 import com.oracle.truffle.api.frame.MaterializedFrame;
 import com.oracle.truffle.api.profiles.ConditionProfile;
+import com.oracle.truffle.r.runtime.FastROptions;
 import com.oracle.truffle.r.runtime.RCaller;
+import com.oracle.truffle.r.runtime.RInternalError;
 import com.oracle.truffle.r.runtime.RPerfStats;
 import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.data.RPromise.Closure;
@@ -388,9 +390,19 @@ public final class RDataFactory {
 
     public static RPromise createEagerPromise(PromiseState state, Closure exprClosure, Object eagerValue, Assumption notChangedNonLocally, RCaller targetFrame, EagerFeedback feedback,
                     int wrapIndex) {
+        if (FastROptions.noEagerEval()) {
+            throw RInternalError.shouldNotReachHere();
+        }
         return traceDataCreated(new RPromise.EagerPromise(state, exprClosure, eagerValue, notChangedNonLocally, targetFrame, feedback, wrapIndex));
     }
 
+    public static RPromise createPromisedPromise(Closure exprClosure, Object eagerValue, Assumption notChangedNonLocally, RCaller targetFrame, EagerFeedback feedback) {
+        if (FastROptions.noEagerEval()) {
+            throw RInternalError.shouldNotReachHere();
+        }
+        return traceDataCreated(new RPromise.PromisedPromise(exprClosure, eagerValue, notChangedNonLocally, targetFrame, feedback));
+    }
+
     public static RPairList createPairList() {
         return traceDataCreated(new RPairList());
     }

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RPromise.java

@@ -249,7 +249,7 @@ public class RPromise implements RTypedValue {
      * originally read from has not been altered in the mean time. If this cannot be guaranteed for
      * any reason, a Promise gets {@link #deoptimize()} (which includes {@link #materialize()}ion).
      */
-    public static final class EagerPromise extends RPromise {
+    public static class EagerPromiseBase extends RPromise {
         private final Object eagerValue;
 
         private final Assumption notChangedNonLocally;
@@ -263,7 +263,7 @@ public class RPromise implements RTypedValue {
          */
         private boolean deoptimized = false;
 
-        EagerPromise(PromiseState state, Closure closure, Object eagerValue, Assumption notChangedNonLocally, RCaller targetFrame, EagerFeedback feedback, int wrapIndex) {
+        EagerPromiseBase(PromiseState state, Closure closure, Object eagerValue, Assumption notChangedNonLocally, RCaller targetFrame, EagerFeedback feedback, int wrapIndex) {
             super(state, (MaterializedFrame) null, closure);
             assert state != PromiseState.Explicit;
             this.eagerValue = eagerValue;
@@ -320,6 +320,26 @@ public class RPromise implements RTypedValue {
         }
     }
 
+    /**
+     * This is a "proper" eager promise.
+     */
+    public static final class EagerPromise extends EagerPromiseBase {
+        EagerPromise(PromiseState state, Closure closure, Object eagerValue, Assumption notChangedNonLocally, RCaller targetFrame, EagerFeedback feedback, int wrapIndex) {
+            super(state, closure, eagerValue, notChangedNonLocally, targetFrame, feedback, wrapIndex);
+        }
+    }
+
+    /**
+     * It's a variant of an eager promise used to store another promise, distinguished mostly for
+     * accounting purposes.
+     */
+    public static final class PromisedPromise extends EagerPromiseBase {
+
+        PromisedPromise(Closure closure, Object eagerValue, Assumption notChangedNonLocally, RCaller targetFrame, EagerFeedback feedback) {
+            super(PromiseState.Promised, closure, eagerValue, notChangedNonLocally, targetFrame, feedback, -1);
+        }
+    }
+
     /**
      * Used to allow feedback on {@link EagerPromise} evaluation.
      */
@@ -377,7 +397,7 @@ public class RPromise implements RTypedValue {
 
         public RPromise createPromisedPromise(RPromise promisedPromise, Assumption notChangedNonLocally, RCaller targetFrame, EagerFeedback feedback) {
             assert state == PromiseState.Supplied;
-            return RDataFactory.createEagerPromise(PromiseState.Promised, exprClosure, promisedPromise, notChangedNonLocally, targetFrame, feedback, -1);
+            return RDataFactory.createPromisedPromise(exprClosure, promisedPromise, notChangedNonLocally, targetFrame, feedback);
         }
 
         public Object getExpr() {

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/nodes/EvaluatedArgumentsFastPath.java

@@ -25,6 +25,7 @@ package com.oracle.truffle.r.runtime.nodes;
 import java.util.Arrays;
 
 import com.oracle.truffle.r.runtime.ArgumentsSignature;
+import com.oracle.truffle.r.runtime.FastROptions;
 import com.oracle.truffle.r.runtime.data.FastPathFactory;
 
 final class EvaluatedArgumentsFastPath implements FastPathFactory {
@@ -47,7 +48,7 @@ final class EvaluatedArgumentsFastPath implements FastPathFactory {
 
     @Override
     public boolean forcedEagerPromise(int index) {
-        return forcedArguments[index];
+        return FastROptions.noEagerEval() ? false : forcedArguments[index];
     }
 
     public String toString(ArgumentsSignature signature) {

com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/tck/FastRDebugTest.java

@@ -46,7 +46,7 @@ import com.oracle.truffle.api.source.Source;
 import com.oracle.truffle.api.vm.EventConsumer;
 import com.oracle.truffle.api.vm.PolyglotEngine;
 import com.oracle.truffle.api.vm.PolyglotEngine.Value;
-import com.oracle.truffle.r.runtime.data.RPromise.EagerPromise;
+import com.oracle.truffle.r.runtime.data.RPromise.EagerPromiseBase;
 
 public class FastRDebugTest {
     private Debugger debugger;
@@ -282,8 +282,8 @@ public class FastRDebugTest {
 
     Object getRValue(Object value) {
         // This will only work in simple cases
-        if (value instanceof EagerPromise) {
-            return ((EagerPromise) value).getValue();
+        if (value instanceof EagerPromiseBase) {
+            return ((EagerPromiseBase) value).getValue();
         }
         return value;
     }