[wasm] optional legacy JS interop #79622

pavelsavara · 2022-12-13T20:15:34Z

currently only build time switch, not workload switch

removes runtime\net6-legacy\* from dotnet.js
- release size 338118 -> 304870 = 33248
removes relevant C code in driver.c from dotnet.wasm
- release size 2409118 -> 2407787 = 1331
removes legacy stuff from System.Runtime.InteropServices.JavaScript.dll
- release size 32256 -> 24064 = 8192

ghost · 2022-12-13T20:15:44Z

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

currently only build time switch, not workload switch

removes runtime\net6-legacy\* from dotnet.js
- release size 338118 -> 304870
removes relevant C code in driver.c from dotnet.wasm
- release size 2409118 -> 2407787
removes legacy stuff from System.Runtime.InteropServices.JavaScript.dll
- release size 32256 -> 24064

Author:	pavelsavara
Assignees:	pavelsavara
Labels:	`arch-wasm`, `area-System.Runtime.InteropServices.JavaScript`
Milestone:	8.0.0

pavelsavara · 2022-12-14T18:00:54Z

We should make decision on how to proceed with this

merge close to what it is and set MonoWasmLegacyJsInterop to false

for the threaded binaries as the legacy interop doesn't work well anyway there
possibly for new binary flavor (or mixed with EH, SIMD)

invest more effort and make this workload linker flag

we would have to split the C methods into separate binary object
we would have to somehow explain the flag to trimmer on dev machine for IL stuff.
- There is [DynamicDependecy] which need to be configurable.
we would need to make the JS part separate ES6 module
- because we don't have rollup on the dev machine
and then deal with SDK impact of the new files

is easier
same pattern could be also applied to Jitterpretter too

thoughts ? @kg @lewing @lambdageek

kg · 2022-12-14T18:03:44Z

I think we should start with 1 (merge close to what it is now, only disable for threading and for new scenarios going forward) and then prepare 2 as a separate PR

src/libraries/System.Runtime.InteropServices.JavaScript/src/Resources/Strings.resx

radical · 2022-12-14T19:24:09Z

src/mono/wasm/build/WasmApp.Native.targets

+      <EmscriptenEnvVars Include="MonoWasmLegacyJsInterop=$(MonoWasmLegacyJsInterop)"/>
+      <EmscriptenEnvVars Include="MonoWasmThreads=$(MonoWasmThreads)"/>


Should these be set only when they have value?

They must have value even if we just default it to false or true.

The env variable is the way how to evaluate it in dotnet.es6.lib.js while linking.
But when this code would be running on user dev machine, I don't know how to get the value which was previously used to compile this runtime flavor in our CI.

There was USE_PTHREADS which is pre-processor directive, but it works only for existing emcc settings. Whatever could be passed with -s XXX. Benefit is, that emcc-link.rsp would have that value stored for the runtime flavor.

But here I want to add something which emcc doesn't know about and so I went with env variables. But those are not in emcc-link.rsp. As I write this comment now I rememebred that there is emcc-props.json which could carry that value for me. That would work.

But is that wasted effort if we go for 2) as next step ?

src/mono/wasm/runtime/cwraps.ts

src/mono/wasm/runtime/exports-linker.ts

src/mono/wasm/runtime/startup.ts

src/mono/wasm/build/WasmApp.Native.targets

# Conflicts: # src/mono/wasm/runtime/cwraps.ts

Co-authored-by: Ankit Jain <radical@gmail.com>

lewing · 2022-12-21T02:05:25Z

I love the idea here perhaps we can gate the actual managed bits on a linker flag like the latest simd settings and keep most of the size improvement that way? I'd love a better way to solve the unmanage<->managed linking problem but we don't always need a general solution.

...rvices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs

marek-safar · 2022-12-21T09:17:16Z

...nteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs

