Ensure docker is properly notified of NI #12443
Open
+7
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Looks like `com.oracle.graalvm.isaot` is not set properly when running in native image. Also, we must provide `--jvm` when we execute in JVM mode (the default mode for now). This allows LS to start without any issues when invoked via docker.
hubertp
requested review from
4e6,
JaroslavTulach,
radeusgd and
Akirathan
as code owners
hubertp
added
the
CI: No changelog needed
There was a problem hiding this comment.
Seeing the change, I guess we are not using com.oracle.graalvm.isaot properly elsewhere. There are other alternatives we should probably be using like ImageInfo.inImageRuntimeCode.
|
|
||
| /opt/enso/bin/enso $PROFILING_OPTIONS --log-level "$LOG_LEVEL" --rpc-port $RPC_PORT --data-port $DATA_PORT --root-id "$LS_ROOT_ID" --interface "$INTERFACE" "$@" | ||
| /opt/enso/bin/enso $PROFILING_OPTIONS $NATIVE_PROP --log-level "$LOG_LEVEL" --rpc-port $RPC_PORT --data-port $DATA_PORT --root-id "$LS_ROOT_ID" --interface "$INTERFACE" "$@" |
There was a problem hiding this comment.
- This is strange: Where do we process
-D? - I know we do process
--vm.D, but I never heard of-D: - Add --vm.D cmdline option #10798
| @@ -17,5 +17,9 @@ PROFILING_OPTIONS="" | |||
| if [ "$PROFILING_FILENAME" != "" ] && [ "$PROFILING_TIME" != "" ]; then | |||
| PROFILING_OPTIONS="--profiling-path /opt/enso/profiling/$PROFILING_FILENAME --profiling-time=$PROFILING_TIME" | |||
| fi | |||
| NATIVE_PROP="--jvm" | |||
| if [ "$ENSO_LAUNCHER" != "" ] && [ "$ENSO_LAUNCHER" == "native" ]; then | |||
| NATIVE_PROP="-Dcom.oracle.graalvm.isaot=true" | |||
There was a problem hiding this comment.
- manually setting this property is unfortunate
- it is probably result of this request of mine
- where I wanted to drop dependency on Truffle API
- we have already faced that issue once here
- I assume we are doing something wrong (per my guidance)
There was a problem hiding this comment.
Yes, the primary reason for using the property is unwillingness to add dependencies.
There was a problem hiding this comment.
unwillingness to add dependencies
- the dependencies are already included
- Avoid usage of isaot property in favor of ImageInfo #12445 doesn't add any new dependency
- we seem to be using
com.oracle.graalvm.isaotimproperly - better to use
ImageInfoas Avoid usage of isaot property in favor of ImageInfo #12445 demonstrates
This was referenced Mar 8, 2025
|
JaroslavTulach
requested changes
Mar 9, 2025
There was a problem hiding this comment.
- assuming Avoid usage of isaot property in favor of ImageInfo #12445 renders this PR unnecessary
- I prefer to integrate just Avoid usage of isaot property in favor of ImageInfo #12445
- and close this PR unmerged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Looks like
com.oracle.graalvm.isaotis not set properly when running in native image. Also, we must provide--jvmwhen we execute in JVM mode (the default mode for now).This allows LS to start without any issues when invoked via docker.
Important Notes
I'd like to take a closer look at how we build the executable provided to docker and maybe avoid this band-aid but for now this unblocks staging. Tested locally by sending appropriate message to server's websocket.
Previously:
Now:
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.