From 5c2d0033147495c9367b7379b2f9aee86288c851 Mon Sep 17 00:00:00 2001
From: Lukas Stadler <lukas.stadler@oracle.com>
Date: Mon, 12 Dec 2016 10:55:07 +0100
Subject: [PATCH] factor out String interning in Attr/UpdateAttr and make it
thread safe
---
.../truffle/r/nodes/builtin/base/Attr.java | 35 ++----------
.../r/nodes/builtin/base/UpdateAttr.java | 55 +++++++++----------
2 files changed, 33 insertions(+), 57 deletions(-)
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 662b6e581b..0329bf95e8 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
@@ -28,8 +28,6 @@ import static com.oracle.truffle.r.nodes.builtin.CastBuilder.Predef.toBoolean;
import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
import static com.oracle.truffle.r.runtime.builtins.RBuiltinKind.PRIMITIVE;
-import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Fallback;
@@ -42,11 +40,12 @@ import com.oracle.truffle.r.nodes.attributes.IterableAttributeNode;
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.GetRowNamesAttributeNode;
import com.oracle.truffle.r.nodes.builtin.CastBuilder;
import com.oracle.truffle.r.nodes.builtin.RBuiltinNode;
+import com.oracle.truffle.r.nodes.builtin.base.UpdateAttr.InternStringNode;
+import com.oracle.truffle.r.nodes.builtin.base.UpdateAttrNodeGen.InternStringNodeGen;
import com.oracle.truffle.r.nodes.function.opt.UpdateShareableChildValueNode;
import com.oracle.truffle.r.runtime.RError;
import com.oracle.truffle.r.runtime.RError.Message;
import com.oracle.truffle.r.runtime.RRuntime;
-import com.oracle.truffle.r.runtime.Utils;
import com.oracle.truffle.r.runtime.builtins.RBuiltin;
import com.oracle.truffle.r.runtime.data.RAttributable;
import com.oracle.truffle.r.runtime.data.RAttributesLayout;
@@ -64,9 +63,9 @@ public abstract class Attr extends RBuiltinNode {
private final ConditionProfile searchPartialProfile = ConditionProfile.createBinaryProfile();
private final BranchProfile errorProfile = BranchProfile.create();
- @CompilationFinal private String cachedName = "";
- @CompilationFinal private String cachedInternedName = "";
@Child private UpdateShareableChildValueNode sharedAttrUpdate = UpdateShareableChildValueNode.create();
+ @Child private InternStringNode intern = InternStringNodeGen.create();
+
@Child private GetAttributeNode attrAccess = GetAttributeNode.create();
@Child private IterableAttributeNode iterAttrAccess = IterableAttributeNode.create();
@@ -83,28 +82,6 @@ public abstract class Attr extends RBuiltinNode {
casts.arg("exact").asLogicalVector().findFirst().map(toBoolean());
}
- private String intern(String name) {
- if (cachedName == null) {
- // unoptimized case
- return Utils.intern(name);
- }
- if (cachedName == name) {
- // cached case
- return cachedInternedName;
- }
- CompilerDirectives.transferToInterpreterAndInvalidate();
- // Checkstyle: stop StringLiteralEquality
- if (cachedName == "") {
- // Checkstyle: resume StringLiteralEquality
- cachedName = name;
- cachedInternedName = Utils.intern(name);
- } else {
- cachedName = null;
- cachedInternedName = null;
- }
- return Utils.intern(name);
- }
-
private Object searchKeyPartial(DynamicObject attributes, String name) {
Object val = RNull.instance;
@@ -141,7 +118,7 @@ public abstract class Attr extends RBuiltinNode {
@Specialization(guards = "!isRowNamesAttr(name)")
protected Object attr(RAbstractContainer container, String name, boolean exact) {
- return attrRA(container, intern(name), exact);
+ return attrRA(container, intern.execute(name), exact);
}
@Specialization(guards = "isRowNamesAttr(name)")
@@ -163,7 +140,7 @@ public abstract class Attr extends RBuiltinNode {
@TruffleBoundary
protected Object attr(Object object, Object name, Object exact) {
if (object instanceof RAttributable) {
- return attrRA((RAttributable) object, intern((String) name), (boolean) exact);
+ return attrRA((RAttributable) object, intern.execute((String) name), (boolean) exact);
} else {
errorProfile.enter();
throw RError.nyi(this, "object cannot be attributed");
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttr.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttr.java
index 34c1d680f8..44c90112ee 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttr.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttr.java
@@ -29,17 +29,21 @@ import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
import static com.oracle.truffle.r.runtime.builtins.RBuiltinKind.PRIMITIVE;
import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+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.dsl.TypeSystemReference;
+import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.profiles.BranchProfile;
+import com.oracle.truffle.r.nodes.EmptyTypeSystemFlatLayout;
import com.oracle.truffle.r.nodes.attributes.SetAttributeNode;
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetClassAttributeNode;
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetDimAttributeNode;
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetRowNamesAttributeNode;
import com.oracle.truffle.r.nodes.builtin.CastBuilder;
import com.oracle.truffle.r.nodes.builtin.RBuiltinNode;
+import com.oracle.truffle.r.nodes.builtin.base.UpdateAttrNodeGen.InternStringNodeGen;
import com.oracle.truffle.r.nodes.unary.CastIntegerNode;
import com.oracle.truffle.r.nodes.unary.CastIntegerNodeGen;
import com.oracle.truffle.r.nodes.unary.CastListNode;
@@ -75,8 +79,25 @@ public abstract class UpdateAttr extends RBuiltinNode {
@Child private SetAttributeNode setGenAttrNode;
@Child private SetDimAttributeNode setDimNode;
- @CompilationFinal private String cachedName = "";
- @CompilationFinal private String cachedInternedName = "";
+ @Child private InternStringNode intern = InternStringNodeGen.create();
+
+ @TypeSystemReference(EmptyTypeSystemFlatLayout.class)
+ public abstract static class InternStringNode extends Node {
+
+ public abstract String execute(String value);
+
+ @Specialization(limit = "3", guards = "value == cachedValue")
+ protected static String internCached(@SuppressWarnings("unused") String value,
+ @SuppressWarnings("unused") @Cached("value") String cachedValue,
+ @Cached("intern(value)") String interned) {
+ return interned;
+ }
+
+ @Specialization(contains = "internCached")
+ protected static String intern(String value) {
+ return Utils.intern(value);
+ }
+ }
@Override
protected void createCasts(CastBuilder casts) {
@@ -118,31 +139,9 @@ public abstract class UpdateAttr extends RBuiltinNode {
return (RAbstractVector) castVector.execute(value);
}
- private String intern(String name) {
- if (cachedName == null) {
- // unoptimized case
- return Utils.intern(name);
- }
- if (cachedName == name) {
- // cached case
- return cachedInternedName;
- }
- CompilerDirectives.transferToInterpreterAndInvalidate();
- // Checkstyle: stop StringLiteralEquality
- if (cachedName == "") {
- // Checkstyle: resume StringLiteralEquality
- cachedName = name;
- cachedInternedName = Utils.intern(name);
- } else {
- cachedName = null;
- cachedInternedName = null;
- }
- return Utils.intern(name);
- }
-
@Specialization
protected RAbstractContainer updateAttr(RAbstractContainer container, String name, RNull value) {
- String internedName = intern(name);
+ String internedName = intern.execute(name);
RAbstractContainer result = (RAbstractContainer) container.getNonShared();
// the name is interned, so identity comparison is sufficient
if (internedName == RRuntime.DIM_ATTR_KEY) {
@@ -187,7 +186,7 @@ public abstract class UpdateAttr extends RBuiltinNode {
@Specialization(guards = "!nullValue(value)")
protected RAbstractContainer updateAttr(RAbstractContainer container, String name, Object value) {
- String internedName = intern(name);
+ String internedName = intern.execute(name);
RAbstractContainer result = (RAbstractContainer) container.getNonShared();
// the name is interned, so identity comparison is sufficient
if (internedName == RRuntime.DIM_ATTR_KEY) {
@@ -246,7 +245,7 @@ public abstract class UpdateAttr extends RBuiltinNode {
if (object instanceof RShareable) {
object = ((RShareable) object).getNonShared();
}
- String internedName = intern((String) name);
+ String internedName = intern.execute((String) name);
if (object instanceof RAttributable) {
RAttributable attributable = (RAttributable) object;
if (value == RNull.instance) {
--
GitLab