Skip to content
Snippets Groups Projects
Commit 906c4d5f authored by stepan's avatar stepan
Browse files

Fix: temporary shareable should not be 'value' of explicit RPromise

parent a3af7d8f
No related branches found
No related tags found
No related merge requests found
...@@ -50,6 +50,7 @@ import com.oracle.truffle.r.runtime.data.RPromise; ...@@ -50,6 +50,7 @@ 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.EagerPromise;
import com.oracle.truffle.r.runtime.data.RPromise.EagerPromiseBase; import com.oracle.truffle.r.runtime.data.RPromise.EagerPromiseBase;
import com.oracle.truffle.r.runtime.data.RPromise.PromiseState; import com.oracle.truffle.r.runtime.data.RPromise.PromiseState;
import com.oracle.truffle.r.runtime.data.RShareable;
import com.oracle.truffle.r.runtime.nodes.RBaseNode; import com.oracle.truffle.r.runtime.nodes.RBaseNode;
/** /**
...@@ -237,6 +238,16 @@ public class PromiseHelperNode extends RBaseNode { ...@@ -237,6 +238,16 @@ public class PromiseHelperNode extends RBaseNode {
return promise.getValue(); return promise.getValue();
} }
Object obj = generateValueDefaultSlowPath(frame, promise); Object obj = generateValueDefaultSlowPath(frame, promise);
// if the value is temporary, we increment the reference count. The reason is that
// temporary values are considered available to be reused and altered (e.g. as a
// result of arithmetic operation), which is what we do not want to happen to a
// value that we are saving as the promise result.
if (obj instanceof RShareable) {
RShareable shareable = (RShareable) obj;
if (shareable.isTemporary()) {
shareable.incRefCount();
}
}
promise.setValue(obj); promise.setValue(obj);
return obj; return obj;
} }
......
...@@ -77177,6 +77177,10 @@ Error in get("x") : object 'x' not found ...@@ -77177,6 +77177,10 @@ Error in get("x") : object 'x' not found
#{ x <- function(){3} ; gg <- function() { assign("x", 4) ; g <- function() { if (FALSE) { x <- 2 } ; f <- function() { h <- function() { x() } ; h() } ; f() } ; g() } ; gg() } #{ x <- function(){3} ; gg <- function() { assign("x", 4) ; g <- function() { if (FALSE) { x <- 2 } ; f <- function() { h <- function() { x() } ; h() } ; f() } ; g() } ; gg() }
[1] 3 [1] 3
   
##com.oracle.truffle.r.test.library.base.TestPromiseOptimizations.testDeoptimization#
#{ delayedAssign('x', c(1,2,3)); x/180; x }
[1] 1 2 3
##com.oracle.truffle.r.test.library.base.TestPromiseOptimizations.testDeoptimization# ##com.oracle.truffle.r.test.library.base.TestPromiseOptimizations.testDeoptimization#
#{ f <- function(x) { assign('e', function() {x}, parent.frame()) } ; a <- 1 ; f( a ) ; a <- 10 ; e() } #{ f <- function(x) { assign('e', function() {x}, parent.frame()) } ; a <- 1 ; f( a ) ; a <- 10 ; e() }
[1] 10 [1] 10
...@@ -77217,6 +77221,10 @@ Error in get("x") : object 'x' not found ...@@ -77217,6 +77221,10 @@ Error in get("x") : object 'x' not found
#{ f <- function(x) { sys.frame(sys.nframe()) } ; a <- 1 ; e <- f( a ) ; a <- 10 ; e$x } #{ f <- function(x) { sys.frame(sys.nframe()) } ; a <- 1 ; e <- f( a ) ; a <- 10 ; e$x }
[1] 10 [1] 10
   
##com.oracle.truffle.r.test.library.base.TestPromiseOptimizations.testDeoptimization#
#{ pi/180; pi }
[1] 3.141593
##com.oracle.truffle.r.test.library.base.TestSimpleArithmetic.testArithmeticUpdate# ##com.oracle.truffle.r.test.library.base.TestSimpleArithmetic.testArithmeticUpdate#
#{ x <- 3 ; f <- function(z) { if (z) { x <- 1 } ; x <- 1L + x ; x } ; f(FALSE) } #{ x <- 3 ; f <- function(z) { if (z) { x <- 1 } ; x <- 1L + x ; x } ; f(FALSE) }
[1] 4 [1] 4
/* /*
* Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
...@@ -61,5 +61,11 @@ public class TestPromiseOptimizations extends TestBase { ...@@ -61,5 +61,11 @@ public class TestPromiseOptimizations extends TestBase {
// "delayedAssign" RFunction: (Special case of but handled by: delayedAssign) // "delayedAssign" RFunction: (Special case of but handled by: delayedAssign)
assertEval("{ f <- function(x) { delayedAssign('b', function() {x}, sys.frame(sys.nframe()), parent.frame()); } ; a <- 1 ; f( a ) ; a <- 10 ; b() }"); assertEval("{ f <- function(x) { delayedAssign('b', function() {x}, sys.frame(sys.nframe()), parent.frame()); } ; a <- 1 ; f( a ) ; a <- 10 ; b() }");
// the value of 'pi' promise may look like temporary vector, but the arithmetic operation
// must not re-use it for the result
assertEval("{ pi/180; pi }");
// similar situation as above
assertEval("{ delayedAssign('x', c(1,2,3)); x/180; x }");
} }
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment