[NativeAOT LLVM] Enable JS interop #2440

maraf · 2023-11-24T15:08:04Z

The goal is not to enable all JS import/export scenarios, but start with some simple ones
Updated smoke tests
The mono_wasm_bind_js_function and mono_wasm_bind_cs_function are duplicated for NativeAOT case
Due to DisableImplicitFrameworkReferences=true in tests, generators are added as explicit references

Contributes to #2434

elringus · 2023-12-08T19:18:28Z

@maraf Sorry if I'm asking something irrelevant here, but do I understand correctly that there are plans to move WASM target from Mono to NativeAOT/LLVM? If that's the case, will the existing "high-level" JS interop API (the JSImport/Export stuff from System.Runtime.InteropServices.JavaScript) still work with the new backend or it'll be something different?

maraf · 2023-12-09T19:21:08Z

@maraf Sorry if I'm asking something irrelevant here, but do I understand correctly that there are plans to move WASM target from Mono to NativeAOT/LLVM?

There is no such plan at the moment. Anyway NativeAOT/LLVM has come a long way and we are looking at comparison

If that's the case, will the existing "high-level" JS interop API (the JSImport/Export stuff from System.Runtime.InteropServices.JavaScript) still work with the new backend or it'll be something different?

The goal of the #2434 is to have the same/close as possible API for both Mono and NativeAOT/LLVM in the context of the browser, so that is possible try to wasmbrowser based apps on NativeAOT/LLVM.

…t bits

src/mono/wasm/runtime/invoke-cs.ts

jkotas · 2023-12-18T15:40:48Z

...stem.Runtime.InteropServices.JavaScript/src/System.Runtime.InteropServices.JavaScript.csproj

@@ -22,7 +22,8 @@
  </ItemGroup>

  <ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'browser'">
-    <Compile Include="$(CommonPath)Interop\Browser\Interop.Runtime.cs" Link="System\Runtime\InteropServices\JavaScript\Interop\Interop.Runtime.cs" />
+    <Compile Include="$(CommonPath)Interop\Browser\Interop.Runtime.cs" Link="System\Runtime\InteropServices\JavaScript\Interop\Interop.Runtime.cs" Condition="'$(RuntimeFlavor)' != 'CoreCLR'" />


libraries build system is not setup to build different binaries per runtime flavor. We have all libraries (except CoreLib) 100% shared between all runtimes flavors.

This change is ok as a quick hack for runtimelab, but it would not be upstreamable. Add a comment about it?

(The proper way to do this would be to come up with a scheme that works for all runtime flavors and/or move any runtime specific parts into CoreLib.)

I would expect this can be solved similar to Linq.Expressions, with an internal feature switch (always off for Mono, always on for NativeAOT).

SingleAccretion · 2023-12-18T17:39:02Z

src/tests/nativeaot/SmokeTests/DotnetJs/Program.cs

+        internal static partial int Math(int a, int b, int c);
+
+        [JSExport]
+        internal static int Square(int x)
+        {
+            Console.WriteLine($"Computing square of {x}");
+            return x^2;
+        }


Could you write more tests? For JSExport, you can take inspiration (aka copy code) from the SharedLibrary test - the main items of interest are EH and GC exercising methods.

For both JSExport and JSImport, it would be good to have some number of methods that exercise the actual marshaling (strings, arrays, tasks, delegates, basically, challening cases from the table in https://learn.microsoft.com/en-us/aspnet/core/blazor/javascript-interoperability/import-export-interop?view=aspnetcore-8.0). Perhaps these tests are already written somewhere and too could be copied...

I guess it will take some time yet before you are able to run this https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JSImportExportTest.cs

Right. We don't need super-comprehensive testing here, but something that would fail if some basic aspect of the system gets broken.

I would rather use standard JS interop tests when we get there. I don't see value in duplicating them here. This is just a smoke test.

Also, a lot of JS interop scenarios don't work yet and are out of scope for this PR

Also, a lot of JS interop scenarios don't work yet and are out of scope for this PR

Right. I would expect that as they are enabled, they get added to this test. Could you add a few simple ones that already work then (like I would expect strings to work)?

One of the reasons why it is valuable to have this testing is that even when you enable the full libraries test suite, it will take > minute to compile, while this test will take much less. Libraries tests are not suitable for inner loop.

Strings for example don't work

Interesting that they don't. Well, let's leave this be until they do.

We need dotnet/runtime#95180 for strings

We need dotnet/runtime#95180 for strings

Not in time for this PR, but I will start the next merge soon.

It's ok, no rush on that one, I have other things to do :)

src/libraries/Common/src/Interop/Browser/Interop.Runtime.NativeAOT.cs

SingleAccretion · 2023-12-18T17:52:34Z

src/libraries/Common/src/Interop/Browser/Interop.Runtime.NativeAOT.cs

+            BindJSFunction(function_name, function_name.Length, module_name, module_name.Length, signature, out bound_function_js_handle, out is_exception);
+            if (is_exception != 0)
+                result = "Runtime.BindJSFunction failed";
+            else
+                result = "";


The original Mono functions were internal calls and would run in cooperative GC mode (i. e. as if they were managed code). DllImport is PInvoke, which puts the code into preemptive mode. I wonder if this will cause problems.

(Code in preemptive mode moslty cannot touch managed state).

what's the plan for running GC for NAOT on browser ? What would trigger it and what would schedule finalizers ?

what's the plan for running GC for NAOT on browser ? What would trigger it and what would schedule finalizers ?

Same as on other platforms - you GC whenever GC decides its time to GC. Practically, this will happens on allocations, or explicit GC.Collects.

Finalization is a little bit NYI right now (#2240), but it could follow the same plan (nice because WASI would have to do that), or be hooked into the event loop.

I guess we need to make decision about yielding to browser event loop.
Big part of JS interop is moot without it (promises, network events, UI).

Well, I would expect the yielding to happen naturally in async code, as you hit awaits and such. GC shouldn't really play much into it? We would of course have the Browser-specific thread pool implementation.

src/mono/wasm/runtime/invoke-cs.ts

src/mono/wasm/runtime/invoke-js.ts

src/mono/wasm/runtime/invoke-cs.ts

…rue works properly

SingleAccretion

Just a few comments, we are quite close. Thank you for collapsing back TS changes!

src/coreclr/nativeaot/BuildIntegration/DotNetJsApi.targets

src/libraries/Common/src/Interop/Browser/Interop.Runtime.NativeAOT.cs

src/mono/wasm/runtime/invoke-cs.ts

…ndling

SingleAccretion

LGTM modulo two questions and assuming green CI. Thank you!

I would also ask to add a note about the JSInterop library build workaround to #2434.

src/mono/wasm/runtime/dotnet.d.ts

src/mono/wasm/runtime/types/emscripten.ts

src/mono/wasm/runtime/invoke-cs.ts

pavelsavara · 2023-12-19T14:00:00Z

src/mono/wasm/runtime/roots.ts

@@ -60,6 +61,15 @@ export function mono_wasm_new_root_buffer_from_pointer(offset: VoidPtr, capacity
 * Releasing this root will not de-allocate the root space. You still need to call .release().
 */
 export function mono_wasm_new_external_root<T extends MonoObject>(address: VoidPtr | MonoObjectRef): WasmRoot<T> {
+    if (NativeAOT) {


Since we don't need GC roots for MonoString* and we will delete legacy interop soon, I hope that we could figure out way how to get rid of all roots too (on main). That would make your life easier.

I'm looking forward to it

maraf added the area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly) label Nov 24, 2023

maraf self-assigned this Nov 24, 2023

Replace icalls with dllimports for JS import/export. Update javascrip…

e218cf7

…t bits

maraf force-pushed the jsinterop branch from 2d39054 to e218cf7 Compare December 14, 2023 14:04

maraf added 8 commits December 14, 2023 15:07

Add js import sample

cbb8d6f

Fix typescript build

44611b9

Fix Program.cs

9f6993e

Fix Program.cs

0f36fbf

Try to fix JS interop with explicit project reference

b07ad92

Check math result

3f05288

Comment about analyzer references

132bc23

Move DllImports to a new file

3aec518

maraf changed the title ~~[NativeAOT LLVM] JS Interop~~ [NativeAOT LLVM] Enable JS interop Dec 18, 2023

maraf added 4 commits December 18, 2023 14:36

Split mono_wasm_bind_js_function to mono and naot

6b67394

Split mono_wasm_bind_cs_function

43a5389

Fix emscripten import names

1a666ec

Add JSExport to sample

b8621b8

maraf commented Dec 18, 2023

View reviewed changes

src/mono/wasm/runtime/invoke-cs.ts Outdated Show resolved Hide resolved

maraf requested review from a team and pavelsavara December 18, 2023 14:23

maraf marked this pull request as ready for review December 18, 2023 14:24

jkotas reviewed Dec 18, 2023

View reviewed changes

maraf added 2 commits December 18, 2023 17:19

Use different exit codes than runtime does

d2107e4

Feedback about Interop.Runtime.cs

4ee6148

SingleAccretion reviewed Dec 18, 2023

View reviewed changes

src/mono/wasm/runtime/invoke-cs.ts Outdated Show resolved Hide resolved

SingleAccretion reviewed Dec 18, 2023

View reviewed changes

src/mono/wasm/runtime/invoke-js.ts Outdated Show resolved Hide resolved

SingleAccretion reviewed Dec 18, 2023

View reviewed changes

maraf added 8 commits December 19, 2023 10:47

Split typescript imports

da24ea2

Fix sample

fce6919

Revert typescript split

a4a2a1c

Fix export sample

2542cc9

Remove export sample

eee20bd

Make JSExport work

d112a86

Comments

a0cb512

Import DotNetJsApi.targets earlier so IlcExportUnmanagedEntrypoints=t…

3b3b7a5

…rue works properly

SingleAccretion reviewed Dec 19, 2023

View reviewed changes

maraf added 5 commits December 19, 2023 12:01

Add exception sample. Move todos to string marshaling in exception ha…

0ecbd84

…ndling

Feedback

4b81dab

Use library import and utf16

bb0fed7

utf16ToString instead of Module.UTF16ToString

79ff965

Write to ExportsFile only if it doesn't contain required exports

bfd9d76

maraf requested a review from SingleAccretion December 19, 2023 11:53

SingleAccretion approved these changes Dec 19, 2023

View reviewed changes

src/mono/wasm/runtime/dotnet.d.ts Outdated Show resolved Hide resolved

src/mono/wasm/runtime/types/emscripten.ts Outdated Show resolved Hide resolved

maraf added 2 commits December 19, 2023 13:51

Drop emscripten's UTF16ToString

4ffd55f

Quotes around variables

7025ec8

pavelsavara approved these changes Dec 19, 2023

View reviewed changes

maraf added 2 commits December 19, 2023 15:11

Feedback

2b48d5a

More feedback

edc82cb

maraf merged commit 3d61458 into dotnet:feature/NativeAOT-LLVM Dec 19, 2023
11 checks passed

maraf deleted the jsinterop branch December 19, 2023 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NativeAOT LLVM] Enable JS interop #2440

[NativeAOT LLVM] Enable JS interop #2440

maraf commented Nov 24, 2023 •

edited

Loading

elringus commented Dec 8, 2023

maraf commented Dec 9, 2023

jkotas Dec 18, 2023

jkotas Dec 18, 2023

SingleAccretion Dec 18, 2023

SingleAccretion Dec 18, 2023

pavelsavara Dec 18, 2023

SingleAccretion Dec 18, 2023

maraf Dec 18, 2023

SingleAccretion Dec 18, 2023

maraf Dec 18, 2023

SingleAccretion Dec 18, 2023

maraf Dec 19, 2023

yowl Dec 19, 2023

maraf Dec 19, 2023

SingleAccretion Dec 18, 2023

pavelsavara Dec 18, 2023

SingleAccretion Dec 18, 2023

pavelsavara Dec 18, 2023

SingleAccretion Dec 18, 2023

SingleAccretion left a comment

SingleAccretion left a comment

pavelsavara Dec 19, 2023

maraf Dec 19, 2023

[NativeAOT LLVM] Enable JS interop #2440

[NativeAOT LLVM] Enable JS interop #2440

Conversation

maraf commented Nov 24, 2023 • edited Loading

elringus commented Dec 8, 2023

maraf commented Dec 9, 2023

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

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

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

SingleAccretion left a comment

Choose a reason for hiding this comment

SingleAccretion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maraf commented Nov 24, 2023 •

edited

Loading