Native Language Server integration with PM #11880

hubertp · 2024-12-16T15:43:45Z

Pull Request Description

The first attempt at

Building enso native-image that includes full Language Server
Integrating the executable as an experimental feature during project startup

The change (for now) assumes that enso executable appears in Enso's default engines directory.
To build and run the new integration one has to

engine-runner/buildNativeImage
Run PM

The Runner will automatically detect the presence of a native image and attempt to run LS in AOT mode.

This change also adds a copy of some of logback's code (a simple socket server) as it was impossible to debug serialization bugs without some additional logging. Potentially to be removed in the final PR.

This is by no means a final change in this area. We need to make it possible to include all other Standard libs, at least. But it's a step forward that allows devs to experiment.

Important Notes

There is a change in JsonConnectionController which only sets the controller as initialized when all resources are set up. Previously, it seems, the asynchronous setup could lead to some race conditions.

Exchange between LS and GUI (before):

(after)

As one can see, speedup in startup is rather massive.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.

The first attempt at a) building `enso` native-image that includes full Language Server b) integrating the executable as an experimental feature during project startup The change (for now) assumes that `enso` executable appears in Enso's default `engines` directory. To build and run the new integration one has to a) `engine-runner/buildNativeImage` b) run PM with `--native-language-server` This change also adds a copy of some of logback's code (`SocketAppender` or a simple socket server`) as it was impossible to debug serialization bugs without some additional logging.

Otherwise Akka and other code is simply dead-code eliminated in native-image.

When message transport is missing, context should be properly built, even in AOT mode.

Problems revealed after a clean build

JaroslavTulach

I am glad to see language server startup improvement
that's what native image technology is good for and I am glad to see it delivers on its promise
for a while I was confused by the dependency changes
looks like it is mostly to access TruffleOptions.AOT from the non-Truffle part of our codebase
please keep Truffle API out of the dependencies of launcher, boot, etc. projects
use Boolean.getBoolean("com.oracle.graalvm.isaot") directly
I suggest to avoid introduction of --native-... CLI options
- we may not need them at all at this stage
- just prefer bin/enso or bin\enso.exe native binary when it exists
- there already is well functioning --jvm option
- we may want to expose it from project manager CLI, but
- it is already accessible via ENSO_OPTS and that may be enough for now
I suggest to separate separate *-config.json and put them into language-server source tree
I am glad to see the Launching Enso programs instantly #10121 work moving forward

build.sbt

JaroslavTulach · 2025-01-03T06:03:42Z

engine/common/src/main/java/org/enso/common/ContextFactory.java

@@ -228,6 +232,19 @@ public Context build() {
          .allowCreateThread(true);
    }

+    if (engineOptions != null) {
+      // In AOT mode one must not use a shared engine; the latter causes issues when initializing
+      // message transport - it is set to `null`.


Interesting. I don't see a reason wny transport shouldn't work. Maybe it is a bug. Do we have a (small) reproducer that we could report to GraalVM guys and find out what they think?

...er/src/main/java/org/enso/interpreter/instrument/runtime/server/RuntimeServerInstrument.java

JaroslavTulach · 2025-01-03T07:03:30Z

...er/src/main/java/org/enso/interpreter/instrument/runtime/server/RuntimeServerInstrument.java

@@ -108,7 +108,7 @@ public void onContextClosed(TruffleContext context) {}
  @Override
  protected void onCreate(Env env) {
    this.env = env;
-    env.registerService(this);
+
    if (TruffleOptions.AOT) {


The usage of TruffleOptions.AOT is correct here as this is a Truffle instrument and it needs dependency on Truffle API.

lib/scala/project-manager/src/main/scala/org/enso/projectmanager/boot/Cli.scala

...version-manager/src/main/scala/org/enso/runtimeversionmanager/runner/NativeJavaCommand.scala

...la/runtime-version-manager/src/main/scala/org/enso/runtimeversionmanager/runner/Runner.scala

Despite being advertised as such, native image doesn't set `com.oracle.graalvm.isaot` and it has to be provided separately if we don't want to use `TruffleOptions`.

...r/src/main/resources/META-INF/native-image/org/enso/languageserver/serialization-config.json

Context creation for AOT needs to define a separate Engine configuration rather than using a shared engine to be able to use message transport.

JaroslavTulach · 2025-01-07T08:58:42Z

...version-manager/src/main/scala/org/enso/runtimeversionmanager/runner/NativeExecCommand.scala

+    engine: Engine,
+    jvmSettings: JVMSettings
+  ): Seq[String] =
+    Seq("-Dcom.oracle.graalvm.isaot=true")


Interesting. Why do we need to provide this argument?

I would expect it is only useful at image built time

or that it is provided by default

btw. Native Image API offers some checks like this too

E.g. if we can access org.graalvm.nativeimage.ImageInfo then we can query "properly" and even distinguish between build time and runtime time.

The property is completely missing during runtime, hence not allowing me to provide different Context configuration.

JaroslavTulach · 2025-01-07T09:15:40Z

The description should be updated as

engine-runner/buildNativeImage
Run PM with --native-language-server

may no longer reflect the reality after 3ce2577

JaroslavTulach · 2025-01-08T06:22:06Z

Congratulation on merging and making another step towards using native binaries as GUI backend. Notes:

generating enso now takes 5m 48s on my Ubuntu. Used to be 3m.
the enso executable size is now 350MB. Used to be ~200MB.

This is the top ten of included packages:

Top 10 origins of code area:
  21.50MB js-language-24.0.0.jar
  15.69MB java.base
   6.75MB org.graalvm.truffle
   5.48MB jdk.internal.vm.compiler
   4.81MB java.xml
   4.15MB akka-http-core_2.13-10.2.10.jar
   4.14MB poi-5.2.3.jar
   3.87MB icu4j-24.0.0.jar
   3.34MB org.enso.languageserver.protocol.json

it is surprising to see JavaScript back!
it was removed in Avoid JavaScript and Python in Enso native launcher #11279
according to Launching Enso programs instantly #10121 we don't want/need Python neither JavaScript in the image yet
removing Graal.js would speed build times
removing Graal.js would lower the size of the enso executable significantly
reported as Remove JavaScript from bin/enso executable #12018

Previosly we were wrongly relying on the presence of the file. That way, a bash script meant that NI integration was wrongly used. This change uses Tika, but it has already been present in other subprojects so no additional dependency shall be added. Follow up on #11880.

hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Dec 16, 2024

hubertp added 13 commits December 16, 2024 17:28

nit

61ce655

Add missing reflect config

c4d6873

Otherwise Akka and other code is simply dead-code eliminated in native-image.

Ensure proper Context construction

def1ea7

When message transport is missing, context should be properly built, even in AOT mode.

Add missing Akka entries in reflect-config

339493c

nit

b6015e7

Merge branch 'develop' into wip/hubert/11721-language-server-native

cf8a3ed

Partial revert

96ae8c8

Fix service registration

8883d5c

Problems revealed after a clean build

Merge branch 'develop' into wip/hubert/11721-language-server-native

60f8ffe

Fix Base_Tests in native-image

28650dd

Fix compilation issues

cd0a8d8

Drop Truffle dependency

ebaf50c

fmt

08e1e4f

hubertp marked this pull request as ready for review January 2, 2025 09:18

hubertp requested review from 4e6, JaroslavTulach and Akirathan as code owners January 2, 2025 09:18

Merge branch 'develop' into wip/hubert/11721-language-server-native

5547b49

JaroslavTulach approved these changes Jan 3, 2025

View reviewed changes

hubertp added 2 commits January 3, 2025 15:35

Drop TruffleOptions dependency

1588cec

Despite being advertised as such, native image doesn't set `com.oracle.graalvm.isaot` and it has to be provided separately if we don't want to use `TruffleOptions`.

Move NI config to LS resources

55f6623

JaroslavTulach reviewed Jan 3, 2025

View reviewed changes

...r/src/main/resources/META-INF/native-image/org/enso/languageserver/serialization-config.json Show resolved Hide resolved

enso-bot bot mentioned this pull request Jan 4, 2025

Text.find slows down with the number of invocations #11859

Closed

hubertp added 3 commits January 6, 2025 11:05

Remove automatically added module dependency

ea2f214

Move AOT Context-creation logic to factory

6e8c05b

Context creation for AOT needs to define a separate Engine configuration rather than using a shared engine to be able to use message transport.

PR review

3ce2577

JaroslavTulach reviewed Jan 7, 2025

View reviewed changes

hubertp added 2 commits January 7, 2025 10:25

Merge branch 'develop' into wip/hubert/11721-language-server-native

e82bb61

Merge branch 'develop' into wip/hubert/11721-language-server-native

cadf616

hubertp added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Jan 7, 2025

hubertp merged commit 7f7d0d1 into develop Jan 7, 2025
49 checks passed

hubertp deleted the wip/hubert/11721-language-server-native branch January 7, 2025 17:18

JaroslavTulach mentioned this pull request Jan 8, 2025

Remove JavaScript from bin/enso executable #12018

Open

hubertp mentioned this pull request Jan 10, 2025

Better detection for Enso's NI when launching Language Server #12034

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native Language Server integration with PM #11880

Native Language Server integration with PM #11880

hubertp commented Dec 16, 2024 •

edited

Loading

JaroslavTulach left a comment

JaroslavTulach Jan 3, 2025

JaroslavTulach Jan 3, 2025

JaroslavTulach Jan 7, 2025

hubertp Jan 7, 2025

JaroslavTulach commented Jan 7, 2025

JaroslavTulach commented Jan 8, 2025 •

edited

Loading

Native Language Server integration with PM #11880

Native Language Server integration with PM #11880

Conversation

hubertp commented Dec 16, 2024 • edited Loading

Pull Request Description

Important Notes

Checklist

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach Jan 3, 2025

Choose a reason for hiding this comment

JaroslavTulach Jan 3, 2025

Choose a reason for hiding this comment

JaroslavTulach Jan 7, 2025

Choose a reason for hiding this comment

hubertp Jan 7, 2025

Choose a reason for hiding this comment

JaroslavTulach commented Jan 7, 2025

JaroslavTulach commented Jan 8, 2025 • edited Loading

hubertp commented Dec 16, 2024 •

edited

Loading

JaroslavTulach commented Jan 8, 2025 •

edited

Loading