Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Mar 1, 2023

Initial work to make trim legacy interop from native code and JavaScript.

Split off from #79622

Changes

  • added WasmEnableLegacyJsInterop MSBuild feature.
    • discussion about name here
    • defaults to true
    • It's available in runtime build as well as in application re-link via workload. At this time they need to be used together.
    • the re-link with WasmEnableLegacyJsInterop==false will not make the JS smaller. That would be follow-up work.
    • it maps to ENABLE_LEGACY_JS_INTEROP in native code
  • moved all legacy native functions to end of driver.c under #ifdef ENABLE_LEGACY_JS_INTEROP
    • just moved mono_wasm_get_delegate_invoke_ref and mono_wasm_box_primitive_ref lower in the same file.
    • just moved mono_wasm_get_type_name and mono_wasm_get_type_aqn higher in the same file.
    • moved mono_wasm_typed_array_new_ref from corebindings.c to driver.c
  • renamed core_initialize_internals to bindings_initialize_internals and unified it with mono_initialize_internals
    • removed CORE_BINDINGS definition
  • moved legacy cwraps to legacy_c_functions
    • methods in cwraps.ts and corebindings.c and dotnet.es6.lib.js are sorted in the same order
  • made TS type imports more explicit
  • moved mono_method_resolve to export_internal_api
  • made dotnet.es6.lib.js driven my environment variables, so that it could be configured at dev machine build time
    • see EmscriptenEnvVars in WasmApp.Native.targets
    • the same logic is also for rollup and runtime build. See wasm.proj
  • moved loadedFiles from 'Module' to runtimeHelpers

Future work

  • create multiple builds that the workload re-link could choose from based on the flags. This would make it useful for customer.
  • same transformation could be done for jiterpretter

@pavelsavara pavelsavara added this to the 8.0.0 milestone Mar 1, 2023
@pavelsavara pavelsavara requested review from kg and maraf March 1, 2023 14:39
@pavelsavara pavelsavara self-assigned this Mar 1, 2023
@ghost
Copy link

ghost commented Mar 1, 2023

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

Issue Details

Initial work to make trim legacy interop from native code and JavaScript.

Changes

  • added FeatureWasmLegacyJsInterop MSBuild feature.
    • defaults to true
    • It's available in runtime build as well as in application re-link via workload. At this time they need to be used together.
    • the re-link with FeatureWasmLegacyJsInterop==false will not make the JS smaller. That would be follow-up work.
    • it maps to FEATURE_LEGACY_JS_INTEROP in native code
  • removed CORE_BINDINGS definition
  • renamed core_initialize_internals to bindings_initialize_internals and unified it with mono_initialize_internals
  • moved legacy cwraps to legacy_c_functions
  • made TS type imports more explicit
  • moved mono_method_resolve to export_internal_api
  • made dotnet.es6.lib.js driven my environment variables, so that it could be configured at dev machine build time
    • see EmscriptenEnvVars in WasmApp.Native.targets
    • the same logic is also for rollup and runtime build. See wasm.proj
  • moved loadedFiles from 'Module' to runtimeHelpers
  • moved all legacy native functions to end of corebindings.c under #ifdef FEATURE_LEGACY_JS_INTEROP
  • removed unused mono_wasm_unbox_enum
  • methods in cwraps.ts and corebindings.c and dotnet.es6.lib.js are sorted in the same order

Future work

  • create multiple builds that the workload re-link could choose from based on the flags. This would make it useful for customer.
  • same transformation could be done for jiterpretter
Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: 8.0.0

@pavelsavara pavelsavara force-pushed the browser_optional_legacy_interop branch from 688deac to f25546a Compare March 1, 2023 22:40
@kg
Copy link
Contributor

kg commented Mar 1, 2023

Please ensure none of these funcs are used by the jiterpreter (CI ideally will catch it but isn't guaranteed)

@pavelsavara
Copy link
Member Author

Please ensure none of these funcs are used by the jiterpreter (CI ideally will catch it but isn't guaranteed)

@kg
I split method removal to separate PR, in to make it easier to review #82865

The rest of here should be just moving them into conditional block. I will re-base this on the other PR later.

@pavelsavara pavelsavara changed the title [browser][js] legacy JS interop optional via FeatureWasmLegacyJsInterop [browser] legacy JS interop optional via WasmEnableLegacyJsInterop - native Mar 2, 2023
@pavelsavara pavelsavara force-pushed the browser_optional_legacy_interop branch from f25546a to 4e5b832 Compare March 2, 2023 09:38
@pavelsavara pavelsavara marked this pull request as ready for review March 2, 2023 14:06
# Conflicts:
#	src/mono/wasi/runtime/CMakeLists.txt
@pavelsavara
Copy link
Member Author

CI failure is #80619

@pavelsavara pavelsavara merged commit 82871f4 into dotnet:main Mar 3, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 3, 2023
@pavelsavara pavelsavara deleted the browser_optional_legacy_interop branch September 2, 2024 15:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants