Run ydoc-server with GraalVM #9528

4e6 · 2024-03-25T08:28:57Z

Pull Request Description

part of #7954

Important Notes

The workflow is:

$ npm install -- just in case
$ npm --workspace=enso-gui2 run build-ydoc-server-polyglot -- build the ydocServer.js bundle
$ sbt ydoc-server/assembly -- build the ydoc server jar
env POLYGLOT_YDOC_SERVER=true npm --workspace=enso-gui2 run dev -- run the dev server with the polyglot ydoc server. Providing POLYGLOT_YDOC_SERVER_DEBUG=true env variable enables the chrome debugger

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,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

JaroslavTulach · 2024-03-25T09:52:47Z

lib/java/js-websocket/src/main/java/org/enso/polyfill/Main.java

+    }
+
+    public static void main(String[] args) throws Exception {
+        ClasspathResource.createTempFile(WASM_PATH);


According to @kazcw we should just call the Parser.parseInput instead of trying to load Rust part of the parser as WASM.

We can, and it would be more efficient--but doing so would have implications for versioning, a topic which I think in the 13-March meeting we decided to decide later. One possibility discussed was the UI injecting the parser (which would be the WASM core and the JS layer) so that the frontend could remain tightly coupled with it, and then we would use something else as the stable interface (I pictured an IrBuilder API).

If we are definitely OK with requiring the frontend and backend to be versions with the same parser build, we should make the change to using the linked library (and we should add a parser version check to the protocol).

Right now versioning isn't an issue (everything is packaged as a single deliverable, distribution). But should IDE inject its version of the yserver into (completely) different version of an engine, then relying on engine's parser isn't an option. I should collect these constraints somewhere.

Right now versioning isn't an issue (everything is packaged as a single deliverable, distribution).

Versioning isn't an issue in production, but the only reason versioning doesn't currently seem like an issue during development is that the frontend and backend are loosely-coupled in practice, with our current source-code/span-tree interface. When we move the parser into the ydoc-server, it will become essential that the frontend and backend are using the same parser version (including both the WASM and app/gui2/shared).

Versioning isn't an issue in production

..in current production we provide two kinds of deliverables:

standalone packages containing both GUI and the backend

the cloud which also packages GUI and backend of the same release tag like 2024.1.1-nightly.2024.3.26 (right @PabloBuchu?)

With such a setup we may pay no attention to versioning...

frontend and backend are loosely-coupled in practice

... as frontend and backend are build time coupled - e.g. they are build, tested and known to work together. In such a setup it is clear that ...

the frontend and backend are using the same parser version

They do use the same version, as they are built, tested, packaged together.

As such I am suggesting to @4e6 to call the JNI version of the parser. I believe it is a simpler, few lines code change. Running our parser in its WASM version is also possible (there is GraalWasm, but it is more complex to setup and it'd be (slightly) slower, slower to get up to the full speed and more memory hungry because of WASM interpreter metadata overhead. That's why I'd start with integrating the parser via its JNI version. Only then investigate how to integrate the WASM variant.

Please find a link to additional information about incremental compilation plan here.

Versioning isn't an issue in production

..in current production we provide two kinds of deliverables:

* [ standalone packages containing both GUI and the backend](https://github.com/enso-org/enso/releases/tag/2024.1.1-nightly.2024.3.26) * the cloud which also packages GUI and backend of the _same release tag_ like [2024.1.1-nightly.2024.3.26](https://github.com/enso-org/enso/releases/tag/2024.1.1-nightly.2024.3.26) (right @PabloBuchu?)

With such a setup we may pay no attention to versioning...

frontend and backend are loosely-coupled in practice

... as frontend and backend are build time coupled - e.g. they are build, tested and known to work together. In such a setup it is clear that ...

the frontend and backend are using the same parser version

They do use the same version, as they are built, tested, packaged together.

Right... "versioning isn't an issue in production."

However, this change would cause some inconvenience during development. Developers frequently use versions other than the packaged versions enumerated above. It is currently possible for frontend developers to pull changes from develop without rebuilding the backend. I do so frequently, and I expect I am not alone in this. If we make this change, the coupling will be much tighter in practice. Frontend developers (and library developers?) will need to be aware when a commit touches the parser and they need to rebuild the backend. Developers working on the parser will have a much longer code/test cycle. Maybe this is acceptable, but these are costs we should be aware of if we're making this change.

app/gui2/package.json

app/gui2/ydoc-server/server.ts

lib/java/js-websocket/src/main/java/org/enso/polyfill/timers/TimersPolyfill.java

lib/java/js-websocket/src/main/resources/org/enso/polyfill/websocket/websocket-polyfill.js

lib/java/polyglot-ydoc-server/src/test/java/org/enso/polyfill/EncodingPolyfillTest.java

JaroslavTulach · 2024-04-03T03:27:21Z

lib/java/polyglot-ydoc-server/src/main/java/org/enso/polyfill/encoding/EncodingPolyfill.java

+                var data = arguments[2].as(int[].class);
+
+                var charset = encoding == null ? StandardCharsets.UTF_8 : Charset.forName(encoding);
+                // Convert unsigned Uint8Array to byte[]


Try to obtain .as(ByteBuffer.class) - I've heard it should be possible to exchange it from/to Uint8Array.

The ByteBuffer interop currently works only with arrays that are backed by the Java buffers https://github.com/oracle/graaljs/blob/master/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/interop/InteropByteBufferTest.java

We may try using Value.readBuffer method to extract byte array without the conversion

lib/java/polyglot-ydoc-server/src/test/java/org/enso/polyfill/EncodingPolyfillTest.java

lib/java/polyglot-ydoc-server/src/main/java/org/enso/polyfill/ClasspathResource.java

lib/java/polyglot-ydoc-server/src/test/java/org/enso/polyfill/EncodingPolyfillTest.java

build.sbt

lib/java/polyglot-ydoc-server/src/main/java/org/enso/polyfill/Main.java

lib/java/ydoc-server/src/test/java/org/enso/ydoc/polyfill/ParserPolyfillTest.java

lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java

JaroslavTulach · 2024-04-18T06:07:34Z

lib/rust/parser/jni/src/lib.rs

+/// Determine the token variant of the provided input.
+#[allow(unsafe_code)]
+#[no_mangle]
+pub extern "system" fn Java_org_enso_syntax2_Parser_isIdentOrOperator(


This method is also needed here: #8779 (comment)

from code clarity perspective - we should have just a single implementation

from performance perspective - it is better to stay in the JVM than jump out and back just to compare letters in a string - the overhead of input buffer conversion isn't negliable

not only a JVM vs. JNI problem

also JavaScript vs. WASM problem

However if the performance wasn't a problem so far, we probably don't need to care. It is more important to get the functionality right.

4e6 · 2024-04-19T12:25:40Z

It's alive! First run with the polyglot ydoc server.

ydoc_server_first.mp4

JaroslavTulach · 2024-04-20T05:45:39Z

It's alive! First run with the polyglot ydoc server.

I can reproduce your success myself if I:

enso$ POLYGLOT_YDOC_SERVER=true npm --workspace=enso-gui2 run build-ydoc-server-polyglot
enso$ sbt ydoc-server/assembly
enso$ POLYGLOT_YDOC_SERVER=true npm --workspace=enso-gui2 run dev

Small step for ydoc, but a huge leap for Enso! Congrats.

app/gui2/src/stores/project/index.ts

JaroslavTulach · 2024-04-20T06:14:41Z

Small step for ydoc, but a huge leap for Enso! Congrats.

I even got debugging working with:

enso$ POLYGLOT_YDOC_SERVER_DEBUG=true npm --workspace=enso-gui2 run dev
ydoc: Debugger listening on ws://127.0.0.1:34567/HwpxX-S2_c-dB0qRAMhY-LmSWALU7RSvXF5IBRkgeoA
For help, see: https://www.graalvm.org/tools/chrome-debugger
ydoc: E.g. in Chrome open: devtools://devtools/bundled/js_app.html?ws=127.0.0.1:34567/HwpxX-S2_c-dB0qRAMhY-LmSWALU7RSvXF5IBRkgeoA

I can connect to your process via ChromeDevTools:

app/gui2/vite.config.ts

somebody1234

approving dashboard changes
(the only changes there is a new ydocUrl parameter that gets threaded through the entire app to Editor.tsx)

JaroslavTulach

I see the whole support is optional right now - it doesn't affect workflow yet. Let's integrate and move on!
Please make sure the instructions in PR description to build and use this in the description are correct
Having a CI test that performs these instructions and runs (and shutdowns automatically) the ydoc-server would be a plus, but it is not strictly necessary right now.

JaroslavTulach · 2024-05-02T04:51:30Z

lib/java/ydoc-server/src/main/java/org/enso/ydoc/Main.java

@@ -14,6 +14,8 @@ public class Main {
  private Main() {}

  public static void main(String[] args) throws Exception {
+    System.setProperty("helidon.serialFilter.pattern", "javax.management.**;java.lang.**;java.rmi.**;javax.security.auth.Subject;!*");


I found this advice on stackoverflow and it does seem to help and configure the JEP-290 filter, so it allows JMX. I can connect to the process with JVisualVM polyglot sampler now.

I'd say the first edit is fine on my computer.

hubertp · 2024-05-02T07:52:20Z

It does feel like the first edit is taking considerably longer.

hubertp

post merge comments

hubertp · 2024-05-02T08:00:08Z

app/gui2/vite.config.ts

+        }
+        if (!existsSync(ydocServerJar)) {
+          const cwd = fileURLToPath(new URL('../..', import.meta.url))
+          const sbt = spawn('sbt', ['ydoc-server/assembly'], { cwd })


That seems like an unnecessary dependency. If someone forgot to build the jar, tough luck.

hubertp · 2024-05-02T08:04:23Z

build.sbt

-    frgaalJavaCompilerSetting
+    Compile / javaSource := baseDirectory.value / "generate-java" / "java"


Why no longer under frgaal?

It caused some compilation issues. Disabled because the project does not use Java 21 features.

build.sbt

hubertp · 2024-05-02T08:12:19Z

lib/java/ydoc-server/src/main/java/org/enso/ydoc/Main.java

+    var ydoc = Main.class.getResource(YDOC_SERVER_PATH);
+    var contextBuilder = WebEnvironment.createContext().allowIO(IOAccess.ALL);
+
+    Sampling.init();


Yes, optional. Just moved the logic to a separate class.

hubertp · 2024-05-02T08:23:59Z

lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/web/AbortController.java

+    log.debug(Arguments.toString(arguments));
+
+    return switch (command) {
+      case RANDOM_UUID -> UUID.randomUUID().toString();


isn't this a c&p mistake? There is no random-uuid for AbortController

I'll clean it up in the next PR

Addressing some PR comments, DRY.

While playing with the implementation addressed some PR comments of mine and applied DRY.

JaroslavTulach · 2024-05-27T14:31:03Z

follow up: Start Ydoc with the language server #9862

4e6 added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 25, 2024

4e6 self-assigned this Mar 25, 2024

JaroslavTulach reviewed Mar 25, 2024

View reviewed changes

Frizi reviewed Mar 25, 2024

View reviewed changes

enso-bot bot mentioned this pull request Mar 27, 2024

Conversion of UnresolvedConstructor to real constructor isn't instrumented #9381

Closed

4e6 force-pushed the wip/db/graalvm-ydoc-server branch from d31f221 to 80bcd19 Compare April 2, 2024 10:59

JaroslavTulach reviewed Apr 3, 2024

View reviewed changes

lib/java/polyglot-ydoc-server/src/test/java/org/enso/polyfill/EncodingPolyfillTest.java Outdated Show resolved Hide resolved

JaroslavTulach reviewed Apr 3, 2024

View reviewed changes

lib/java/polyglot-ydoc-server/src/test/java/org/enso/polyfill/EncodingPolyfillTest.java Outdated Show resolved Hide resolved

JaroslavTulach reviewed Apr 3, 2024

View reviewed changes

lib/java/polyglot-ydoc-server/src/test/java/org/enso/polyfill/EncodingPolyfillTest.java Outdated Show resolved Hide resolved

hubertp reviewed Apr 4, 2024

View reviewed changes

lib/java/polyglot-ydoc-server/src/main/java/org/enso/polyfill/ClasspathResource.java Outdated Show resolved Hide resolved

lib/java/polyglot-ydoc-server/src/test/java/org/enso/polyfill/EncodingPolyfillTest.java Outdated Show resolved Hide resolved

JaroslavTulach reviewed Apr 10, 2024

View reviewed changes

build.sbt Show resolved Hide resolved

4e6 force-pushed the wip/db/graalvm-ydoc-server branch from d71e200 to f1da6ec Compare April 11, 2024 10:17

JaroslavTulach reviewed Apr 12, 2024

View reviewed changes

lib/java/polyglot-ydoc-server/src/main/java/org/enso/polyfill/Main.java Outdated Show resolved Hide resolved

4e6 force-pushed the wip/db/graalvm-ydoc-server branch 4 times, most recently from da0280b to 9d7c750 Compare April 17, 2024 20:22

JaroslavTulach reviewed Apr 18, 2024

View reviewed changes

lib/java/ydoc-server/src/test/java/org/enso/ydoc/polyfill/ParserPolyfillTest.java Show resolved Hide resolved

JaroslavTulach reviewed Apr 18, 2024

View reviewed changes

lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java Outdated Show resolved Hide resolved

JaroslavTulach reviewed Apr 18, 2024

View reviewed changes

JaroslavTulach reviewed Apr 20, 2024

View reviewed changes

app/gui2/src/stores/project/index.ts Outdated Show resolved Hide resolved

enso-bot bot mentioned this pull request Apr 22, 2024

Generate completion of Table.join join criteria using data from both joined tables #5629

Closed

4e6 force-pushed the wip/db/graalvm-ydoc-server branch 3 times, most recently from a87e4f7 to f38c358 Compare April 29, 2024 09:14

JaroslavTulach reviewed Apr 29, 2024

View reviewed changes

app/gui2/vite.config.ts Show resolved Hide resolved

4e6 marked this pull request as ready for review May 1, 2024 11:42

4e6 requested review from PabloBuchu, indiv0, somebody1234, MrFlashAccount, farmaazon, vitvakatu and mwu-tow as code owners May 1, 2024 11:42

somebody1234 approved these changes May 1, 2024

View reviewed changes

misc: lint

a316b88

hubertp added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label May 1, 2024

misc: npm typecheck

8400376

kazcw approved these changes May 1, 2024

View reviewed changes

4e6 added 3 commits May 1, 2024 18:34

fix: npm run lint

a662c60

fix: lint strict-boolean-expressions

4f17e3b

fix: prettier

629d6b9

JaroslavTulach approved these changes May 2, 2024

View reviewed changes

Enable serialization of JMX beans

f508ffa

JaroslavTulach reviewed May 2, 2024

View reviewed changes

4e6 added the CI: Ready to merge This PR is eligible for automatic merge label May 2, 2024

mergify bot merged commit 5995a00 into develop May 2, 2024
37 checks passed

mergify bot deleted the wip/db/graalvm-ydoc-server branch May 2, 2024 06:28

hubertp reviewed May 2, 2024

View reviewed changes

hubertp added a commit that referenced this pull request May 2, 2024

Minor follow up to #9528

6ef0821

Addressing some PR comments, DRY.

hubertp added a commit that referenced this pull request May 2, 2024

Minor follow up to #9528

2fbfecb

Addressing some PR comments, DRY.

kazcw mentioned this pull request May 2, 2024

npm run dev not working #9847

Closed

2 tasks

enso-bot bot mentioned this pull request May 3, 2024

Treat errors from Frgaal compiler as errors, not as warnings #8513

Closed

mergify bot pushed a commit that referenced this pull request May 7, 2024

Minor follow up to #9528 (#9841)

930f3c5

While playing with the implementation addressed some PR comments of mine and applied DRY.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run ydoc-server with GraalVM #9528

Run ydoc-server with GraalVM #9528

4e6 commented Mar 25, 2024 •

edited

Loading

JaroslavTulach Mar 25, 2024

kazcw Mar 25, 2024 •

edited

Loading

JaroslavTulach Mar 25, 2024 •

edited

Loading

kazcw Mar 25, 2024

JaroslavTulach Mar 26, 2024 •

edited

Loading

kazcw Mar 26, 2024 •

edited

Loading

JaroslavTulach Apr 3, 2024 •

edited

Loading

4e6 Apr 9, 2024 •

edited

Loading

JaroslavTulach Apr 18, 2024

4e6 commented Apr 19, 2024

JaroslavTulach commented Apr 20, 2024 •

edited

Loading

JaroslavTulach commented Apr 20, 2024

somebody1234 left a comment

JaroslavTulach left a comment

JaroslavTulach May 2, 2024 •

edited

Loading

hubertp commented May 2, 2024

hubertp left a comment

hubertp May 2, 2024

hubertp May 2, 2024

4e6 May 2, 2024

hubertp May 2, 2024

4e6 May 2, 2024

hubertp May 2, 2024 •

edited

Loading

4e6 May 2, 2024

JaroslavTulach commented May 27, 2024

		frgaalJavaCompilerSetting
		Compile / javaSource := baseDirectory.value / "generate-java" / "java"

Run ydoc-server with GraalVM #9528

Run ydoc-server with GraalVM #9528

Conversation

4e6 commented Mar 25, 2024 • edited Loading

Pull Request Description

Important Notes

Checklist

Choose a reason for hiding this comment

kazcw Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

JaroslavTulach Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

kazcw Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

JaroslavTulach Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

4e6 Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4e6 commented Apr 19, 2024

JaroslavTulach commented Apr 20, 2024 • edited Loading

JaroslavTulach commented Apr 20, 2024

somebody1234 left a comment

Choose a reason for hiding this comment

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach May 2, 2024 • edited Loading

Choose a reason for hiding this comment

hubertp commented May 2, 2024

hubertp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hubertp May 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach commented May 27, 2024

4e6 commented Mar 25, 2024 •

edited

Loading

kazcw Mar 25, 2024 •

edited

Loading

JaroslavTulach Mar 25, 2024 •

edited

Loading

JaroslavTulach Mar 26, 2024 •

edited

Loading

kazcw Mar 26, 2024 •

edited

Loading

JaroslavTulach Apr 3, 2024 •

edited

Loading

4e6 Apr 9, 2024 •

edited

Loading

JaroslavTulach commented Apr 20, 2024 •

edited

Loading

JaroslavTulach May 2, 2024 •

edited

Loading

hubertp May 2, 2024 •

edited

Loading