From 13b0c3f3fba988a8206d1476333785c6346c234e Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Thu, 8 Sep 2016 10:19:06 +0200
Subject: [PATCH] Reference counting for list elements
Lists can hold elements that are not in a consistent state with respect to the sharing model.
However, such elements are put into consistent state once they are read from the list, their
state can be inferred from the state of the owning list.
This means that temporary elements can be freely put into lists without incrementing their ref-count.
Moreover, when a list is copied into another variable or passed as an argument to a function,
we only have to increment the ref-count of the list (no recursion needed).
When accessing an element of a list, one should use ExtractListElement node, which handles
the transition to consistent state. This must be used wherever the state matters: when returning
the value as a result of a builtin, putting it into another list, putting it as an attribute, etc.
On the other hand, when one only wants to peek at the value, but then forget the reference,
it is OK to get it directly using getDataAt and similar methods.
---
.../oracle/truffle/r/nodes/RASTBuilder.java | 2 +-
.../vector/CachedReplaceVectorNode.java | 22 ++--
.../access/vector/ExtractListElement.java | 119 ++++++++++++++++++
.../access/vector/WriteIndexedVectorNode.java | 27 ++--
.../function/opt/ReuseNonSharedNode.java | 19 ++-
.../r/nodes/function/opt/ShareObjectNode.java | 59 +++++++++
.../truffle/r/runtime/data/RExpression.java | 3 +-
.../truffle/r/runtime/data/RListBase.java | 33 ++++-
.../truffle/r/runtime/data/RShareable.java | 6 +
.../data/RSharingAttributeStorage.java | 1 +
.../data/model/RAbstractContainer.java | 5 +
.../truffle/r/test/ExpectedTestOutput.test | 6 +
.../r/test/library/base/TestSimpleLists.java | 1 +
13 files changed, 266 insertions(+), 37 deletions(-)
create mode 100644 com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/ExtractListElement.java
create mode 100644 com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ShareObjectNode.java
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTBuilder.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTBuilder.java
index 9fd0300971..7cc0b1120e 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTBuilder.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTBuilder.java
@@ -322,7 +322,7 @@ public final class RASTBuilder implements RCodeBuilder<RSyntaxNode> {
if (i < calls.size() - 1) {
instructions.add(WriteVariableNode.createAnonymous("*tmpr*" + (tempNamesIndex + i), update, WriteVariableNode.Mode.INVISIBLE));
} else {
- instructions.add(WriteVariableNode.createAnonymous(variable.getIdentifier(), update, WriteVariableNode.Mode.INVISIBLE, isSuper));
+ instructions.add(WriteVariableNode.createAnonymous(variable.getIdentifier(), update, WriteVariableNode.Mode.REGULAR, isSuper));
}
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/CachedReplaceVectorNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/CachedReplaceVectorNode.java
index b11e1480cb..2d88a57700 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/CachedReplaceVectorNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/CachedReplaceVectorNode.java
@@ -244,16 +244,21 @@ final class CachedReplaceVectorNode extends CachedVectorNode {
return wrapResult(vector, repType);
}
vector = resizeVector(vector, maxOutOfBounds);
+ } else {
+ vector = vector.materialize();
}
- vector = vector.materialize();
- if (originalVector != vector && vector instanceof RShareable) {
- RShareable shareable = (RShareable) vector;
- // we created a new object, and this needs to be non-temporary
- if (shareable.isTemporary()) {
- shareable.incRefCount();
- }
- }
+ // Note: the refCount of elements inside lists can stay the same. If we are replacing in a
+ // what was originally shared list, we made a shallow copy of it, but all its elements must
+ // be in shared state already anyway. If we are replacing in non-shared list and we just
+ // threw it away to replace it with larger one, the elements are in non-shared or temporary
+ // state, which is again OK.
+ //
+ // The refcount of the list/vector itself: if the write node in "v <- `$<-`(...)" sees that
+ // we are assigning the same object that 'v' already contains, it is going to skip the
+ // assignment and skip the refCount increment. If we created a new object by
+ // resizing/materializing 'vector', it will be marked as 'temporary' and its refCount
+ // incremented during the assignment step.
vectorLength = targetLengthProfile.profile(vector.getLength());
@@ -487,7 +492,6 @@ final class CachedReplaceVectorNode extends CachedVectorNode {
shareable = (RShareable) returnVector.copy();
returnVector = (RAbstractVector) shareable;
assert shareable.isTemporary();
- shareable.incRefCount();
}
}
returnVector = sharedClassProfile.profile(returnVector);
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
new file mode 100644
index 0000000000..3a292f4988
--- /dev/null
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/vector/ExtractListElement.java
@@ -0,0 +1,119 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.truffle.r.nodes.access.vector;
+
+import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.Fallback;
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.profiles.ConditionProfile;
+import com.oracle.truffle.r.nodes.access.vector.ExtractListElementNodeGen.UpdateStateOfListElementNodeGen;
+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.
+ *
+ * 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
+ * directly through {@link RAbstractListVector#getDataAt(int)} or
+ * {@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}.
+ */
+public abstract class ExtractListElement extends Node {
+
+ public abstract Object execute(RAbstractContainer container, int index);
+
+ @Specialization
+ protected Object doList(RListBase list, int index, @Cached("create()") UpdateStateOfListElement updateStateNode) {
+ Object element = list.getDataAt(index);
+ return updateStateNode.updateState(list, element);
+ }
+
+ @Specialization(guards = "isNotList(container)")
+ protected Object doOthers(RAbstractContainer container, int index) {
+ return container.getDataAtAsObject(index);
+ }
+
+ protected static boolean isNotList(RAbstractContainer x) {
+ return !(x instanceof RAbstractListVector);
+ }
+
+ 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("createBinaryProfile()") ConditionProfile sharedValue, //
+ @Cached("createBinaryProfile()") ConditionProfile temporaryOwner) {
+ if (sharedValue.profile(value.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 (value.isTemporary()) {
+ // make it at least non-shared (parent list must be also at least non-shared)
+ value.incRefCount();
+ }
+ if (owner.isShared()) {
+ // owner is shared, make the value shared too
+ value.incRefCount();
+ }
+ }
+
+ @Fallback
+ 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)
+ }
+ }
+}
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 bb5f786efd..a927fefac5 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,6 +32,8 @@ 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.function.opt.ShareObjectNode;
import com.oracle.truffle.r.nodes.profile.AlwaysOnBranchProfile;
import com.oracle.truffle.r.nodes.profile.IntValueProfile;
import com.oracle.truffle.r.nodes.profile.VectorLengthProfile;
@@ -43,7 +45,6 @@ import com.oracle.truffle.r.runtime.data.RComplex;
import com.oracle.truffle.r.runtime.data.RIntSequence;
import com.oracle.truffle.r.runtime.data.RMissing;
import com.oracle.truffle.r.runtime.data.RScalarVector;
-import com.oracle.truffle.r.runtime.data.RShareable;
import com.oracle.truffle.r.runtime.data.RTypedValue;
import com.oracle.truffle.r.runtime.data.model.RAbstractComplexVector;
import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
@@ -473,17 +474,17 @@ 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;
WriteListAction(boolean setListElementAsObject, boolean isReplace) {
this.setListElementAsObject = setListElementAsObject;
this.isReplace = isReplace;
- }
-
- private Object copyValueOnAssignment(Object value) {
- if (isShareable.profile(value instanceof RShareable)) {
- ((RShareable) value).incRefCount();
+ if (!isReplace) {
+ updateStateOfListElement = insert(UpdateStateOfListElement.create());
+ } else {
+ shareObjectNode = insert(ShareObjectNode.create());
}
- return value;
}
@Override
@@ -498,9 +499,17 @@ abstract class WriteIndexedVectorNode extends Node {
} else {
rightValue = ((RAbstractContainer) rightAccess).getDataAtAsObject(rightStore, rightIndex);
}
- if (isReplace && leftAccess.getDataAtAsObject(leftStore, leftIndex) != rightValue) {
- rightValue = copyValueOnAssignment(rightValue);
+
+ if (isReplace) {
+ // we are replacing within the same list
+ if (leftAccess.getDataAtAsObject(leftStore, leftIndex) != rightValue) {
+ shareObjectNode.execute(rightValue);
+ }
+ } else {
+ // we are writing into a list data that are being read from possibly another list
+ updateStateOfListElement.execute(rightAccess, rightValue);
}
+
leftAccess.setDataAt(leftStore, leftIndex, rightValue);
valueNACheck.checkListElement(rightValue);
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ReuseNonSharedNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ReuseNonSharedNode.java
index 70aa1febcc..2c16f2947c 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ReuseNonSharedNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ReuseNonSharedNode.java
@@ -30,6 +30,12 @@ import com.oracle.truffle.api.profiles.ValueProfile;
import com.oracle.truffle.r.runtime.data.RShareable;
import com.oracle.truffle.r.runtime.data.RSharingAttributeStorage;
+/**
+ * Internal node that should be used whenever you want to alter some data: if the data is shared,
+ * then it creates a copy, otherwise it returns the data. It does not increment the reference count
+ * of the result in either case, but that is typically handled by write variable node, put container
+ * element node or by put attribute node.
+ */
public abstract class ReuseNonSharedNode extends Node {
public static ReuseNonSharedNode create() {
@@ -41,33 +47,22 @@ public abstract class ReuseNonSharedNode extends Node {
@Specialization
protected RShareable getStorage(RSharingAttributeStorage value, //
@Cached("createBinaryProfile()") ConditionProfile isSharedProfile, //
- @Cached("createBinaryProfile()") ConditionProfile isTemporaryProfile, //
@Cached("createClassProfile()") ValueProfile copyProfile) {
if (isSharedProfile.profile(value.isShared())) {
RShareable res = copyProfile.profile(value).copy();
assert res.isTemporary();
- res.incRefCount();
return res;
}
- if (isTemporaryProfile.profile(value.isTemporary())) {
- value.incRefCount();
- }
return value;
}
@Specialization(contains = "getStorage")
protected static RShareable getRShareable(RShareable value, //
- @Cached("createBinaryProfile()") ConditionProfile isSharedProfile, //
- @Cached("createBinaryProfile()") ConditionProfile isTemporaryProfile) {
+ @Cached("createBinaryProfile()") ConditionProfile isSharedProfile) {
if (isSharedProfile.profile(value.isShared())) {
RShareable res = value.copy();
- assert res.isTemporary();
- res.incRefCount();
return res;
}
- if (isTemporaryProfile.profile(value.isTemporary())) {
- value.incRefCount();
- }
return value;
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ShareObjectNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ShareObjectNode.java
new file mode 100644
index 0000000000..1804aefa97
--- /dev/null
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/opt/ShareObjectNode.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.truffle.r.nodes.function.opt;
+
+import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.profiles.ConditionProfile;
+import com.oracle.truffle.r.runtime.data.RShareable;
+
+/**
+ * Internal node that should be used whenever you need to increment reference count of some object.
+ * If the object is not instance of {@link RShareable} or if it is shared permanent, then does
+ * nothing.
+ */
+public abstract class ShareObjectNode extends Node {
+ public abstract Object execute(Object obj);
+
+ public static ShareObjectNode create() {
+ return ShareObjectNodeGen.create();
+ }
+
+ @Specialization
+ protected Object doShareable(RShareable obj, @Cached("createBinaryProfile()") ConditionProfile sharedPermanent) {
+ if (sharedPermanent.profile(!obj.isSharedPermanent())) {
+ obj.incRefCount();
+ }
+ return obj;
+ }
+
+ @Specialization(guards = "!isRShareable(obj)")
+ protected Object doNonShareable(Object obj) {
+ return obj;
+ }
+
+ protected static boolean isRShareable(Object value) {
+ return value instanceof RShareable;
+ }
+}
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RExpression.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RExpression.java
index db152ea75c..d787eefaa1 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RExpression.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RExpression.java
@@ -197,7 +197,8 @@ public class RExpression implements RShareable, RAbstractContainer {
@Override
public RShareable makeSharedPermanent() {
- return data.makeSharedPermanent();
+ data.makeSharedPermanent();
+ return this;
}
@Override
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java
index 5c00f39e98..981f73569a 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java
@@ -32,8 +32,19 @@ import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
import com.oracle.truffle.r.runtime.ops.na.NACheck;
/**
- * Important note: none of the methods, including e.g. {@link #copy()} do not handle
- * {@link RShareable}. It is responsibility of the caller to increment refCount appropriately.
+ * Note on sharing mode for list elements: by default the sharing state of elements in a list can be
+ * inconsistent, e.g. a list referenced by one local variable may contain temporary vectors, or list
+ * references by more variables (shared list) may contain non-shared elements. The sharing state of
+ * the elements should be made consistent on reading from the list by the callers! When we read from
+ * shared list, we make the element shared. When we read from non-shared list, we make the element
+ * at least non-shared. There is no possible way, how a list can contain a non-shared element not
+ * owned by it, given that any element of some other list must be first read into variable (the
+ * extraction from list makes it at least non-shared, then the write makes it shared) and only then
+ * it can be put inside another list. This is however not true for internal code, which may read
+ * data from a list and then put it into another list, in such case it is responsibility of the code
+ * to increment the refcount of such data. Consult also the documentation of
+ * {@code ExtractListElement}, which is a node that can extract an element of a list or abstract
+ * vector and put it in the consistent sharing state.
*/
public abstract class RListBase extends RVector implements RAbstractListVector {
@@ -55,6 +66,11 @@ public abstract class RListBase extends RVector implements RAbstractListVector {
return data;
}
+ /**
+ * Note: elements inside lists may be in inconsistent state reference counting wise. You may
+ * need to put them into consistent state depending on what you use them for, consult the
+ * documentation of {@code ExtractListElement}.
+ */
@Override
public Object getDataAtAsObject(Object store, int index) {
assert store == data;
@@ -113,6 +129,11 @@ public abstract class RListBase extends RVector implements RAbstractListVector {
return isTemporary() ? getDataWithoutCopying() : getDataCopy();
}
+ /**
+ * Note: elements inside lists may be in inconsistent state reference counting wise. You may
+ * need to put them into consistent state depending on what you use them for, consult the
+ * documentation of {@code ExtractListElement}.
+ */
@Override
public final Object getDataAt(int i) {
return data[i];
@@ -123,9 +144,6 @@ public abstract class RListBase extends RVector implements RAbstractListVector {
return RRuntime.toString(getDataAt(index));
}
- /*
- * This method does not increment the reference count, it is responsibility of the caller.
- */
public final RListBase updateDataAt(int i, Object right, @SuppressWarnings("unused") NACheck rightNACheck) {
assert !this.isShared() : "data in shared list must not be updated, make a copy";
data[i] = right;
@@ -167,6 +185,11 @@ public abstract class RListBase extends RVector implements RAbstractListVector {
}
}
+ /**
+ * Note: elements inside lists may be in inconsistent state reference counting wise. You may
+ * need to put them into consistent state depending on what you use them for, consult the
+ * documentation of {@code ExtractListElement}.
+ */
@Override
public final Object getDataAtAsObject(int index) {
return this.getDataAt(index);
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RShareable.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RShareable.java
index 6229258c14..5729a5ce08 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RShareable.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RShareable.java
@@ -40,8 +40,14 @@ public interface RShareable extends RTypedValue {
return copy();
}
+ /**
+ * It is invalid to invoke this method on 'shared permanent' shareables.
+ */
void incRefCount();
+ /**
+ * It is invalid to invoke this method on 'temporary' shareables.
+ */
void decRefCount();
boolean isSharedPermanent();
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 e049da26b8..90032b7255 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
@@ -41,6 +41,7 @@ public abstract class RSharingAttributeStorage extends RAttributeStorage impleme
@Override
public final void incRefCount() {
+ assert refCount != SHARED_PERMANENT_VAL : "cannot incRefCount of shared permanent value";
refCount++;
}
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/model/RAbstractContainer.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/model/RAbstractContainer.java
index befd4e8678..2125cce5ee 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/model/RAbstractContainer.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/model/RAbstractContainer.java
@@ -51,6 +51,11 @@ public interface RAbstractContainer extends RAttributable, RTypedValue {
Object getDataAtAsObject(int index);
+ /**
+ * Note: elements inside lists may be in inconsistent state reference counting wise. You may
+ * need to put them into consistent state depending on what you use them for, consult the
+ * documentation of {@code ExtractListElement}.
+ */
default Object getDataAtAsObject(@SuppressWarnings("unused") Object store, int index) {
return getDataAtAsObject(index);
}
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
index 07c05794d5..6e6ff64b27 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
@@ -64229,6 +64229,12 @@ $`1`
[1] 1
+##com.oracle.truffle.r.test.library.base.TestSimpleLists.testListRefcounting
+#x <- c(1,2,3); l <- list(x); x[[1]] <- 42; l;
+[[1]]
+[1] 1 2 3
+
+
##com.oracle.truffle.r.test.library.base.TestSimpleLists.testListRefcounting
#z <- c(1,4,8); a<-list(); a$x <- z; a$x[[1]] <- 42; list(a=a, z=z)
$a
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestSimpleLists.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestSimpleLists.java
index 4351ff70cf..dcd71d109a 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestSimpleLists.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestSimpleLists.java
@@ -120,6 +120,7 @@ public class TestSimpleLists extends TestBase {
// lists returned from built-ins
assertEval("r <- split(1,1); r[[1]] / 2; r;");
assertEval("r <- split(1,1); x <- r; r[[1]] <- 42; x;");
+ assertEval("x <- c(1,2,3); l <- list(x); x[[1]] <- 42; l;");
// (potential) cycles
assertEval(Ignored.Unimplemented, "l <- list(list(list())); l[[1]][[1]] <- l; l");
--
GitLab