Merge pull request #602 in G/fastr from...

Merge pull request #602 in G/fastr from ~LUKAS.STADLER_ORACLE.COM/fastr:feature/sync_promise to master

* commit '9b55ddc9':
  refactor RPromise: change state to bitfield, synchronized evaluation of explicit promises, fast path for eager promises in MissingNode

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

@@ -36,6 +36,7 @@ import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
+import com.oracle.truffle.api.profiles.PrimitiveValueProfile;
 import com.oracle.truffle.api.profiles.ValueProfile;
 import com.oracle.truffle.r.nodes.InlineCacheNode;
 import com.oracle.truffle.r.nodes.function.opt.ShareObjectNode;
@@ -91,7 +92,7 @@ public class PromiseHelperNode extends RBaseNode {
         @TruffleBoundary
         public boolean deoptimizeFrame(MaterializedFrame frame) {
             boolean deoptOne = false;
-            for (FrameSlot slot : frame.getFrameDescriptor().getSlots()) {
+            for (FrameSlot slot : frame.getFrameDescriptor().getSlots().toArray(new FrameSlot[0])) {
                 // We're only interested in RPromises
                 if (slot.getKind() != FrameSlotKind.Object) {
                     continue;
@@ -117,7 +118,7 @@ public class PromiseHelperNode extends RBaseNode {
         }
 
         private boolean deoptimize(RPromise promise) {
-            if (!promise.getState().isDefaultOpt()) {
+            if (!PromiseState.isDefaultOpt(promise.getState())) {
                 deoptimizeProfile.enter();
                 EagerPromiseBase eager = (EagerPromiseBase) promise;
                 return eager.deoptimize();
@@ -135,7 +136,7 @@ public class PromiseHelperNode extends RBaseNode {
     @Children private final WrapArgumentNode[] wrapNodes = new WrapArgumentNode[ArgumentStatePush.MAX_COUNTED_ARGS];
     private final ConditionProfile shouldWrap = ConditionProfile.createBinaryProfile();
 
-    private final ValueProfile optStateProfile = ValueProfile.createIdentityProfile();
+    @CompilationFinal private PrimitiveValueProfile optStateProfile = PrimitiveValueProfile.createEqualityProfile();
     private final ValueProfile isValidAssumptionProfile = ValueProfile.createIdentityProfile();
     private final ValueProfile promiseFrameProfile = ValueProfile.createClassProfile();
 
@@ -155,9 +156,16 @@ public class PromiseHelperNode extends RBaseNode {
         }
 
         Object obj;
-        PromiseState state = optStateProfile.profile(promise.getState());
-        if (state.isDefaultOpt()) {
-            obj = generateValueDefault(frame, state, promise);
+        int state = optStateProfile.profile(promise.getState());
+        if (PromiseState.isExplicit(state)) {
+            CompilerDirectives.transferToInterpreter();
+            // reset profiles, this is very likely a one-time event
+            isEvaluatedProfile = ConditionProfile.createBinaryProfile();
+            optStateProfile = PrimitiveValueProfile.createEqualityProfile();
+            return evaluateSlowPath(frame, promise);
+        }
+        if (PromiseState.isDefaultOpt(state)) {
+            obj = generateValueDefault(frame, promise);
         } else {
             obj = generateValueEager(frame, state, (EagerPromiseBase) promise);
         }
@@ -171,7 +179,7 @@ public class PromiseHelperNode extends RBaseNode {
         }
     }
 
-    private Object generateValueDefault(VirtualFrame frame, PromiseState state, RPromise promise) {
+    private Object generateValueDefault(VirtualFrame frame, RPromise promise) {
         // Check for dependency cycle
         if (isUnderEvaluation(promise)) {
             throw RError.error(RError.SHOW_CALLER, RError.Message.PROMISE_CYCLE);
@@ -181,32 +189,29 @@ public class PromiseHelperNode extends RBaseNode {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
                 promiseClosureCache = insert(InlineCacheNode.createPromise(FastROptions.PromiseCacheSize.getNonNegativeIntValue()));
             }
+            promise.setUnderEvaluation();
             if (isInOriginFrame(frame, promise)) {
-                // state change must happen inside of conditional as isInOriginalFrame checks the
-                // state
-                promise.setState(PromiseState.UnderEvaluation);
                 return promiseClosureCache.execute(frame, promise.getClosure());
             } else {
-                promise.setState(PromiseState.UnderEvaluation);
                 Frame promiseFrame = promiseFrameProfile.profile(promise.getFrame());
                 assert promiseFrame != null;
                 return promiseClosureCache.execute(wrapPromiseFrame(frame, promiseFrame), promise.getClosure());
             }
         } finally {
-            promise.setState(state);
+            promise.resetUnderEvaluation();
         }
     }
 
-    private Object generateValueEager(VirtualFrame frame, PromiseState state, EagerPromiseBase promise) {
-        assert state.isEager() || state == PromiseState.Promised;
+    private Object generateValueEager(VirtualFrame frame, int state, EagerPromiseBase promise) {
+        assert !PromiseState.isDefaultOpt(state);
         if (!isDeoptimized(promise)) {
             Assumption eagerAssumption = isValidAssumptionProfile.profile(promise.getIsValidAssumption());
             if (eagerAssumption.isValid()) {
-                if (state == PromiseState.Promised) {
+                if (!PromiseState.isEager(state)) {
                     RPromise nextPromise = (RPromise) promise.getEagerValue();
                     return checkNextNode().evaluate(frame, nextPromise);
                 } else {
-                    assert state.isEager();
+                    assert PromiseState.isEager(state);
                     return getEagerValue(frame, (EagerPromise) promise);
                 }
             } else {
@@ -218,7 +223,7 @@ public class PromiseHelperNode extends RBaseNode {
             }
         }
         // Call
-        return generateValueDefault(frame, state, promise);
+        return generateValueDefault(frame, promise);
     }
 
     public static Object evaluateSlowPath(VirtualFrame frame, RPromise promise) {
@@ -227,11 +232,21 @@ public class PromiseHelperNode extends RBaseNode {
             return promise.getValue();
         }
 
+        int state = promise.getState();
+        if (PromiseState.isExplicit(state)) {
+            synchronized (promise) {
+                if (promise.isEvaluated()) {
+                    return promise.getValue();
+                }
+                Object obj = generateValueDefaultSlowPath(frame, promise);
+                promise.setValue(obj);
+                return obj;
+            }
+        }
         Object obj;
-        PromiseState state = promise.getState();
-        if (state.isDefaultOpt()) {
+        if (PromiseState.isDefaultOpt(state)) {
             // Evaluate guarded by underEvaluation
-            obj = generateValueDefaultSlowPath(frame, state, promise);
+            obj = generateValueDefaultSlowPath(frame, promise);
         } else {
             obj = generateValueEagerSlowPath(frame, state, (EagerPromiseBase) promise);
         }
@@ -239,13 +254,13 @@ public class PromiseHelperNode extends RBaseNode {
         return obj;
     }
 
-    private static Object generateValueDefaultSlowPath(VirtualFrame frame, PromiseState state, RPromise promise) {
+    private static Object generateValueDefaultSlowPath(VirtualFrame frame, RPromise promise) {
         // Check for dependency cycle
         if (promise.isUnderEvaluation()) {
             throw RError.error(RError.SHOW_CALLER, RError.Message.PROMISE_CYCLE);
         }
         try {
-            promise.setState(PromiseState.UnderEvaluation);
+            promise.setUnderEvaluation();
 
             if (promise.isInOriginFrame(frame)) {
                 return promise.getClosure().eval(frame.materialize());
@@ -257,7 +272,7 @@ public class PromiseHelperNode extends RBaseNode {
                 return promise.getClosure().eval(promiseFrame.materialize());
             }
         } finally {
-            promise.setState(state);
+            promise.resetUnderEvaluation();
         }
     }
 
@@ -266,16 +281,15 @@ public class PromiseHelperNode extends RBaseNode {
                         RCaller.createForPromise(RArguments.getCall(promiseFrame), frame));
     }
 
-    private static Object generateValueEagerSlowPath(VirtualFrame frame, PromiseState state, EagerPromiseBase promise) {
-        assert state.isEager() || state == PromiseState.Promised;
+    private static Object generateValueEagerSlowPath(VirtualFrame frame, int state, EagerPromiseBase promise) {
+        assert !PromiseState.isDefaultOpt(state);
         if (!promise.isDeoptimized()) {
             Assumption eagerAssumption = promise.getIsValidAssumption();
             if (eagerAssumption.isValid()) {
-                if (state == PromiseState.Promised) {
+                if (!PromiseState.isEager(state)) {
                     RPromise nextPromise = (RPromise) promise.getEagerValue();
                     return evaluateSlowPath(frame, nextPromise);
                 } else {
-                    assert state.isEager();
                     Object o = promise.getEagerValue();
                     if (promise.wrapIndex() != ArgumentStatePush.INVALID_INDEX) {
                         return ShareObjectNode.share(o);
@@ -290,7 +304,7 @@ public class PromiseHelperNode extends RBaseNode {
             }
         }
         // Call
-        return generateValueDefaultSlowPath(frame, state, promise);
+        return generateValueDefaultSlowPath(frame, promise);
     }
 
     /**
@@ -298,7 +312,7 @@ public class PromiseHelperNode extends RBaseNode {
      * <code>null</code>
      */
     public void materialize(RPromise promise) {
-        if (isOptEagerProfile.profile(promise.getState().isEager()) || isOptPromisedProfile.profile(promise.getState() == PromiseState.Promised)) {
+        if (isDefaultOptProfile.profile(!PromiseState.isDefaultOpt(promise.getState()))) {
             EagerPromiseBase eager = (EagerPromiseBase) promise;
             eager.materialize();
         }
@@ -332,7 +346,7 @@ public class PromiseHelperNode extends RBaseNode {
         return isEvaluatedProfile.profile(promise.isEvaluated());
     }
 
-    private final ConditionProfile isEvaluatedProfile = ConditionProfile.createBinaryProfile();
+    @CompilationFinal private ConditionProfile isEvaluatedProfile = ConditionProfile.createBinaryProfile();
     private final ConditionProfile underEvaluationProfile = ConditionProfile.createBinaryProfile();
     private final ConditionProfile isNullFrameProfile = ConditionProfile.createBinaryProfile();
 
@@ -342,8 +356,7 @@ public class PromiseHelperNode extends RBaseNode {
     private final ValueProfile valueProfile = ValueProfile.createClassProfile();
 
     // Eager
-    private final ConditionProfile isOptEagerProfile = ConditionProfile.createBinaryProfile();
-    private final ConditionProfile isOptPromisedProfile = ConditionProfile.createBinaryProfile();
+    private final ConditionProfile isDefaultOptProfile = ConditionProfile.createBinaryProfile();
     private final ConditionProfile isDeoptimizedProfile = ConditionProfile.createBinaryProfile();
     private final ValueProfile eagerValueProfile = ValueProfile.createClassProfile();
 

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

@@ -32,7 +32,6 @@ 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.EagerPromiseBase;
-import com.oracle.truffle.r.runtime.data.RPromise.PromiseState;
 
 /**
  * This class implements the behavior for {@link RMissing} which is needed inside this module, as it
@@ -130,12 +129,11 @@ public class RMissingHelper {
                 return true;
             }
 
-            PromiseState state = promise.getState();
             try {
                 if (promise.isEvaluated()) {
                     return false;
                 }
-                promise.setState(PromiseState.UnderEvaluation);
+                promise.setUnderEvaluation();
                 // TODO Profile necessary here???
                 if (promise instanceof EagerPromiseBase) {
                     EagerPromiseBase eagerPromise = (EagerPromiseBase) promise;
@@ -151,7 +149,7 @@ public class RMissingHelper {
                 // promise.materialize(globalMissingPromiseProfile);
                 result = isMissingArgument(promise.getFrame(), rvn.getIdentifier());
             } finally {
-                promise.setState(state);
+                promise.resetUnderEvaluation();
             }
         }
         return result;

com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/signature/MissingNode.java

@@ -46,7 +46,7 @@ import com.oracle.truffle.r.runtime.RInternalError;
 import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.data.RArgsValuesAndNames;
 import com.oracle.truffle.r.runtime.data.RPromise;
-import com.oracle.truffle.r.runtime.data.RPromise.PromiseState;
+import com.oracle.truffle.r.runtime.data.RPromise.EagerPromise;
 import com.oracle.truffle.r.runtime.nodes.RSyntaxElement;
 import com.oracle.truffle.r.runtime.nodes.RSyntaxLookup;
 
@@ -142,6 +142,9 @@ public final class MissingNode extends OperatorNode {
                 if (isSymbolNullProfile.profile(symbol == null)) {
                     return false;
                 } else {
+                    if (promise instanceof EagerPromise && !((EagerPromise) promise).isDeoptimized()) {
+                        return false;
+                    }
                     if (recursiveDesc != null) {
                         promiseHelper.materialize(promise); // Ensure that promise holds a frame
                     }
@@ -155,16 +158,15 @@ public final class MissingNode extends OperatorNode {
                         if (recursiveDesc == null) {
                             promiseHelper.materialize(promise); // Ensure that promise holds a frame
                         }
-                        PromiseState state = promise.getState();
                         try {
-                            promise.setState(PromiseState.UnderEvaluation);
+                            promise.setUnderEvaluation();
                             if (recursive == null) {
                                 CompilerDirectives.transferToInterpreterAndInvalidate();
                                 recursive = insert(MissingCheckCache.create(level + 1));
                             }
                             return recursive.execute(promise.getFrame(), symbol);
                         } finally {
-                            promise.setState(state);
+                            promise.resetUnderEvaluation();
                         }
                     }
                 }

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

@@ -49,6 +49,13 @@ import com.oracle.truffle.r.runtime.nodes.RSyntaxLookup;
 @ValueType
 public class RPromise implements RTypedValue {
 
+    private static final int DEFAULT_BIT = 0x1;
+    private static final int FULL_PROMISE_BIT = 0x2;
+    private static final int EAGER_BIT = 0x4;
+    private static final int EXPLICIT_BIT = 0x8;
+    private static final int UNDER_EVALUATION_BIT = 0x10;
+    private static final int UNDER_EVALUATION_MASK = 0x0f;
+
     /**
      * This enum encodes the source, optimization and current state of a promise.
      */
@@ -57,45 +64,55 @@ public class RPromise implements RTypedValue {
          * This promise is created for an argument that has been supplied to the function call and
          * thus has to be evaluated inside the caller frame.
          */
-        Supplied,
+        Supplied(FULL_PROMISE_BIT),
         /**
          * This promise is created for an argument that was 'missing' at the function call and thus
          * contains it's default value and has to be evaluated inside the _callee_ frame.
          */
-        Default,
+        Default(DEFAULT_BIT | FULL_PROMISE_BIT),
         /**
          * A supplied promise that was optimized to eagerly evaluate its value.
          */
-        EagerSupplied,
+        EagerSupplied(EAGER_BIT),
         /**
          * A default promise that was optimized to eagerly evaluate its value.
          */
-        EagerDefault,
+        EagerDefault(EAGER_BIT | DEFAULT_BIT),
         /**
          * This promise was created to wrap around a parameter value that is a promise itself.
          */
-        Promised,
+        Promised(0),
         /**
          * This promise is not a function argument at all. (Created by 'delayedAssign', for
          * example).
          */
-        Explicit,
+        Explicit(EXPLICIT_BIT | FULL_PROMISE_BIT),
         /**
          * This promise is currently being evaluated. This necessary to avoid cyclic evaluation, and
          * can by checked via {@link #isUnderEvaluation()}.
          */
-        UnderEvaluation;
+        UnderEvaluation(UNDER_EVALUATION_BIT);
+
+        private final int bits;
+
+        PromiseState(int bits) {
+            this.bits = bits;
+        }
+
+        public static boolean isDefaultOpt(int state) {
+            return (state & FULL_PROMISE_BIT) != 0;
+        }
 
-        public boolean isDefaultOpt() {
-            return this == PromiseState.Default || this == PromiseState.Supplied || this == PromiseState.Explicit || this == PromiseState.UnderEvaluation;
+        public static boolean isEager(int state) {
+            return (state & EAGER_BIT) != 0;
         }
 
-        public boolean isEager() {
-            return this == PromiseState.EagerDefault || this == PromiseState.EagerSupplied;
+        public static boolean isExplicit(int state) {
+            return (state & EXPLICIT_BIT) != 0;
         }
     }
 
-    private PromiseState state;
+    private int state;
 
     /**
      * @see #getFrame()
@@ -104,7 +121,7 @@ public class RPromise implements RTypedValue {
     @CompilationFinal protected MaterializedFrame execFrame;
 
     /**
-     * Might not be <code>null</code>.
+     * May not be <code>null</code>.
      */
     private final Closure closure;
 
@@ -117,7 +134,7 @@ public class RPromise implements RTypedValue {
      * This creates a new tuple (expr, env, closure, value=null), which may later be evaluated.
      */
     RPromise(PromiseState state, MaterializedFrame execFrame, Closure closure) {
-        this.state = state;
+        this.state = state.bits;
         this.execFrame = execFrame;
         this.closure = closure;
     }
@@ -127,7 +144,7 @@ public class RPromise implements RTypedValue {
      */
     RPromise(PromiseState state, Closure closure, Object value) {
         assert value != null;
-        this.state = state;
+        this.state = state.bits;
         this.closure = closure;
         this.value = value;
         // Not needed as already evaluated:
@@ -139,11 +156,11 @@ public class RPromise implements RTypedValue {
         return RType.Promise;
     }
 
-    public final PromiseState getState() {
+    public final int getState() {
         return state;
     }
 
-    public final void setState(PromiseState state) {
+    public final void setState(int state) {
         assert !isEvaluated();
         this.state = state;
     }
@@ -160,7 +177,7 @@ public class RPromise implements RTypedValue {
     }
 
     public final boolean isDefaultArgument() {
-        return state == PromiseState.Default || state == PromiseState.EagerDefault;
+        return (state & DEFAULT_BIT) != 0;
     }
 
     public final boolean isNullFrame() {
@@ -234,7 +251,7 @@ public class RPromise implements RTypedValue {
      */
     public final boolean isUnderEvaluation() {
         assert !isEvaluated();
-        return state == PromiseState.UnderEvaluation;
+        return (state & UNDER_EVALUATION_BIT) != 0;
     }
 
     @Override
@@ -464,7 +481,7 @@ public class RPromise implements RTypedValue {
         }
 
         private static RootCallTarget generateCallTarget(RNode expr) {
-            return RContext.getEngine().makePromiseCallTarget(expr, CLOSURE_WRAPPER_NAME);
+            return RContext.getEngine().makePromiseCallTarget(expr, CLOSURE_WRAPPER_NAME + System.identityHashCode(expr));
         }
 
         public RBaseNode getExpr() {
@@ -490,4 +507,14 @@ public class RPromise implements RTypedValue {
     public boolean isS4() {
         return false;
     }
+
+    public void setUnderEvaluation() {
+        assert (state & UNDER_EVALUATION_BIT) == 0;
+        state |= UNDER_EVALUATION_BIT;
+    }
+
+    public void resetUnderEvaluation() {
+        assert (state & UNDER_EVALUATION_BIT) != 0;
+        state &= UNDER_EVALUATION_MASK;
+    }
 }

com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_delayedAssign.java

@@ -31,7 +31,7 @@ public class TestBuiltin_delayedAssign extends TestBase {
         assertEval(Output.IgnoreErrorContext, "{ f <- function() { delayedAssign(\"x\",y); delayedAssign(\"y\",x); g(x, y)}; g <- function(x, y) { x + y }; f() }");
         assertEval("{ f <- function() { delayedAssign(\"x\",y); delayedAssign(\"y\",x); list(x, y)}; f() }");
         assertEval(Output.IgnoreErrorContext, "{ f <- function() { delayedAssign(\"x\",y); delayedAssign(\"y\",x); paste(x, y)}; f() }");
-        assertEval("{ f <- function() { delayedAssign(\"x\",y); delayedAssign(\"y\",x); print(x, y)}; f() }");
+        assertEval(Output.IgnoreErrorContext, "{ f <- function() { delayedAssign(\"x\",y); delayedAssign(\"y\",x); print(x, y)}; f() }");
         assertEval("{ f <- function() { p <- 0; for (i in 1:10) { if (i %% 2 == 0) { delayedAssign(\"a\", p + 1); } else { a <- p + 1; }; p <- a; }; p }; f() }");
         assertEval("{ f <- function() { x <- 4 ; delayedAssign(\"x\", y); y <- 10; x  } ; f() }");
         assertEval("{ h <- new.env(parent=emptyenv()) ; delayedAssign(\"x\", y, h, h) ; assign(\"y\", 2, h) ; get(\"x\", h) }");