From c2b4ca874cae1ed97198bbfca9023305f63cd37b Mon Sep 17 00:00:00 2001
From: Zbynek Slajchrt <zbynek.slajchrt@oracle.com>
Date: Thu, 15 Dec 2016 18:34:29 +0100
Subject: [PATCH] A fix-up of refence counting when setting an attribute
---
.../truffle/r/library/stats/RMultinom.java | 4 +-
.../truffle/r/nodes/builtin/base/Attr.java | 4 +-
.../access/vector/ExtractListElement.java | 74 ++-----------------
.../access/vector/WriteIndexedVectorNode.java | 6 +-
.../r/nodes/attributes/SetAttributeNode.java | 6 +-
.../attributes/SetFixedAttributeNode.java | 6 +-
.../SpecialAttributesFunctions.java | 43 +++++++----
...ava => UpdateShareableChildValueNode.java} | 33 +++++++--
.../test/builtins/TestBuiltin_attrassign.java | 6 ++
.../r/test/builtins/TestBuiltin_list.java | 5 ++
10 files changed, 89 insertions(+), 98 deletions(-)
rename com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/{UpdateSharedAttributeNode.java => UpdateShareableChildValueNode.java} (67%)
diff --git a/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/stats/RMultinom.java b/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/stats/RMultinom.java
index 7a49a79e36..101343b125 100644
--- a/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/stats/RMultinom.java
+++ b/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/stats/RMultinom.java
@@ -29,7 +29,7 @@ import com.oracle.truffle.api.profiles.ValueProfile;
import com.oracle.truffle.r.library.stats.RandGenerationFunctions.RandomNumberProvider;
import com.oracle.truffle.r.nodes.attributes.GetFixedAttributeNode;
import com.oracle.truffle.r.nodes.attributes.SetFixedAttributeNode;
-import com.oracle.truffle.r.nodes.attributes.UpdateSharedAttributeNode;
+import com.oracle.truffle.r.nodes.attributes.UpdateShareableChildValueNode;
import com.oracle.truffle.r.nodes.builtin.CastBuilder;
import com.oracle.truffle.r.nodes.builtin.RExternalBuiltinNode;
import com.oracle.truffle.r.nodes.function.opt.ReuseNonSharedNode;
@@ -58,7 +58,7 @@ public abstract class RMultinom extends RExternalBuiltinNode.Arg3 {
@Cached("create()") ReuseNonSharedNode reuseNonSharedNode,
@Cached("createClassProfile()") ValueProfile randGeneratorClassProfile,
@Cached("createBinaryProfile()") ConditionProfile hasAttributesProfile,
- @Cached("create()") UpdateSharedAttributeNode updateSharedAttributeNode,
+ @Cached("create()") UpdateShareableChildValueNode updateSharedAttributeNode,
@Cached("createNames()") GetFixedAttributeNode getNamesNode,
@Cached("createDimNames()") SetFixedAttributeNode setDimNamesNode) {
RAbstractDoubleVector nonSharedProbs = (RAbstractDoubleVector) reuseNonSharedNode.execute(probsVec);
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Attr.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Attr.java
index 10853a996c..4cce1df543 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Attr.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Attr.java
@@ -40,7 +40,7 @@ import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.r.nodes.attributes.GetAttributeNode;
import com.oracle.truffle.r.nodes.attributes.IterableAttributeNode;
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.GetRowNamesAttributeNode;
-import com.oracle.truffle.r.nodes.attributes.UpdateSharedAttributeNode;
+import com.oracle.truffle.r.nodes.attributes.UpdateShareableChildValueNode;
import com.oracle.truffle.r.nodes.builtin.CastBuilder;
import com.oracle.truffle.r.nodes.builtin.RBuiltinNode;
import com.oracle.truffle.r.runtime.RError;
@@ -67,7 +67,7 @@ public abstract class Attr extends RBuiltinNode {
@CompilationFinal private String cachedName = "";
@CompilationFinal private String cachedInternedName = "";
- @Child private UpdateSharedAttributeNode sharedAttrUpdate = UpdateSharedAttributeNode.create();
+ @Child private UpdateShareableChildValueNode sharedAttrUpdate = UpdateShareableChildValueNode.create();
@Child private GetAttributeNode attrAccess = GetAttributeNode.create();
@Child private IterableAttributeNode iterAttrAccess = IterableAttributeNode.create();
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/ExtractListElement.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/ExtractListElement.java
index 7d5a092165..b1bc27f5d8 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/ExtractListElement.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/ExtractListElement.java
@@ -26,19 +26,16 @@ import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.dsl.TypeSystemReference;
import com.oracle.truffle.api.nodes.Node;
-import com.oracle.truffle.api.profiles.ConditionProfile;
-import com.oracle.truffle.api.profiles.ValueProfile;
import com.oracle.truffle.r.nodes.EmptyTypeSystemFlatLayout;
-import com.oracle.truffle.r.nodes.access.vector.ExtractListElementNodeGen.UpdateStateOfListElementNodeGen;
+import com.oracle.truffle.r.nodes.attributes.UpdateShareableChildValueNode;
import com.oracle.truffle.r.runtime.data.RListBase;
-import com.oracle.truffle.r.runtime.data.RShareable;
import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
import com.oracle.truffle.r.runtime.data.model.RAbstractListVector;
-import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
/**
* Internal node that extracts data under given index from any RAbstractContainer. In the case of
- * RListBase, it also invokes {@link UpdateStateOfListElement} on the element before returning it.
+ * RListBase, it also invokes {@link UpdateShareableChildValueNode} on the element before returning
+ * it.
*
* There are two reasons for why one accesses an element of a list: to peek at it, possibly
* calculate some values from it, and then forget it. In such case, it is OK to access the element
@@ -46,9 +43,9 @@ import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
* {@link RAbstractContainer#getDataAtAsObject(int)}. However, if the object is going to be returned
* to the user either as return value of a built-in, put inside a list, put as an attribute, or its
* true reference count matters for some other reason, then its reference count must be put into a
- * consistent state, which is done by {@link UpdateStateOfListElement}. This node is a convenient
- * wrapper that performs the extraction as well as invocation of {@link UpdateStateOfListElement}.
- * See also the documentation of {@link RListBase}.
+ * consistent state, which is done by {@link UpdateShareableChildValueNode}. This node is a
+ * convenient wrapper that performs the extraction as well as invocation of
+ * {@link UpdateShareableChildValueNode}. See also the documentation of {@link RListBase}.
*/
@TypeSystemReference(EmptyTypeSystemFlatLayout.class)
public abstract class ExtractListElement extends Node {
@@ -60,7 +57,7 @@ public abstract class ExtractListElement extends Node {
}
@Specialization
- protected Object doList(RListBase list, int index, @Cached("create()") UpdateStateOfListElement updateStateNode) {
+ protected Object doList(RListBase list, int index, @Cached("create()") UpdateShareableChildValueNode updateStateNode) {
Object element = list.getDataAt(index);
return updateStateNode.updateState(list, element);
}
@@ -73,61 +70,4 @@ public abstract class ExtractListElement extends Node {
protected static boolean isNotList(RAbstractContainer x) {
return !(x instanceof RAbstractListVector);
}
-
- @TypeSystemReference(EmptyTypeSystemFlatLayout.class)
- public abstract static class UpdateStateOfListElement extends Node {
-
- public abstract void execute(Object owner, Object item);
-
- /**
- * Provides more convenient interface for the {@link #execute(Object, Object)} method.
- */
- public final <T> T updateState(RAbstractContainer owner, T item) {
- execute(owner, item);
- return item;
- }
-
- public static UpdateStateOfListElement create() {
- return UpdateStateOfListElementNodeGen.create();
- }
-
- @Specialization
- protected void doShareableValues(RListBase owner, RShareable value,
- @Cached("createClassProfile()") ValueProfile valueProfile,
- @Cached("createBinaryProfile()") ConditionProfile sharedValue,
- @Cached("createBinaryProfile()") ConditionProfile temporaryOwner) {
- RShareable profiledValue = valueProfile.profile(value);
- if (sharedValue.profile(profiledValue.isShared())) {
- // it is already shared, not need to do anything
- return;
- }
-
- if (temporaryOwner.profile(owner.isTemporary())) {
- // This can happen, for example, when we immediately extract out of a temporary
- // list that was returned by a built-in, like: strsplit(...)[[1L]]. We do not need
- // to transition the element, it may stay temporary.
- return;
- }
-
- if (profiledValue.isTemporary()) {
- // make it at least non-shared (parent list must be also at least non-shared)
- profiledValue.incRefCount();
- }
- if (owner.isShared()) {
- // owner is shared, make the value shared too
- profiledValue.incRefCount();
- }
- }
-
- @Specialization(guards = "isFallback(owner, value)")
- protected void doFallback(Object owner, Object value) {
- assert !(value instanceof RShareable && owner instanceof RAbstractVector && !(owner instanceof RListBase)) : "RShareables can only live inside lists and no other vectors.";
- // nop: either value is not RShareable, or the owner is "list" like structure with
- // reference semantics (e.g. REnvironment)
- }
-
- protected static boolean isFallback(Object owner, Object value) {
- return !(value instanceof RShareable) || !(owner instanceof RListBase);
- }
- }
}
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 b96380cab8..35975529b8 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
@@ -32,7 +32,7 @@ import com.oracle.truffle.api.profiles.BranchProfile;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.profiles.LoopConditionProfile;
import com.oracle.truffle.api.profiles.ValueProfile;
-import com.oracle.truffle.r.nodes.access.vector.ExtractListElement.UpdateStateOfListElement;
+import com.oracle.truffle.r.nodes.attributes.UpdateShareableChildValueNode;
import com.oracle.truffle.r.nodes.function.opt.ShareObjectNode;
import com.oracle.truffle.r.nodes.profile.AlwaysOnBranchProfile;
import com.oracle.truffle.r.nodes.profile.IntValueProfile;
@@ -473,14 +473,14 @@ abstract class WriteIndexedVectorNode extends Node {
private final boolean setListElementAsObject;
private final boolean isReplace;
- @Child private UpdateStateOfListElement updateStateOfListElement;
+ @Child private UpdateShareableChildValueNode updateStateOfListElement;
@Child private ShareObjectNode shareObjectNode;
WriteListAction(boolean setListElementAsObject, boolean isReplace) {
this.setListElementAsObject = setListElementAsObject;
this.isReplace = isReplace;
if (!isReplace) {
- updateStateOfListElement = UpdateStateOfListElement.create();
+ updateStateOfListElement = UpdateShareableChildValueNode.create();
} else {
shareObjectNode = ShareObjectNode.create();
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetAttributeNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetAttributeNode.java
index aecd9e3c0f..b08100173b 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetAttributeNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetAttributeNode.java
@@ -34,6 +34,7 @@ import com.oracle.truffle.api.object.Shape;
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.function.opt.ShareObjectNode;
import com.oracle.truffle.r.runtime.RInternalError;
import com.oracle.truffle.r.runtime.data.RAttributable;
import com.oracle.truffle.r.runtime.data.RAttributeStorage;
@@ -148,7 +149,8 @@ public abstract class SetAttributeNode extends AttributeAccessNode {
protected void setAttrInAttributable(RAttributable x, String name, Object value,
@Cached("create()") BranchProfile attrNullProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
DynamicObject attributes;
if (attrStorageProfile.profile(x instanceof RAttributeStorage)) {
attributes = ((RAttributeStorage) x).getAttributes();
@@ -167,6 +169,8 @@ public abstract class SetAttributeNode extends AttributeAccessNode {
}
recursive.execute(attributes, name, value);
+
+ updateRefCountNode.execute(value);
}
/**
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetFixedAttributeNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetFixedAttributeNode.java
index fd79ede63e..cccbc1cdfc 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetFixedAttributeNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SetFixedAttributeNode.java
@@ -35,6 +35,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.attributes.SpecialAttributesFunctions.SetSpecialAttributeNode;
+import com.oracle.truffle.r.nodes.function.opt.ShareObjectNode;
import com.oracle.truffle.r.runtime.RInternalError;
import com.oracle.truffle.r.runtime.data.RAttributable;
import com.oracle.truffle.r.runtime.data.RAttributeStorage;
@@ -126,7 +127,8 @@ public abstract class SetFixedAttributeNode extends FixedAttributeAccessNode {
protected void setAttrInAttributable(RAttributable x, Object value,
@Cached("create()") BranchProfile attrNullProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
DynamicObject attributes;
if (attrStorageProfile.profile(x instanceof RAttributeStorage)) {
attributes = ((RAttributeStorage) x).getAttributes();
@@ -145,6 +147,8 @@ public abstract class SetFixedAttributeNode extends FixedAttributeAccessNode {
}
recursive.execute(attributes, value);
+
+ updateRefCountNode.execute(value);
}
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SpecialAttributesFunctions.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SpecialAttributesFunctions.java
index e596ca4f5b..bc0e2c8a3a 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SpecialAttributesFunctions.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/SpecialAttributesFunctions.java
@@ -31,6 +31,7 @@ import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.profiles.LoopConditionProfile;
import com.oracle.truffle.api.profiles.ValueProfile;
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctionsFactory.GetDimAttributeNodeGen;
+import com.oracle.truffle.r.nodes.function.opt.ShareObjectNode;
import com.oracle.truffle.r.runtime.RError;
import com.oracle.truffle.r.runtime.RInternalError;
import com.oracle.truffle.r.runtime.RRuntime;
@@ -318,7 +319,8 @@ public final class SpecialAttributesFunctions {
@Cached("create()") SetDimNamesAttributeNode setDimNamesNode,
@Cached("create()") BranchProfile attrNullProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
RVector<?> xProfiled = xTypeProfile.profile(x);
if (newNames.getLength() > xProfiled.getLength()) {
namesTooLongProfile.enter();
@@ -342,7 +344,7 @@ public final class SpecialAttributesFunctions {
return;
}
- super.setAttrInAttributable(xProfiled, newNames, attrNullProfile, attrStorageProfile, xTypeProfile);
+ super.setAttrInAttributable(xProfiled, newNames, attrNullProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
}
@@ -454,7 +456,8 @@ public final class SpecialAttributesFunctions {
protected void setOneDimInVector(RVector<?> x, int dim,
@Cached("create()") BranchProfile attrNullProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
RAbstractContainer xProfiled = contArgClassProfile.profile(x);
int[] dims = new int[]{dim};
@@ -467,17 +470,19 @@ public final class SpecialAttributesFunctions {
attrNullProfile.enter();
attrs = RAttributesLayout.createDim(dimVec);
xProfiled.initAttributes(attrs);
+ updateRefCountNode.execute(dimVec);
return;
}
- super.setAttrInAttributable(x, dimVec, attrNullProfile, attrStorageProfile, xTypeProfile);
+ super.setAttrInAttributable(x, dimVec, attrNullProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
@Specialization(insertBefore = "setAttrInAttributable")
protected void setDimsInVector(RVector<?> x, RAbstractIntVector dims,
@Cached("create()") BranchProfile attrNullProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
RAbstractContainer xProfiled = contArgClassProfile.profile(x);
verifyDimensions(xProfiled.getLength(), dims);
@@ -486,10 +491,11 @@ public final class SpecialAttributesFunctions {
attrNullProfile.enter();
attrs = RAttributesLayout.createDim(dims);
xProfiled.initAttributes(attrs);
+ updateRefCountNode.execute(dims);
return;
}
- super.setAttrInAttributable(x, dims, attrNullProfile, attrStorageProfile, xTypeProfile);
+ super.setAttrInAttributable(x, dims, attrNullProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
@Specialization(insertBefore = "setAttrInAttributable")
@@ -683,7 +689,8 @@ public final class SpecialAttributesFunctions {
@Cached("create()") BranchProfile resizeDimsProfile,
@Cached("create()") BranchProfile attrNullProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
int[] dimensions = getDimNode.getDimensions(x);
if (dimensions == null) {
nullDimsProfile.enter();
@@ -727,10 +734,11 @@ public final class SpecialAttributesFunctions {
if (x.getAttributes() == null) {
attrNullProfile.enter();
x.initAttributes(RAttributesLayout.createDimNames(resDimNames));
+ updateRefCountNode.execute(resDimNames);
return;
}
- super.setAttrInAttributable(x, resDimNames, attrNullProfile, attrStorageProfile, xTypeProfile);
+ super.setAttrInAttributable(x, resDimNames, attrNullProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
private static boolean isValidDimLength(RStringVector x, int expectedDim) {
@@ -824,13 +832,15 @@ public final class SpecialAttributesFunctions {
protected void setRowNamesInVector(RVector<?> x, RAbstractVector newRowNames,
@Cached("create()") BranchProfile attrNullProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
if (x.getAttributes() == null) {
attrNullProfile.enter();
x.initAttributes(RAttributesLayout.createRowNames(newRowNames));
+ updateRefCountNode.execute(newRowNames);
return;
}
- setAttrInAttributable(x, newRowNames, attrNullProfile, attrStorageProfile, xTypeProfile);
+ setAttrInAttributable(x, newRowNames, attrNullProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
@Specialization(insertBefore = "setAttrInAttributable")
@@ -923,8 +933,9 @@ public final class SpecialAttributesFunctions {
@Cached("createBinaryProfile()") ConditionProfile nullClassProfile,
@Cached("createBinaryProfile()") ConditionProfile notNullClassProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
- handleVector(vector, null, removeClassAttrNode, initAttrProfile, nullAttrProfile, nullClassProfile, notNullClassProfile, attrStorageProfile, xTypeProfile);
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
+ handleVector(vector, null, removeClassAttrNode, initAttrProfile, nullAttrProfile, nullClassProfile, notNullClassProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
@Specialization(insertBefore = "setAttrInAttributable")
@@ -935,7 +946,8 @@ public final class SpecialAttributesFunctions {
@Cached("createBinaryProfile()") ConditionProfile nullClassProfile,
@Cached("createBinaryProfile()") ConditionProfile notNullClassProfile,
@Cached("createBinaryProfile()") ConditionProfile attrStorageProfile,
- @Cached("createClassProfile()") ValueProfile xTypeProfile) {
+ @Cached("createClassProfile()") ValueProfile xTypeProfile,
+ @Cached("create()") ShareObjectNode updateRefCountNode) {
DynamicObject attrs = vector.getAttributes();
boolean initializeAttrs = initAttrProfile.profile(attrs == null && classAttr != null && classAttr.getLength() != 0);
@@ -943,6 +955,7 @@ public final class SpecialAttributesFunctions {
nullAttrProfile.enter();
attrs = RAttributesLayout.createClass(classAttr);
vector.initAttributes(attrs);
+ updateRefCountNode.execute(classAttr);
}
if (nullClassProfile.profile(attrs != null && (classAttr == null || classAttr.getLength() == 0))) {
removeAttributeMapping(vector, attrs, removeClassAttrNode);
@@ -953,7 +966,7 @@ public final class SpecialAttributesFunctions {
// TODO: Isn't this redundant when the same operation is done after the
// loop?
if (!initializeAttrs) {
- super.setAttrInAttributable(vector, classAttr, nullAttrProfile, attrStorageProfile, xTypeProfile);
+ super.setAttrInAttributable(vector, classAttr, nullAttrProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
// setClassAttrNode.execute(attrs, classAttr);
if (vector.getElementClass() != RInteger.class) {
@@ -971,7 +984,7 @@ public final class SpecialAttributesFunctions {
}
if (!initializeAttrs) {
- super.setAttrInAttributable(vector, classAttr, nullAttrProfile, attrStorageProfile, xTypeProfile);
+ super.setAttrInAttributable(vector, classAttr, nullAttrProfile, attrStorageProfile, xTypeProfile, updateRefCountNode);
}
}
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/UpdateSharedAttributeNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/UpdateShareableChildValueNode.java
similarity index 67%
rename from com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/UpdateSharedAttributeNode.java
rename to com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/UpdateShareableChildValueNode.java
index 58bbb08234..34ec149990 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/UpdateSharedAttributeNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/attributes/UpdateShareableChildValueNode.java
@@ -27,11 +27,12 @@ import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.profiles.ValueProfile;
import com.oracle.truffle.r.runtime.data.RShareable;
+import com.oracle.truffle.r.runtime.data.RSharingAttributeStorage;
import com.oracle.truffle.r.runtime.nodes.RBaseNode;
-public abstract class UpdateSharedAttributeNode extends RBaseNode {
+public abstract class UpdateShareableChildValueNode extends RBaseNode {
- protected UpdateSharedAttributeNode() {
+ protected UpdateShareableChildValueNode() {
}
public abstract void execute(Object owner, Object attrValue);
@@ -41,12 +42,13 @@ public abstract class UpdateSharedAttributeNode extends RBaseNode {
return item;
}
- public static UpdateSharedAttributeNode create() {
- return UpdateSharedAttributeNodeGen.create();
+ public static UpdateShareableChildValueNode create() {
+ return UpdateShareableChildValueNodeGen.create();
}
@Specialization
protected void doShareableValues(RShareable owner, RShareable value,
+ @Cached("createClassProfile()") ValueProfile ownerProfile,
@Cached("createClassProfile()") ValueProfile valueProfile,
@Cached("createBinaryProfile()") ConditionProfile sharedValue,
@Cached("createBinaryProfile()") ConditionProfile temporaryOwner) {
@@ -56,17 +58,34 @@ public abstract class UpdateSharedAttributeNode extends RBaseNode {
return;
}
+ if (owner instanceof RSharingAttributeStorage) {
+ // monomorphic invocations of the owner
+ RSharingAttributeStorage shOwner = (RSharingAttributeStorage) owner;
+ incRef(shOwner, profiledValue, temporaryOwner);
+ } else {
+ // invoking a type-profiled owner
+ RShareable ownerProfiled = ownerProfile.profile(owner);
+ incRef(ownerProfiled, profiledValue, temporaryOwner);
+ }
+ }
+
+ private static void incRef(RShareable owner, RShareable value, ConditionProfile temporaryOwner) {
if (temporaryOwner.profile(owner.isTemporary())) {
+ // This can happen, for example, when we immediately extract out of a temporary
+ // list that was returned by a built-in, like: strsplit(...)[[1L]]. We do not need
+ // to transition the element, it may stay temporary.
return;
}
- if (profiledValue.isTemporary()) {
+ // the owner is at least non-shared
+
+ if (value.isTemporary()) {
// make it at least non-shared (the owner must be also at least non-shared)
- profiledValue.incRefCount();
+ value.incRefCount();
}
if (owner.isShared()) {
// owner is shared, make the attribute value shared too
- profiledValue.incRefCount();
+ value.incRefCount();
}
}
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_attrassign.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_attrassign.java
index b8b553bad9..ab83f538b2 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_attrassign.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_attrassign.java
@@ -98,6 +98,12 @@ public class TestBuiltin_attrassign extends TestBase {
assertEval("argv <- list(structure(c(0, -187, -34, 0, 165, 0, -95, 121, 107, 0, 41, 0, 0, 93, 0), .Dim = c(5L, 3L)), 'dimnames', value = NULL);`attr<-`(argv[[1]],argv[[2]],argv[[3]]);");
}
+ @Test
+ public void testRefCount() {
+ assertEval("x <- c(1,2); attr(x, \"foo\") <- c(\"a\",\"b\"); y <- x; attr(x,\"foo\")[[1]] <- \"c\"; y");
+ assertEval("x <- c(1,2,3); y <- 42; attr(y, 'at') <- x; x[[1]] <- 2; attr(y, 'at')");
+ }
+
@Test
public void testArgsCasts() {
assertEval(Output.IgnoreErrorContext, "x<-42; attr(x, NULL) <- NULL");
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_list.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_list.java
index 0317997c24..e612304d8f 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_list.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_list.java
@@ -347,6 +347,11 @@ public class TestBuiltin_list extends TestBase {
"argv <- list(ANY = structure(function (x, y = NULL) .Internal(crossprod(x, y)), target = structure('ANY', class = structure('signature', package = 'methods'), .Names = 'x', package = 'methods'), defined = structure('ANY', class = structure('signature', package = 'methods'), .Names = 'x', package = 'methods'), generic = structure('crossprod', package = 'base'), class = structure('derivedDefaultMethod', package = 'methods')));list(argv[[1]]);");
}
+ @Test
+ public void testRefCount() {
+ assertEval("{ l <- list(a=c(1,2)); l2 <- l; l$a[[1]] <- 3; l2 }");
+ }
+
@Test
public void testList() {
assertEval("{ list(a=1, b=2) }");
--
GitLab