Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to Truffle PE mode before calling into Enso #5783

Merged
merged 10 commits into from
Mar 3, 2023
Merged

Switch to Truffle PE mode before calling into Enso #5783

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8fcaf60
Switch to Truffle PE mode before calling into Enso
JaroslavTulach Mar 1, 2023
cddd152
Don't include internal nodes in the stacktrace
JaroslavTulach Mar 1, 2023
8f0dcb7
Don't include internal ExecutionService nodes in stacktraces
JaroslavTulach Mar 1, 2023
9f64dcc
Compose the string behind truffle boundary
JaroslavTulach Mar 1, 2023
c7d1438
Internal frames are now hidden
JaroslavTulach Mar 1, 2023
ad9d69c
Value<Function> is no longer on the stack, as it is internal frame
JaroslavTulach Mar 2, 2023
9910fbc
Merge branch 'develop' into wip/jtulach/SwitchToPe_5782
mergify[bot] Mar 2, 2023
c3dc114
Merge branch 'develop' into wip/jtulach/SwitchToPe_5782
mergify[bot] Mar 2, 2023
bc56681
Merge branch 'develop' into wip/jtulach/SwitchToPe_5782
mergify[bot] Mar 2, 2023
160f324
Merge branch 'develop' into wip/jtulach/SwitchToPe_5782
mergify[bot] Mar 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 71 additions & 17 deletions engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -57,9 +57,10 @@ public class ExecutionService {
private final EnsoContext context;
private final Optional<IdExecutionService> 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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -233,15 +234,15 @@ 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)
throws UnsupportedMessageException, ArityException, UnknownIdentifierException,
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);
}
Expand All @@ -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.");
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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.");
}
Expand All @@ -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;
Expand All @@ -449,4 +454,53 @@ public String getExceptionMessage(PanicException panic) {
context.getThreadManager().leave(p);
}
}

@SuppressWarnings("unchecked")
private static <E extends Exception> E raise(Class<E> type, Exception ex) throws E {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotta love Java :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this method instead of putting throw ex directly at callsite?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see that for example ArityException or UnsupportedTypeException extend InteropException which apparently does not seem to inherit RuntimeException.

So I guess this is the reason for this magic?

But that seems totally unsafe - since we do an unchecked cast to RuntimeException of something that we know that it doesn't extend RuntimeException.

If the cast is unchecked, I guess it will go through, but it will violate the typesystem rules. That sounds very worrying at first sight.

Can we get a comment (in the code, as part of this function's doc comment) explaining why we are doing it and why is it safe in this context? (If it is safe...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a standard way to propagate checked exception thru Java language. Java language has (too) strict rules about checked exceptions, but there is nothing like that in the JVM (as demonstrated by Scala ignoring checked exceptions altogether). Sometimes it is useful to propagate checked exception even the Java method signatures are against it. This is the case. We have to tunnel the exception thru CallTarget interface now. Rather than wrapping the exception into runtime and unpacking it, I am trying to throw it directly. Btw. this is not the first place where this coding technique is used. There are other raise methods around the codebase already.

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);
}
}

}
}
2 changes: 2 additions & 0 deletions engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/ErrorResolver.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down