From 185b8b6298a22d41ed0fca092344dfff33394e63 Mon Sep 17 00:00:00 2001 From: Lukas Stadler <lukas.stadler@oracle.com> Date: Mon, 12 Dec 2016 12:09:51 +0100 Subject: [PATCH] thread safety for UseMethodFunctionLookup, remove UseMethodInternalNode and NoGenericMethodException --- .../r/nodes/builtin/base/AsVector.java | 26 +++++---- .../truffle/r/nodes/builtin/base/Bind.java | 21 ++++--- .../truffle/r/nodes/function/RCallNode.java | 13 +---- .../nodes/function/S3FunctionLookupNode.java | 35 +++++------ .../nodes/function/UseMethodInternalNode.java | 58 ------------------- 5 files changed, 48 insertions(+), 105 deletions(-) delete mode 100644 com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/UseMethodInternalNode.java diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/AsVector.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/AsVector.java index 92d043c445..6f3c3bc9c0 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/AsVector.java +++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/AsVector.java @@ -41,10 +41,11 @@ import com.oracle.truffle.r.nodes.builtin.CastBuilder; import com.oracle.truffle.r.nodes.builtin.RBuiltinNode; import com.oracle.truffle.r.nodes.builtin.base.AsVectorNodeGen.AsVectorInternalNodeGen; import com.oracle.truffle.r.nodes.builtin.base.AsVectorNodeGen.AsVectorInternalNodeGen.CastPairListNodeGen; +import com.oracle.truffle.r.nodes.function.CallMatcherNode; import com.oracle.truffle.r.nodes.function.ClassHierarchyNode; import com.oracle.truffle.r.nodes.function.ClassHierarchyNodeGen; import com.oracle.truffle.r.nodes.function.S3FunctionLookupNode; -import com.oracle.truffle.r.nodes.function.UseMethodInternalNode; +import com.oracle.truffle.r.nodes.function.S3FunctionLookupNode.Result; import com.oracle.truffle.r.nodes.unary.CastComplexNode; import com.oracle.truffle.r.nodes.unary.CastDoubleNode; import com.oracle.truffle.r.nodes.unary.CastExpressionNode; @@ -77,7 +78,9 @@ public abstract class AsVector extends RBuiltinNode { @Child private AsVectorInternal internal = AsVectorInternalNodeGen.create(); @Child private ClassHierarchyNode classHierarchy = ClassHierarchyNodeGen.create(false, false); - @Child private UseMethodInternalNode useMethod; + + @Child private S3FunctionLookupNode lookup; + @Child private CallMatcherNode callMatcher; private final ConditionProfile hasClassProfile = ConditionProfile.createBinaryProfile(); @@ -98,16 +101,19 @@ public abstract class AsVector extends RBuiltinNode { // However, removing it causes unit test failures RStringVector clazz = classHierarchy.execute(x); if (hasClassProfile.profile(clazz != null)) { - if (useMethod == null) { - // Note: this dispatch takes care of factor, because there is as.vector.factor - // specialization in R + // Note: this dispatch takes care of factor, because there is as.vector.factor + // specialization in R + if (lookup == null) { CompilerDirectives.transferToInterpreterAndInvalidate(); - useMethod = insert(new UseMethodInternalNode("as.vector", SIGNATURE, false)); + lookup = insert(S3FunctionLookupNode.create(false, false)); } - try { - return useMethod.execute(frame, clazz, new Object[]{x, mode}); - } catch (S3FunctionLookupNode.NoGenericMethodException e) { - // fallthrough + Result lookupResult = lookup.execute(frame, "as.vector", clazz, null, frame.materialize(), null); + if (lookupResult != null) { + if (callMatcher == null) { + CompilerDirectives.transferToInterpreterAndInvalidate(); + callMatcher = insert(CallMatcherNode.create(false)); + } + return callMatcher.execute(frame, SIGNATURE, new Object[]{x, mode}, lookupResult.function, lookupResult.targetFunctionName, lookupResult.createS3Args(frame)); } } return internal.execute(x, mode); diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Bind.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Bind.java index 0a95c508e5..13294d4f36 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Bind.java +++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Bind.java @@ -43,8 +43,9 @@ import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetDimAt import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetDimNamesAttributeNode; import com.oracle.truffle.r.nodes.builtin.CastBuilder; import com.oracle.truffle.r.nodes.builtin.RBuiltinNode; +import com.oracle.truffle.r.nodes.function.CallMatcherNode; import com.oracle.truffle.r.nodes.function.S3FunctionLookupNode; -import com.oracle.truffle.r.nodes.function.UseMethodInternalNode; +import com.oracle.truffle.r.nodes.function.S3FunctionLookupNode.Result; import com.oracle.truffle.r.nodes.helpers.InheritsCheckNode; import com.oracle.truffle.r.nodes.unary.CastComplexNode; import com.oracle.truffle.r.nodes.unary.CastDoubleNode; @@ -97,10 +98,12 @@ public abstract class Bind extends RBaseNode { public abstract Object execute(VirtualFrame frame, int deparseLevel, Object[] args, RArgsValuesAndNames promiseArgs, int precedence); @Child private CastToVectorNode castVector; - @Child private UseMethodInternalNode dcn; @Child private CastLogicalNode castLogical; @Child private GetDimAttributeNode getDimsNode; + @Child private S3FunctionLookupNode lookup; + @Child private CallMatcherNode callMatcher; + private final BindType type; private final ConditionProfile nullNamesProfile = ConditionProfile.createBinaryProfile(); @@ -162,15 +165,19 @@ public abstract class Bind extends RBaseNode { @Specialization(guards = {"args.length > 0", "isDataFrame(args)"}) protected Object allDataFrame(VirtualFrame frame, int deparseLevel, @SuppressWarnings("unused") Object[] args, RArgsValuesAndNames promiseArgs, @SuppressWarnings("unused") int precedence) { - if (dcn == null) { + if (lookup == null) { CompilerDirectives.transferToInterpreterAndInvalidate(); - dcn = insert(new UseMethodInternalNode(type.toString(), SIGNATURE, false)); + lookup = insert(S3FunctionLookupNode.create(false, false)); } - try { - return dcn.execute(frame, DATA_FRAME_CLASS, new Object[]{deparseLevel, promiseArgs}); - } catch (S3FunctionLookupNode.NoGenericMethodException e) { + Result lookupResult = lookup.execute(frame, type.toString(), DATA_FRAME_CLASS, null, frame.materialize(), null); + if (lookupResult == null) { throw RInternalError.shouldNotReachHere(); } + if (callMatcher == null) { + CompilerDirectives.transferToInterpreterAndInvalidate(); + callMatcher = insert(CallMatcherNode.create(false)); + } + return callMatcher.execute(frame, SIGNATURE, new Object[]{deparseLevel, promiseArgs}, lookupResult.function, lookupResult.targetFunctionName, lookupResult.createS3Args(frame)); } private Object bindInternal(int deparseLevel, Object[] args, RArgsValuesAndNames promiseArgs, CastNode castNode, boolean needsVectorCast, SetDimAttributeNode setDimNode, diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java index 9d15a92107..4e172dbb8c 100644 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java +++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java @@ -62,7 +62,6 @@ import com.oracle.truffle.r.nodes.builtin.RBuiltinNode; import com.oracle.truffle.r.nodes.builtin.RBuiltinRootNode; import com.oracle.truffle.r.nodes.function.PromiseHelperNode.PromiseCheckHelperNode; import com.oracle.truffle.r.nodes.function.RCallNodeGen.FunctionDispatchNodeGen; -import com.oracle.truffle.r.nodes.function.S3FunctionLookupNode.NoGenericMethodException; import com.oracle.truffle.r.nodes.function.S3FunctionLookupNode.Result; import com.oracle.truffle.r.nodes.function.call.CallRFunctionNode; import com.oracle.truffle.r.nodes.function.call.PrepareArguments; @@ -399,21 +398,13 @@ public abstract class RCallNode extends RCallBaseNode implements RSyntaxNode, RS RStringVector typeX = classHierarchyNodeX.execute(promiseHelperNode.checkEvaluate(frame, args[typeXIdx])); Result resultX = null; if (implicitTypeProfileX.profile(typeX != null)) { - try { - resultX = dispatchLookupX.execute(frame, builtin.getName(), typeX, dispatch.getGroupGenericName(), frame.materialize(), null); - } catch (NoGenericMethodException e) { - // fall-through - } + resultX = dispatchLookupX.execute(frame, builtin.getName(), typeX, dispatch.getGroupGenericName(), frame.materialize(), null); } Result resultY = null; if (args.length > 1 && dispatch == RDispatch.OPS_GROUP_GENERIC) { RStringVector typeY = classHierarchyNodeY.execute(promiseHelperNode.checkEvaluate(frame, args[1])); if (implicitTypeProfileY.profile(typeY != null)) { - try { - resultY = dispatchLookupY.execute(frame, builtin.getName(), typeY, dispatch.getGroupGenericName(), frame.materialize(), null); - } catch (NoGenericMethodException e) { - // fall-through - } + resultY = dispatchLookupY.execute(frame, builtin.getName(), typeY, dispatch.getGroupGenericName(), frame.materialize(), null); } } diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java index 41082f5156..2ddb547431 100644 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java +++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java @@ -23,7 +23,6 @@ import com.oracle.truffle.api.frame.FrameSlot; import com.oracle.truffle.api.frame.FrameSlotTypeException; import com.oracle.truffle.api.frame.MaterializedFrame; import com.oracle.truffle.api.frame.VirtualFrame; -import com.oracle.truffle.api.nodes.ControlFlowException; import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.nodes.NodeCost; import com.oracle.truffle.api.nodes.NodeInfo; @@ -33,6 +32,7 @@ import com.oracle.truffle.api.profiles.ValueProfile; import com.oracle.truffle.r.nodes.access.variables.LocalReadVariableNode; import com.oracle.truffle.r.nodes.access.variables.ReadVariableNode; import com.oracle.truffle.r.runtime.ArgumentsSignature; +import com.oracle.truffle.r.runtime.RArguments.S3Args; import com.oracle.truffle.r.runtime.RError; import com.oracle.truffle.r.runtime.RInternalError; import com.oracle.truffle.r.runtime.RRuntime; @@ -74,18 +74,22 @@ public abstract class S3FunctionLookupNode extends RBaseNode { this.targetFunctionName = targetFunctionName; this.groupMatch = groupMatch; } + + public S3Args createS3Args(Frame frame) { + return new S3Args(generic, clazz, targetFunctionName, frame.materialize(), null, null); + } } public static S3FunctionLookupNode create(boolean throwsError, boolean nextMethod) { - return new UseMethodFunctionLookupUninitializedNode(throwsError, nextMethod); + return new UseMethodFunctionLookupUninitializedNode(throwsError, nextMethod, 0); } public static S3FunctionLookupNode createWithError() { - return new UseMethodFunctionLookupUninitializedNode(true, false); + return new UseMethodFunctionLookupUninitializedNode(true, false, 0); } public static S3FunctionLookupNode createWithException() { - return new UseMethodFunctionLookupUninitializedNode(false, false); + return new UseMethodFunctionLookupUninitializedNode(false, false, 0); } @FunctionalInterface @@ -235,19 +239,21 @@ public abstract class S3FunctionLookupNode extends RBaseNode { @NodeInfo(cost = NodeCost.UNINITIALIZED) private static final class UseMethodFunctionLookupUninitializedNode extends S3FunctionLookupNode { - private int depth; + private final int depth; - UseMethodFunctionLookupUninitializedNode(boolean throwsError, boolean nextMethod) { + UseMethodFunctionLookupUninitializedNode(boolean throwsError, boolean nextMethod, int depth) { super(throwsError, nextMethod); + this.depth = depth; } @Override public Result execute(VirtualFrame frame, String genericName, RStringVector type, String group, MaterializedFrame callerFrame, MaterializedFrame genericDefFrame) { CompilerDirectives.transferToInterpreterAndInvalidate(); - if (++depth > MAX_CACHE_DEPTH) { + if (depth > MAX_CACHE_DEPTH) { return replace(new UseMethodFunctionLookupGenericNode(throwsError, nextMethod)).execute(frame, genericName, type, group, callerFrame, genericDefFrame); } else { - UseMethodFunctionLookupCachedNode cachedNode = replace(specialize(frame, genericName, type, group, callerFrame, genericDefFrame, this)); + UseMethodFunctionLookupCachedNode cachedNode = replace( + specialize(frame, genericName, type, group, callerFrame, genericDefFrame, new UseMethodFunctionLookupUninitializedNode(throwsError, nextMethod, depth + 1))); return cachedNode.execute(frame, genericName, type, group, callerFrame, genericDefFrame); } } @@ -329,7 +335,7 @@ public abstract class S3FunctionLookupNode extends RBaseNode { } throw RError.error(this, RError.Message.UNKNOWN_FUNCTION_USE_METHOD, genericName, type); } else { - throw S3FunctionLookupNode.NoGenericMethodException.instance; + return null; } } while (true); CompilerDirectives.transferToInterpreterAndInvalidate(); @@ -431,19 +437,10 @@ public abstract class S3FunctionLookupNode extends RBaseNode { } throw RError.error(this, RError.Message.UNKNOWN_FUNCTION_USE_METHOD, genericName, RRuntime.toString(type)); } else { - throw S3FunctionLookupNode.NoGenericMethodException.instance; + return null; } } return result; } } - - @SuppressWarnings("serial") - public static final class NoGenericMethodException extends ControlFlowException { - private static final NoGenericMethodException instance = new NoGenericMethodException(); - - private NoGenericMethodException() { - // singleton - } - } } diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/UseMethodInternalNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/UseMethodInternalNode.java deleted file mode 100644 index 739931976e..0000000000 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/UseMethodInternalNode.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * This material is distributed under the GNU General Public License - * Version 2. You may review the terms of this license at - * http://www.gnu.org/licenses/gpl-2.0.html - * - * Copyright (c) 2014, Purdue University - * Copyright (c) 2014, 2016, Oracle and/or its affiliates - * - * All rights reserved. - */ -package com.oracle.truffle.r.nodes.function; - -import com.oracle.truffle.api.CompilerDirectives; -import com.oracle.truffle.api.frame.VirtualFrame; -import com.oracle.truffle.r.nodes.function.S3FunctionLookupNode.Result; -import com.oracle.truffle.r.runtime.ArgumentsSignature; -import com.oracle.truffle.r.runtime.FastROptions; -import com.oracle.truffle.r.runtime.RArguments.S3Args; -import com.oracle.truffle.r.runtime.RInternalError; -import com.oracle.truffle.r.runtime.data.RStringVector; -import com.oracle.truffle.r.runtime.nodes.RNode; - -public final class UseMethodInternalNode extends RNode { - - @Child private ClassHierarchyNode classHierarchyNode = ClassHierarchyNodeGen.create(true, true); - @Child private S3FunctionLookupNode lookup = S3FunctionLookupNode.create(false, false); - @Child private CallMatcherNode callMatcher = CallMatcherNode.create(false); - @Child private PreProcessArgumentsNode argPreProcess; - - private final String generic; - private final ArgumentsSignature signature; - private final boolean wrap; - - public UseMethodInternalNode(String generic, ArgumentsSignature signature, boolean wrap) { - this.generic = generic; - this.signature = signature; - this.wrap = wrap && FastROptions.InvisibleArgs.getBooleanValue(); - } - - public Object execute(VirtualFrame frame, RStringVector type, Object[] arguments) { - Result lookupResult = lookup.execute(frame, generic, type, null, frame.materialize(), null); - if (wrap) { - assert arguments != null; - if (argPreProcess == null || argPreProcess.getLength() != arguments.length) { - CompilerDirectives.transferToInterpreterAndInvalidate(); - argPreProcess = insert(PreProcessArgumentsNode.create(arguments.length)); - } - argPreProcess.execute(frame, arguments); - } - S3Args s3Args = new S3Args(generic, lookupResult.clazz, lookupResult.targetFunctionName, frame.materialize(), null, null); - return callMatcher.execute(frame, signature, arguments, lookupResult.function, lookupResult.targetFunctionName, s3Args); - } - - @Override - public Object execute(VirtualFrame frame) { - throw RInternalError.shouldNotReachHere(); - } -} -- GitLab