From 8fcaf6031527cb4424327bec7c62a96a1505176c Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Wed, 1 Mar 2023 10:51:37 +0100 Subject: [PATCH 1/6] Switch to Truffle PE mode before calling into Enso --- .../interpreter/service/ExecutionService.java | 88 +++++++++++++++---- 1 file changed, 71 insertions(+), 17 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java b/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java index 026b6ec6f7d7..92ca3f8834cc 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java @@ -3,15 +3,15 @@ import com.oracle.truffle.api.CallTarget; import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.TruffleLogger; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.EventBinding; -import com.oracle.truffle.api.instrumentation.ExecutionEventListener; -import com.oracle.truffle.api.instrumentation.ExecutionEventNode; import com.oracle.truffle.api.instrumentation.ExecutionEventNodeFactory; import com.oracle.truffle.api.interop.ArityException; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.UnknownIdentifierException; import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.interop.UnsupportedTypeException; +import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.api.source.SourceSection; import org.enso.compiler.context.SimpleUpdate; import org.enso.interpreter.instrument.Endpoint; @@ -57,9 +57,10 @@ public class ExecutionService { private final EnsoContext context; private final Optional idExecutionInstrument; private final NotificationHandler.Forwarder notificationForwarder; - private final InteropLibrary interopLibrary = InteropLibrary.getFactory().getUncached(); private final TruffleLogger logger = TruffleLogger.getLogger(LanguageInfo.ID); private final ConnectedLockManager connectedLockManager; + private final CallTarget execute = new ExecuteCallRootNode().getCallTarget(); + private final CallTarget invoke = new InvokeMemberRootNode().getCallTarget(); private final Timer timer; @@ -175,7 +176,7 @@ public void execute( onExceptionalCallback)); Object p = context.getThreadManager().enter(); try { - interopLibrary.execute(call); + execute.call(call); } finally { context.getThreadManager().leave(p); eventNodeFactory.ifPresent(EventBinding::dispose); @@ -233,7 +234,7 @@ public void execute( * Evaluates an expression in the scope of the provided module. * * @param module the module providing a scope for the expression - * @param expression the expression to evluated + * @param expression the expression to evaluated * @return a result of evaluation */ public Object evaluateExpression(Module module, String expression) @@ -241,7 +242,7 @@ public Object evaluateExpression(Module module, String expression) UnsupportedTypeException { Object p = context.getThreadManager().enter(); try { - return interopLibrary.invokeMember(module, MethodNames.Module.EVAL_EXPRESSION, expression); + return invoke.call(module, expression); } finally { context.getThreadManager().leave(p); } @@ -255,7 +256,8 @@ public Object evaluateExpression(Module module, String expression) */ public String toDisplayString(Object receiver) { try { - return interopLibrary.asString(interopLibrary.toDisplayString(receiver)); + var iop = InteropLibrary.getUncached(); + return iop.asString(iop.toDisplayString(receiver)); } catch (UnsupportedMessageException ignored) { CompilerDirectives.shouldNotReachHere("Message support already checked."); } @@ -273,7 +275,7 @@ public Object callFunction(Object fn, Object argument) throws UnsupportedTypeException, ArityException, UnsupportedMessageException { Object p = context.getThreadManager().enter(); try { - return interopLibrary.execute(fn, argument); + return execute.call(fn, new Object[] { argument }); } finally { context.getThreadManager().leave(p); } @@ -321,7 +323,7 @@ public Object callFunctionWithInstrument( onExceptionalCallback)); Object p = context.getThreadManager().enter(); try { - return interopLibrary.execute(function, arguments); + return execute.call(function, arguments); } finally { context.getThreadManager().leave(p); eventNodeFactory.ifPresent(EventBinding::dispose); @@ -392,9 +394,10 @@ public void modifyModuleSources( * @return the associated language, or {@code null} if it doesn't exist */ public String getLanguage(Object o) { - if (interopLibrary.hasSourceLocation(o)) { + var iop = InteropLibrary.getUncached(o); + if (iop.hasSourceLocation(o)) { try { - var sourceSection = interopLibrary.getSourceLocation(o); + var sourceSection = iop.getSourceLocation(o); var source = sourceSection.getSource(); if (source != null) { return source.getLanguage(); @@ -413,9 +416,10 @@ public String getLanguage(Object o) { * @return the associated source section, or {@code null} if it doesn't exist */ public SourceSection getSourceLocation(Object o) { - if (interopLibrary.hasSourceLocation(o)) { + var iop = InteropLibrary.getUncached(o); + if (iop.hasSourceLocation(o)) { try { - return interopLibrary.getSourceLocation(o); + return iop.getSourceLocation(o); } catch (UnsupportedMessageException ignored) { CompilerDirectives.shouldNotReachHere("Message support already checked."); } @@ -430,17 +434,18 @@ public SourceSection getSourceLocation(Object o) { * @return a human-readable version of its contents. */ public String getExceptionMessage(PanicException panic) { - Object p = context.getThreadManager().enter(); + var iop = InteropLibrary.getUncached(); + var p = context.getThreadManager().enter(); try { - return interopLibrary.asString( - interopLibrary.invokeMember(panic.getPayload(), "to_display_text")); + return iop.asString( + iop.invokeMember(panic.getPayload(), "to_display_text")); } catch (UnsupportedMessageException | ArityException | UnknownIdentifierException | UnsupportedTypeException e) { return TypeToDisplayTextNodeGen.getUncached().execute(panic.getPayload()); } catch (Throwable e) { - if (interopLibrary.isException(e)) { + if (iop.isException(e)) { return TypeToDisplayTextNodeGen.getUncached().execute(panic.getPayload()); } else { throw e; @@ -449,4 +454,53 @@ public String getExceptionMessage(PanicException panic) { context.getThreadManager().leave(p); } } + + @SuppressWarnings("unchecked") + private static E raise(Class type, Exception ex) throws E { + throw (E) ex; + } + + private static final class ExecuteCallRootNode extends RootNode { + @Child + private InteropLibrary iop = InteropLibrary.getFactory().createDispatched(5); + + public ExecuteCallRootNode() { + super(null); + } + + @Override + public Object execute(VirtualFrame frame) { + try { + var self = frame.getArguments()[0]; + if (self instanceof FunctionCallInstrumentationNode.FunctionCall call) { + return iop.execute(call); + } else { + var args = (Object[]) frame.getArguments()[1]; + return iop.execute(self, args); + } + } catch (UnsupportedTypeException | ArityException | UnsupportedMessageException ex) { + throw raise(RuntimeException.class, ex); + } + } + } + private static final class InvokeMemberRootNode extends RootNode { + @Child + private InteropLibrary iop = InteropLibrary.getFactory().createDispatched(5); + + public InvokeMemberRootNode() { + super(null); + } + + @Override + public Object execute(VirtualFrame frame) { + var module = frame.getArguments()[0]; + var expression = frame.getArguments()[1]; + try { + return iop.invokeMember(module, MethodNames.Module.EVAL_EXPRESSION, expression); + } catch (UnknownIdentifierException | UnsupportedTypeException | ArityException | UnsupportedMessageException ex) { + throw raise(RuntimeException.class, ex); + } + } + + } } From cddd152140634e70abd25a8192df68799658fcde Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Wed, 1 Mar 2023 14:24:21 +0100 Subject: [PATCH 2/6] Don't include internal nodes in the stacktrace --- .../enso/interpreter/instrument/execution/ErrorResolver.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/ErrorResolver.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/ErrorResolver.scala index 0d3968ed01f8..86357b3d29aa 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/ErrorResolver.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/ErrorResolver.scala @@ -40,6 +40,8 @@ object ErrorResolver { x.getEncapsulatingSourceSection match { case null if x.getRootNode == null => None + case null if x.getRootNode.isInternal => + None case null => Some(Api.StackTraceElement(x.getRootNode.getName, None, None, None)) case section => From 8f0dcb7479c41674ec93a5b865dc1b7ed56f4fe7 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Wed, 1 Mar 2023 17:30:27 +0100 Subject: [PATCH 3/6] Don't include internal ExecutionService nodes in stacktraces --- .../generic/GetExecutableNameNode.java | 7 ++- .../builtin/runtime/GetStackTraceNode.java | 23 ++++++-- .../enso/interpreter/runtime/data/Array.java | 4 +- .../interpreter/service/ExecutionService.java | 53 +++++++++++++------ 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java index 0a91545726b7..d519b4856483 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java @@ -1,5 +1,6 @@ package org.enso.interpreter.node.expression.builtin.interop.generic; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.nodes.Node; @@ -25,7 +26,11 @@ public class GetExecutableNameNode extends Node { Text execute(Object function) { try { - return Text.create(stringsLibrary.asString(functionsLibrary.getExecutableName(function))); + var name = functionsLibrary.getExecutableName(function); + if (name == null || !stringsLibrary.isString(name)) { + throw CompilerDirectives.shouldNotReachHere("name: " + name + " for " + function); + } + return Text.create(stringsLibrary.asString(name)); } catch (UnsupportedMessageException e) { err.enter(); Builtins builtins = EnsoContext.get(this).getBuiltins(); diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/GetStackTraceNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/GetStackTraceNode.java index 1f841aea0052..e9b9ed0d44d3 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/GetStackTraceNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/GetStackTraceNode.java @@ -1,5 +1,6 @@ package org.enso.interpreter.node.expression.builtin.runtime; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.TruffleStackTrace; import com.oracle.truffle.api.nodes.Node; import org.enso.interpreter.dsl.BuiltinMethod; @@ -18,14 +19,28 @@ Array execute() { return stackTraceToArray(exception); } + @CompilerDirectives.TruffleBoundary public static Array stackTraceToArray(Throwable exception) { var elements = TruffleStackTrace.getStackTrace(exception); - if (elements == null) return new Array(); - var ret = Array.allocate(elements.size()); + if (elements == null) { + return Array.empty(); + } + int count = 0; for (int i = 0; i < elements.size(); i++) { var element = elements.get(i); - ret.getItems()[i] = element.getGuestObject(); + if (element.getTarget().getRootNode().isInternal()) { + continue; + } + count++; + } + var arr = new Object[count]; + for (int i = 0, at = 0; i < elements.size(); i++) { + var element = elements.get(i); + if (element.getTarget().getRootNode().isInternal()) { + continue; + } + arr[at++] = element.getGuestObject(); } - return ret; + return new Array(arr); } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Array.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Array.java index f5e8adeaf430..cc4271e0be0f 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Array.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Array.java @@ -118,12 +118,12 @@ public Object readArrayElement( } public long length() { - return this.getItems().length; + return items.length; } /** @return an empty array */ @Builtin.Method(description = "Creates an empty Array", autoRegister = false) - public static Object empty() { + public static Array empty() { return allocate(0); } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java b/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java index 92ca3f8834cc..7b9cd9608c2f 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java @@ -11,6 +11,7 @@ import com.oracle.truffle.api.interop.UnknownIdentifierException; import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.interop.UnsupportedTypeException; +import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.api.source.SourceSection; import org.enso.compiler.context.SimpleUpdate; @@ -59,8 +60,9 @@ public class ExecutionService { private final NotificationHandler.Forwarder notificationForwarder; private final TruffleLogger logger = TruffleLogger.getLogger(LanguageInfo.ID); private final ConnectedLockManager connectedLockManager; - private final CallTarget execute = new ExecuteCallRootNode().getCallTarget(); - private final CallTarget invoke = new InvokeMemberRootNode().getCallTarget(); + private final ExecuteRootNode execute = new ExecuteRootNode(); + private final CallRootNode call = new CallRootNode(); + private final InvokeMemberRootNode invoke = new InvokeMemberRootNode(); private final Timer timer; @@ -176,7 +178,7 @@ public void execute( onExceptionalCallback)); Object p = context.getThreadManager().enter(); try { - execute.call(call); + execute.getCallTarget().call(call); } finally { context.getThreadManager().leave(p); eventNodeFactory.ifPresent(EventBinding::dispose); @@ -242,7 +244,7 @@ public Object evaluateExpression(Module module, String expression) UnsupportedTypeException { Object p = context.getThreadManager().enter(); try { - return invoke.call(module, expression); + return invoke.getCallTarget().call(module, expression); } finally { context.getThreadManager().leave(p); } @@ -275,7 +277,7 @@ public Object callFunction(Object fn, Object argument) throws UnsupportedTypeException, ArityException, UnsupportedMessageException { Object p = context.getThreadManager().enter(); try { - return execute.call(fn, new Object[] { argument }); + return call.getCallTarget().call(fn, new Object[] { argument }); } finally { context.getThreadManager().leave(p); } @@ -323,7 +325,7 @@ public Object callFunctionWithInstrument( onExceptionalCallback)); Object p = context.getThreadManager().enter(); try { - return execute.call(function, arguments); + return call.getCallTarget().call(function, arguments); } finally { context.getThreadManager().leave(p); eventNodeFactory.ifPresent(EventBinding::dispose); @@ -460,34 +462,52 @@ private static E raise(Class type, Exception ex) throws throw (E) ex; } - private static final class ExecuteCallRootNode extends RootNode { - @Child + private static final class ExecuteRootNode extends RootNode { + @Node.Child private InteropLibrary iop = InteropLibrary.getFactory().createDispatched(5); - public ExecuteCallRootNode() { + ExecuteRootNode() { super(null); } @Override public Object execute(VirtualFrame frame) { try { - var self = frame.getArguments()[0]; - if (self instanceof FunctionCallInstrumentationNode.FunctionCall call) { + if (frame.getArguments()[0] instanceof FunctionCallInstrumentationNode.FunctionCall call) { return iop.execute(call); - } else { - var args = (Object[]) frame.getArguments()[1]; - return iop.execute(self, args); } + throw ArityException.create(1, 1, frame.getArguments().length); + } catch (UnsupportedTypeException | ArityException | UnsupportedMessageException ex) { + throw raise(RuntimeException.class, ex); + } + } + } + + private static final class CallRootNode extends RootNode { + @Node.Child + private InteropLibrary iop = InteropLibrary.getFactory().createDispatched(5); + + CallRootNode() { + super(null); + } + + @Override + public Object execute(VirtualFrame frame) { + try { + var self = frame.getArguments()[0]; + var args = (Object[]) frame.getArguments()[1]; + return iop.execute(self, args); } catch (UnsupportedTypeException | ArityException | UnsupportedMessageException ex) { throw raise(RuntimeException.class, ex); } } } + private static final class InvokeMemberRootNode extends RootNode { - @Child + @Node.Child private InteropLibrary iop = InteropLibrary.getFactory().createDispatched(5); - public InvokeMemberRootNode() { + InvokeMemberRootNode() { super(null); } @@ -501,6 +521,5 @@ public Object execute(VirtualFrame frame) { throw raise(RuntimeException.class, ex); } } - } } From 9f64dccd85cadb8e9a705c22c7dc6bde68255483 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Wed, 1 Mar 2023 17:46:58 +0100 Subject: [PATCH 4/6] Compose the string behind truffle boundary --- .../builtin/interop/generic/GetExecutableNameNode.java | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java index d519b4856483..506200f63d7c 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/generic/GetExecutableNameNode.java @@ -28,6 +28,7 @@ Text execute(Object function) { try { var name = functionsLibrary.getExecutableName(function); if (name == null || !stringsLibrary.isString(name)) { + CompilerDirectives.transferToInterpreter(); throw CompilerDirectives.shouldNotReachHere("name: " + name + " for " + function); } return Text.create(stringsLibrary.asString(name)); From c7d1438a70807098d00008d71f72a495f1cdba94 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Wed, 1 Mar 2023 19:14:18 +0100 Subject: [PATCH 5/6] Internal frames are now hidden --- test/Tests/src/Semantic/Error_Spec.enso | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Tests/src/Semantic/Error_Spec.enso b/test/Tests/src/Semantic/Error_Spec.enso index c6bf321dc315..61df78283a47 100644 --- a/test/Tests/src/Semantic/Error_Spec.enso +++ b/test/Tests/src/Semantic/Error_Spec.enso @@ -129,11 +129,11 @@ spec = Test.specify "should provide access to Java stack traces" <| stack_1 = Panic.recover Any (do_a_parse "foo") . stack_trace - stack_1.at 2 . name . should_equal "Error_Spec.do_a_parse" + stack_1.at 0 . name . should_equal "Error_Spec.do_a_parse" stack_2 = Panic.catch Any (do_a_parse "foo") caught_panic-> caught_panic.stack_trace - stack_2.at 2 . name . should_equal "Error_Spec.do_a_parse" + stack_2.at 0 . name . should_equal "Error_Spec.do_a_parse" Test.specify "should be able to be rethrown without changing the stack trace" <| caught_panic = Panic.catch Any throw_a_bar_panicking x->x @@ -189,7 +189,7 @@ spec = Panic.throw caught_panic.payload message_1.get . should_equal 'For input string: "foo"' caught_1.catch . should_be_a JException - caught_1.stack_trace.at 2 . name . should_equal "Error_Spec.do_a_parse" + caught_1.stack_trace.at 0 . name . should_equal "Error_Spec.do_a_parse" message_2 = Ref.new "" caught_2 = Panic.recover Any <| From ad9d69cc279e33912f5ca344e148833362b3304e Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 2 Mar 2023 16:20:38 +0100 Subject: [PATCH 6/6] Value is no longer on the stack, as it is internal frame --- distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso index aeefa4fa3b66..86f726cc12af 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso @@ -27,8 +27,7 @@ get_stack_trace = prim_stack = primitive_get_stack_trace stack_with_prims = Vector.from_polyglot_array prim_stack # (First 2) drops the `Runtime.primitive_get_stack_trace` frame and this one - # (Last 1) drops the `org.graalvm.polyglot.Value.execute` frame - stack = stack_with_prims.drop (First 2) . drop (Last 1) + stack = stack_with_prims.drop (First 2) stack.map wrap_primitive_stack_trace_element ## ADVANCED