-
Notifications
You must be signed in to change notification settings - Fork 323
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.
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
Espresso support with most recent Enso@GraalVM #6966
Conversation
Outdated. Rather read the documentation. Keeping this comment only for historical purposes... To build and test this PR via $ /graalvm-ce-java17-22.3.1/bin/gu install espresso
enso$ sbt --java-home /graalvm-ce-java17-22.3.1/ bootstrap
enso$ sbt --java-home /graalvm-ce-java17-22.3.1/ buildEngineDistribution
enso$ sbt --java-home /graalvm-ce-java17-22.3.1/ engine-runner/buildNativeImage Let's also create a file import Standard.Base.IO
main = IO.println <| "Hello World!" Now we can execute the created bits using different styles... Native Image with Espressoenso$ JAVA_HOME=/graalvm-ce-java17-22.3.1/ ./runner --run h.enso
Hello World! Great, it works. Enso can successfully call into Espresso and invoke JVM with Espressoenso$ JAVA_HOME=/graalvm-ce-java17-22.3.1/
./built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso --run h.enso Useful for debugging and finding out what is going on. Pass in additional JVM without Espressoenso$ ENSO_JAVA=hosted JAVA_HOME=/graalvm-ce-java17-22.3.1/
./built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso --run h.enso Useful to compare the benefits of Espresso over the current (e.g. |
The performance results are interesting. It takes about 1-2s to start Espresso: And there is a huge difference between following programs: main = [ "IO", "Hello World!" ] this one starts almost instantly (as it doesn't need standard libraries, neither Espresso). The version with: import Standard.Base.IO
main = IO.println <| "Hello World!" is slow - it needs to prepare the import Standard.Base.IO
main = [ IO, "Hello World!" ] and this time it is not problem of Espresso, but of preparing the |
ca7d004
to
64029ef
Compare
if (ex.getMessage().contains("No language for id java found.")) { | ||
logger.log(Level.SEVERE, "Environment variable ENSO_JAVA=" + envJava + ", but " + ex.getMessage()); | ||
logger.log(Level.SEVERE, "Use " + System.getProperty("java.home") + "/bin/gu install espresso"); | ||
logger.log(Level.SEVERE, "Continuing in regular Java mode"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hubertp this code produces following errors on the console:
enso$ ENSO_JAVA=espresso ./built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso --run ~/h.enso
[ERROR] [2023-09-16T07:24:36+02:00] [enso.org.enso.interpreter.runtime.EnsoContext] Environment variable ENSO_JAVA=espresso, but No language for id java found. Supported languages are: [python, llvm, js, enso]
[ERROR] [2023-09-16T07:24:36+02:00] [enso.org.enso.interpreter.runtime.EnsoContext] Use /graalvm-community-openjdk-17.0.7+7.1/bin/gu install espresso
[ERROR] [2023-09-16T07:24:36+02:00] [enso.org.enso.interpreter.runtime.EnsoContext] Continuing in regular Java mode
are the messages in the right format? I don't find it particularly human readable...
With f3705ea one can even run the IDE with |
Running test/Test with EspressoWith 513215f and with one more patch (to workaround oracle/graal#7435): enso$ git diff
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/callable/resolver/HostMethodCallNode.java engine/runtime/src/main/java/org/enso/interpreter/node/callable/resolver/HostMethodCallNode.java
index 32de29eb30..5944927e62 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/callable/resolver/HostMethodCallNode.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/callable/resolver/HostMethodCallNode.java
@@ -268,7 +268,7 @@ public abstract class HostMethodCallNode extends Node {
@Shared("hostValueToEnsoNode") @Cached HostValueToEnsoNode hostValueToEnsoNode) {
try {
return hostValueToEnsoNode.execute(instances.instantiate(self, args));
- } catch (UnsupportedMessageException e) {
+ } catch (Error | UnsupportedMessageException e) {
CompilerDirectives.transferToInterpreter();
var ctx = EnsoContext.get(this);
var err = ctx.getBuiltins().error().makeNotInvokable(self); I am able to execute whole test/Tests suite: enso$ .../enso-0.0.0-dev/bin/enso --run test/Tests
1663 tests succeeded.
541 tests failed. Still a bunch of Espresso bugs to fix, but the result isn't completely desperate. |
Jaroslav Tulach reports a new STANDUP for yesterday (2023-09-16): Progress: -
Next Day: Merge Espresso support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume there is no downside on these changes when no in espresso mode.
engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a request to update the SBT build. Otherwise LGTM
engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Show resolved
Hide resolved
} catch (Exception ex) { | ||
if (ex.getMessage().contains("No language for id java found.")) { | ||
logger.log(Level.SEVERE, "Environment variable ENSO_JAVA=" + envJava + ", but " + ex.getMessage()); | ||
logger.log(Level.SEVERE, "Use " + System.getProperty("java.home") + "/bin/gu install espresso"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes we check for JAVA_HOME
and sometimes for java.home
. Sometimes for both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be using System.getProperty("java.home")
as that it guaranteed to be set on OpenJDK. JAVA_HOME
is just a convenience environment variable used by some (usually shell) scripts to locate the java
command.
engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Show resolved
Hide resolved
Jaroslav Tulach reports a new STANDUP for yesterday (2023-09-18): Progress: - running Enso on the latest GraalVM: 2e393dd
Next Day: Polish & merge Espresso support; work on instrumentation |
Jaroslav Tulach reports a new STANDUP for yesterday (2023-09-19): Progress: -
Next Day: Discussions;
|
Pull Request Description
Once upon a time there was #4877 which managed to convince Enso native-image to load standard libraries via Espresso. Since then, however, the codebase changed and the integration with GraalVM 22.3.x and Espresso has to look differently right now. This PR updates the original code to most recent
develop
branch.Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.