[browser] bind JS methods to minified WASM imports

[pavelsavara]

Step one

• replace_linker_placeholders moved to exports-binding.ts file
• replace_linker_placeholders now returns array, not object with function names
  • improves dotnet.runtime.js size a bit (222629->221446 uncompressed)
• rollup.config.js will execute exports-linker.ts in process to extract function names to indexes maping
• wasm.proj will append the json to the end of dotnet.es6.lib.js
• dotnet.es6.lib.js will generate stubs which return it's index

Step two (actual fix)

• replace_linker_placeholders will call all stubs in order to create mapping from (minified) import name to index in the dotnet.runtime.js module exports

Other

• added gitHash to all modules and warn to console if the don't match

Discussion

• in my testing emcc didn't minify the names of the JS stubs, that's pity because they are only used by linker, not at runtime
• we could implement softer version of this PR with names instead of indexes
• if something called the stub, it would receive object like {runtime_idx:1} which would throw, if called from WASM
• we could generate stub, which would throw better message, but would be more complex
• we could generate stub, which would self-bind when possible. I prefer just one binding code path.

Fix #90111

[ghost]

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

Issue Details

---

Step one

• replace_linker_placeholders moved to exports-linker.ts file
• replace_linker_placeholders now returns array, not object with function names
  • improves dotnet.runtime.js size a bit
• dotnet.es6.lib.js needs to have the list in the same order
• dotnet.es6.lib.js will generate stubs which return it's index

Step two (actual fix)

• replace_linker_placeholders will call all stubs in order to create mapping from (minified) import name to index in the dotnet.runtime.js module exports

Discussion

• in my testing emcc didn't minify the names of the JS stubs, that's pity because they are only used by linker, not at runtime
• we could implement softer version of this PR with names instead of indexes
• if something called the stub, it would receive object like {runtime_idx:1} which would throw, if called from WASM
• we could generate stub, which would throw better message, but would be more complex
• we could generate stub, which would self-bind when possible. I prefer just one binding code path.

Fix #90111

Author:	pavelsavara
Assignees:	pavelsavara
Labels:	arch-wasm, os-browser
Milestone:	8.0.0

[radical]

/azp run runtime-wasm

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kg]

I really don't like the order mattering

[pavelsavara]

I really don't like the order mattering

Why not ? What do you propose ?

[kg]

I really don't like the order mattering

Why not ? What do you propose ?

It adds an easy place for very confusing failures to creep in. But I don't completely understand why it needs to be this way

[pavelsavara]

We have more options, I think

• go back to not patching the symbols at all
  • possibly minor perf degradation, because of the extra hop
• generate stubs which return full name of itself (instead of index) and use that to bind it
  • slightly larger JS size
• assume that nobody would further minify dotnet.native.js after emcc passes and use stub function .name
  • I think this is no-go because people transform the JS further
• implement a test to make sure no method index confusion creeped in
• make this more automatic, so that we don't have to synch those lists by hands, via rollup probably.

[kg]

Thanks, out of your options:

* generate stubs which return full name of itself (instead of index) and use that to bind it
  * slightly larger JS size
* implement a test to make sure no method index confusion creeped in
* make this more automatic, so that we don't have to synch those lists by hands, via rollup probably.

all three of those sound fine to me

[pavelsavara]

* make this more automatic, so that we don't have to synch those lists by hands, via rollup probably.

all three of those sound fine to me

Selected the automation, because maintaining both lists was dummy chore already anyway.

Now it could only confuse indexes if you mismatched versions of dotnet.runtime.js and dotnet.es6.lib.js.
Should I assert some fingerprint of the method list or this is good enough ?

[kg]

The reliance on Function.name might bite us someday - add a check to make sure there are no name collisions, because a name collision could trigger an overwrite if I understand this right - but I like this solution

[pavelsavara]

with this change:

dotnet.native.js will contain function _mono_wasm_schedule_timer(){return{runtime_idx:0}} mapped to "y": _mono_wasm_schedule_timer slot of the imports.

The dotnet.runtime.js will have

const Ul = [function (e) { Po && (globalThis.clearTimeout(Po), Po = void 0), Po = n.safeSetTimeout(mono_wasm_schedule_timer_tick, e) }

When both dotnet.runtime.js and dotnet.native.js are minified and diagnosticTracing enabled we may see following log line.
MONO_WASM: Replaced WASM import y stub _mono_wasm_schedule_timer with minified implementation
Because the function in the dotnet.runtime.js doesn't have name.

[pavelsavara]

/azp run runtime-wasm

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[pavelsavara]

I see #86387 failure on CI

[pavelsavara]

Both CI failures are unrelated