From cfb80adecc9b1c3dede1b80d7d87d73a71a0acb5 Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Thu, 22 Sep 2016 10:42:50 +0200
Subject: [PATCH] ListBuiltin and Quote: make share permanent only when cached.
Plus: assertions in RSharingAttributeStorage and removed unnecessary code from WriteIndexedVectorNode.
---
.../r/nodes/builtin/base/ListBuiltin.java | 19 +++++++++++++-----
.../truffle/r/nodes/builtin/base/Quote.java | 20 +++++++++++++------
.../access/vector/WriteIndexedVectorNode.java | 5 ++---
.../data/RSharingAttributeStorage.java | 3 ++-
4 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/ListBuiltin.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/ListBuiltin.java
index 8d1b5a741a..60c315de8d 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/ListBuiltin.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/ListBuiltin.java
@@ -53,7 +53,19 @@ public abstract class ListBuiltin extends RBuiltinNode {
@CompilationFinal private RStringVector suppliedSignatureArgNames;
- protected RStringVector argNameVector(ArgumentsSignature signature) {
+ /**
+ * Creates a shared permanent vector so that it can be re-used for every list(...) with the same
+ * arguments
+ */
+ protected final RStringVector cachedArgNameVector(ArgumentsSignature signature) {
+ RStringVector result = argNameVector(signature);
+ if (result != null) {
+ result.makeSharedPermanent();
+ }
+ return result;
+ }
+
+ protected final RStringVector argNameVector(ArgumentsSignature signature) {
if (namesNull.profile(signature.getNonNullCount() == 0)) {
return null;
}
@@ -63,9 +75,6 @@ public abstract class ListBuiltin extends RBuiltinNode {
names[i] = (orgName == null ? RRuntime.NAMES_ATTR_EMPTY_VALUE : orgName);
}
RStringVector result = RDataFactory.createStringVector(names, RDataFactory.COMPLETE_VECTOR);
- // this is going to be a cached vector re-used for every list(...) with the same arguments,
- // it has to be shared.
- result.makeSharedPermanent();
return result;
}
@@ -81,7 +90,7 @@ public abstract class ListBuiltin extends RBuiltinNode {
protected RList listCached(RArgsValuesAndNames args, //
@Cached("args.getLength()") int cachedLength, //
@SuppressWarnings("unused") @Cached("args.getSignature()") ArgumentsSignature cachedSignature, //
- @Cached("argNameVector(cachedSignature)") RStringVector cachedArgNames) {
+ @Cached("cachedArgNameVector(cachedSignature)") RStringVector cachedArgNames) {
Object[] argArray = args.getArguments();
for (int i = 0; i < cachedLength; i++) {
getShareObjectNode(i).execute(argArray[i]);
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Quote.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Quote.java
index eb82f27253..7fb33e8a49 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Quote.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Quote.java
@@ -44,19 +44,27 @@ public abstract class Quote extends RBuiltinNode {
public abstract Object execute(RPromise expr);
- protected final Object createLanguage(Closure closure) {
- Object o = RASTUtils.createLanguageElement(closure.getExpr().asRSyntaxNode());
- if (shareableProfile.profile(o instanceof RShareable)) {
- ((RShareable) o).makeSharedPermanent();
+ /**
+ * Creates a shared permanent language so that it can be cached and repeatedly returned as the
+ * result.
+ */
+ protected final Object cachedCreateLanguage(Closure closure) {
+ Object result = createLanguage(closure);
+ if (shareableProfile.profile(result instanceof RShareable)) {
+ ((RShareable) result).makeSharedPermanent();
}
- return o;
+ return result;
+ }
+
+ protected final Object createLanguage(Closure closure) {
+ return RASTUtils.createLanguageElement(closure.getExpr().asRSyntaxNode());
}
@SuppressWarnings("unused")
@Specialization(limit = "LIMIT", guards = "cachedClosure == expr.getClosure()")
protected Object quoteCached(RPromise expr,
@Cached("expr.getClosure()") Closure cachedClosure,
- @Cached("createLanguage(cachedClosure)") Object language) {
+ @Cached("cachedCreateLanguage(cachedClosure)") Object language) {
return language;
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/WriteIndexedVectorNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/WriteIndexedVectorNode.java
index a927fefac5..c485fd53a3 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/WriteIndexedVectorNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/WriteIndexedVectorNode.java
@@ -473,7 +473,6 @@ abstract class WriteIndexedVectorNode extends Node {
private final boolean setListElementAsObject;
private final boolean isReplace;
- private final ConditionProfile isShareable = ConditionProfile.createBinaryProfile();
@Child private UpdateStateOfListElement updateStateOfListElement;
@Child private ShareObjectNode shareObjectNode;
@@ -481,9 +480,9 @@ abstract class WriteIndexedVectorNode extends Node {
this.setListElementAsObject = setListElementAsObject;
this.isReplace = isReplace;
if (!isReplace) {
- updateStateOfListElement = insert(UpdateStateOfListElement.create());
+ updateStateOfListElement = UpdateStateOfListElement.create();
} else {
- shareObjectNode = insert(ShareObjectNode.create());
+ shareObjectNode = ShareObjectNode.create();
}
}
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSharingAttributeStorage.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSharingAttributeStorage.java
index 90032b7255..18920ff210 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSharingAttributeStorage.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSharingAttributeStorage.java
@@ -47,7 +47,8 @@ public abstract class RSharingAttributeStorage extends RAttributeStorage impleme
@Override
public final void decRefCount() {
- assert refCount > 0;
+ assert refCount != SHARED_PERMANENT_VAL : "cannot decRefCount of shared permanent value";
+ assert refCount > 0 : "cannot decRefCount when refCount <= 0";
refCount--;
}
--
GitLab