+#if FEATURE_LEGACY_JS_INTEROP
        [DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.LegacyExports", "System.Runtime.InteropServices.JavaScript")]
+#endif


This does not let us to remove LegacyExports because it's a library compile time setting. We could remove this DynamicDependency and use XML descriptor with System.Runtime.InteropServices.JavaScript.LegacyExports and pass it to linker only when legacy interop is enabled.

That XML would become embedded resource, right ? How do I tell the linker to ignore it or not in WasmApp.Native.targets ?

It would be additional file passed conditionally to the linker (we run it all the time to this should work).

For threaded runtime flavor everything would be already trimmed away.

For normal runtime flavor we would keep the code with DynamicDependency above.
User would be able to opt in for trimming on customer's dev machine.

I can follow example of WasmEnableSIMD and ILLink.Substitutions.WasmIntrinsics.xml.
Except I need ILLink.LinkAttributes.xml which could remove the attribute ?

That won't work DynamicDependency is always processed and it only removed later. The attribute cannot be visible to linker in this case to work as expected.

We should discuss the mechanisms we need in the regular build to pass additional linker files based on options as part of the bunder/sdk changes

# Conflicts: # src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs

# Conflicts: # src/mono/wasm/runtime/startup.ts

pavelsavara · 2023-01-03T16:35:14Z

Today I tried to separate the net6-legacy/*.ts into separate rollup->IIFE. If we are able to do that we could
A) download it as separate ES6 module
B) or we could let the emcc linker to merge it into final dotnet.js

I was able to draft detaching the runtime->legacy dependency into small surface of few initialization functions which would the runtime all on the legacy module to connect it.

But, there are many dependencies in the opposite direction. Legacy interop uses almost all internal infrastructure of the runtime. JS code we have. We could export internal "API" for this purpose. It means that all the internal function names would have to be protected from minification.

I'm scratching my head what to do about it conceptually, so that we could use the same approach for jiterpreter code and possibly also for MT infrastructure.

make our runtime.iffe less opaque and trust emcc/closure that it could propagate constants and eliminate dead code.
include rollupJS in the wasm workload
expose internal API between our modules with A) B)
give up JS modularization / trimming features (on dev machine)

pavelsavara · 2023-01-03T23:21:10Z

re 1)

the closure compiler needs to be enabled first [draft][browser] enable closure compiler for dotnet.js #80178
acorn optimizer is not ES6 friendly yet fix JS optimizer fail on ES6 spread operator emscripten-core/emscripten#18461
the flag --closure-args=--define=FeatureWasmLegacyJsInterop=false probably only works in global scope, but we have MODULARIZE=1

pavelsavara · 2023-01-18T09:46:42Z

note to self mono_wasm_new_roots and set_root is unused ?

pavelsavara · 2023-01-19T16:06:19Z

The closure compiler (and the trimming side effects) could be only run without -g that is with WasmNativeStrip==true

ghost · 2023-02-18T17:01:52Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

pavelsavara added 3 commits December 13, 2022 19:17

wip

d411d71

Merge branch 'main' into wasm_optional_legacy_interop

2a4a901

wip

aba32b9

pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript labels Dec 13, 2022

pavelsavara added this to the 8.0.0 milestone Dec 13, 2022

pavelsavara requested review from kg, lambdageek, radekdoulik and maraf December 13, 2022 20:15

pavelsavara self-assigned this Dec 13, 2022

fix

d7c360c

build-analysis bot mentioned this pull request Dec 14, 2022

Tracking issue for CI build timeouts #76454

Closed

radical reviewed Dec 14, 2022

View reviewed changes

pavelsavara and others added 4 commits December 16, 2022 13:39

Merge branch 'main' into wasm_optional_legacy_interop

62ecc51

# Conflicts: # src/mono/wasm/runtime/cwraps.ts

Update src/mono/wasm/runtime/exports-linker.ts

f6fd817

Co-authored-by: Ankit Jain <radical@gmail.com>

Update src/mono/wasm/runtime/cwraps.ts

85e0b93

Co-authored-by: Ankit Jain <radical@gmail.com>

Update src/mono/wasm/runtime/startup.ts

db1b8e6

Co-authored-by: Ankit Jain <radical@gmail.com>

marek-safar reviewed Dec 21, 2022

View reviewed changes

pavelsavara added 4 commits December 26, 2022 21:48

Merge branch 'main' into wasm_optional_legacy_interop

17960ae

# Conflicts: # src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs

fix merge

d14a2bb

Merge branch 'main' into wasm_optional_legacy_interop

258915f

# Conflicts: # src/mono/wasm/runtime/startup.ts

fix

a6e1542

pavelsavara mentioned this pull request Jan 3, 2023

[blazor][wasm] unify startup bit more dotnet/aspnetcore#45628

Merged

wip

f54d2a3

build-analysis bot mentioned this pull request Jan 3, 2023

emcc received SIGKILL #79874

Closed

wip

5afe1ac

ghost closed this Feb 18, 2023

This was referenced Mar 1, 2023

[browser] legacy JS interop optional via WasmEnableLegacyJsInterop - managed #82826

Merged

[browser] legacy JS interop optional via WasmEnableLegacyJsInterop - native #82834

Merged

ghost locked as resolved and limited conversation to collaborators Apr 1, 2023

pavelsavara deleted the wasm_optional_legacy_interop branch September 2, 2024 15:29

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wasm] optional legacy JS interop #79622

[wasm] optional legacy JS interop #79622

pavelsavara commented Dec 13, 2022 •

edited

Loading

ghost commented Dec 13, 2022

pavelsavara commented Dec 14, 2022

kg commented Dec 14, 2022

radical Dec 14, 2022

pavelsavara Dec 16, 2022

lewing commented Dec 21, 2022

marek-safar Dec 21, 2022

pavelsavara Dec 21, 2022

marek-safar Dec 21, 2022

pavelsavara Jan 3, 2023

marek-safar Jan 4, 2023

lewing Jan 6, 2023

pavelsavara commented Jan 3, 2023 •

edited

Loading

pavelsavara commented Jan 3, 2023 •

edited

Loading

pavelsavara commented Jan 18, 2023 •

edited

Loading

pavelsavara commented Jan 19, 2023

ghost commented Feb 18, 2023

		<EmscriptenEnvVars Include="MonoWasmLegacyJsInterop=$(MonoWasmLegacyJsInterop)"/>
		<EmscriptenEnvVars Include="MonoWasmThreads=$(MonoWasmThreads)"/>

[wasm] optional legacy JS interop #79622

[wasm] optional legacy JS interop #79622

Conversation

pavelsavara commented Dec 13, 2022 • edited Loading

ghost commented Dec 13, 2022

pavelsavara commented Dec 14, 2022

kg commented Dec 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lewing commented Dec 21, 2022

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

pavelsavara commented Jan 3, 2023 • edited Loading

pavelsavara commented Jan 3, 2023 • edited Loading

pavelsavara commented Jan 18, 2023 • edited Loading

pavelsavara commented Jan 19, 2023

ghost commented Feb 18, 2023

pavelsavara commented Dec 13, 2022 •

edited

Loading

pavelsavara commented Jan 3, 2023 •

edited

Loading

pavelsavara commented Jan 3, 2023 •

edited

Loading

pavelsavara commented Jan 18, 2023 •

edited

Loading