WASM Bindings optimizations and fixes #41808

kg · 2020-09-03T16:39:20Z

This PR contains various improvements and optimizations to the bindings layer, along with related bug fixes (necessary to land it), including:

A new bind_method API that provides a more efficient alternative to call_method by preparing most of the information needed for an invoke ahead of time. The resulting method accepts bare arguments and can be called like a regular JS function. The resulting method also has a human-readable name and sourceURL so that it is clearly identified in stack traces and in browser devtools' sources tab
A JIT compiler for argument list marshaling to significantly boost its performance
A JIT compiler for bound methods to provide fast marshaling and (where possible) prevent return values from being boxed and causing JS GCs
Changes the syntax for method signatures so that a trailing ! indicates an unmarshaled return value, instead of the previous m (the old syntax was ambiguous and hinders efficient call dispatch)
Fixes multiple call_method call sites that had incorrect signatures
Changes various high-traffic call_method sites to use bind_method instead
Expands the browser sample to contain a simple benchmark that measures the overhead of bindings method calls.
Adds a new mono_wasm_try_unbox_primitive_and_get_type API to the driver that combines the type-check and unbox operations into one call, and writes the primitives to the heap instead of returning them through the wasm<->JS FFI. This enables faster unboxing and the values being written to the native heap ensures they don't get allocated on the JS GC heap.
Adds additional MARSHAL_TYPE_ enumeration values so that values of varying types can be marshaled more accurately.
Fixes some bugs in the existing JS root API and improves its performance.
Optimizes marshaling of JS strings and fixes an existing bug where JS strings were truncated at the first embedded null
Unifies the boxing and unboxing paths for primitives instead of going through managed/unmanaged code to manipulate boxes (with the exception of one API that runtime-test relies on)
Fixes passing and returning enums through the bindings layer (this was broken/half-implemented before)
Fixes issue where the managed wrapper for JS globals like Math would not be re-created after it had been GC'd
Fixes various problems with passing uint32 across the managed/js boundary
Fixes issue System.Runtime.InteropServices.JavaScript.Tests.MarshalTests.TestFunctionApply failed on CI (wasm mono subset) #40112

kg · 2020-09-12T05:07:56Z

An example of what the JIT-compiled functions look like in browser devtools (ignore the bug in this generated code's switch block, I fixed it)

marek-safar · 2020-09-14T08:56:00Z

src/mono/netcore/sample/wasm/browser/Program.cs

@@ -19,5 +19,9 @@ public static int TestMeaning()
        {
            return 42;
        }
+
+        public static float BenchmarkMethod (int i, float f, double d) {


This should go to benchmarks suite

/cc @SamMonoRT

marek-safar · 2020-09-14T08:56:31Z

src/mono/netcore/sample/wasm/browser/index.html

-    Result from Sample.Test.TestMeaning: <span id="out"></span>
+    Result from Sample.Test.TestMeaning: <span id="out"></span><br>
+    Progress: <span id="progress">Starting...</span><br>
+    Avg. Speed: <span id="speed"></span>


This should go to benchmarks suite

Was planning to remove it entirely before merging, but I can move it to the benchmarks suite.

I can't find the benchmarks suite you suggested moving it to, but I split it into its own folder for now. Where precisely did you want it to go?

I was thinking https://github.com/dotnet/performance

src/mono/wasm/runtime/binding_support.js

marek-safar · 2020-09-14T09:08:46Z

src/mono/wasm/runtime/binding_support.js

+			var uriPrefix = "", escapedFunctionIdentifier = "";
+
+			if (name) {
+				uriPrefix = "//# sourceURL=https://mono-wasm.invalid/" + name + "\r\n";


Shouldn't this be more like .generated instead of .invalid?

.invalid is the only reserved TLD that is guaranteed not to ever resolve to anything.

I don't think it actually needs a scheme and a host so you could just use the name.

src/mono/wasm/runtime/binding_support.js

kg · 2020-09-19T01:43:41Z

The string changes in this PR appear to fix the root cause of #41604 and another issue that isn't tracked on github - essentially, large strings seem to cause intermittent crashes and/or memory corruption when crossing the JS<->WASM boundary. I'm working on figuring out whether that fix can easily be cherry-picked out.

lewing · 2020-10-13T18:49:33Z

@kg the remaining failures appear genuine, have you taken a look at them?

lewing

This is looking very good. More comments in a bit.

src/mono/wasm/runtime/library_mono.js

lewing · 2020-10-15T00:57:22Z

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

-                if (!_boundObjects.TryGetValue(jsId, out reference))
+                if (_boundObjects.TryGetValue(jsId, out reference))
+                {
+                    if ((reference.Target == null) || ((reference.Target as JSObject)?.IsDisposed == true))


I'm fairly sure this is just masking the problem, I wonder if the issue is that we need to be able to promote weak refs into strong ones.

I think it's correct in some scenarios for it to have expired, if nothing is retaining a reference to globalThis.Math then the C# wrapper for it should be able to get collected. That's at least what was causing the test failures

lewing · 2020-10-15T00:59:44Z

src/mono/wasm/runtime/driver.c

@@ -105,10 +105,20 @@ mono_wasm_invoke_js (MonoString *str, int *is_exception)
 			res = res.toString ();
 			setValue ($2, 0, "i32");
 		} catch (e) {
-			res = e.toString ();


we need to fix our c and js formatting

lewing · 2020-10-15T01:08:46Z

src/mono/wasm/runtime/binding_support.js

+			var uriPrefix = "", escapedFunctionIdentifier = "";
+
+			if (name) {
+				uriPrefix = "//# sourceURL=https://mono-wasm.invalid/" + name + "\r\n";


I don't think it actually needs a scheme and a host so you could just use the name.

src/mono/wasm/runtime/binding_support.js

Fix a couple broken tests

lewing · 2020-11-11T01:40:29Z

rebased on master to kick off CI again

kg added the arch-wasm WebAssembly architecture label Sep 3, 2020

Dotnet-GitSync-Bot added the area-Meta label Sep 3, 2020

kg requested review from lewing, akoeplinger and vargaz September 3, 2020 16:39

kg force-pushed the bindings-optimizations-1 branch 2 times, most recently from f09e11f to 6467165 Compare September 9, 2020 19:21

kg changed the title ~~[DRAFT] js->managed bindings optimizations~~ WASM Bindings optimizations Sep 12, 2020

kg added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 12, 2020

kg marked this pull request as ready for review September 12, 2020 05:05

kg requested review from EgorBo, marek-safar and steveisok as code owners September 12, 2020 05:05

marek-safar reviewed Sep 14, 2020

View reviewed changes

steveisok reviewed Sep 14, 2020

View reviewed changes

src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved

kg changed the title ~~WASM Bindings optimizations~~ WASM Bindings optimizations and fixes Sep 19, 2020

kg force-pushed the bindings-optimizations-1 branch from e29be9f to 649375b Compare September 24, 2020 01:33

This was referenced Sep 30, 2020

Mono System.Text.Json.Tests on Windows timing out #42677

Open

Mono SIGABRT in System.Drawing.Common affecting some test runs #37838

Closed

kg removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 12, 2020

kg force-pushed the bindings-optimizations-1 branch from 44d5fae to e1144dd Compare October 13, 2020 01:28

runfoapp bot mentioned this pull request Oct 13, 2020

Bundle_Is_Extracted test failing intermittently on Linux #43316

Closed

lewing mentioned this pull request Oct 14, 2020

Simplify the number unboxing logic #43271

Closed

kg force-pushed the bindings-optimizations-1 branch from f83e6cf to 77bccfa Compare October 14, 2020 22:16

lewing reviewed Oct 15, 2020

View reviewed changes

runfoapp bot mentioned this pull request Oct 15, 2020

System.Net.Security.Tests timing out #43403

Closed

kg and others added 26 commits November 10, 2020 19:39

Fix typo

2f1f031

Add some tests

f5e9877

Fix test failures

2821872

Add more error handling and diagnostic messages

61bcc89

Fix a couple broken tests

Repair merge damage

7ad28a0

Remove bindings benchmark

e165bba

Use TypedArray.fill 3-argument version to zero memory

f43d68c

Checkpoint: Introduce format strings

b5e211f

Fix interpolated strings

58bfb83

Test refactoring

392c55e

Checkpoint: Add more test coverage for bindings and interop

6267d66

Checkpoint

506b4f0

Checkpoint

0c9f9d7

Checkpoint: Enum marshaling works

4f33972

Improve test coverage

f9bc6b5

Checkpoint: Unify unboxing of primitive types

a27fefd

Checkpoint: Unify unboxing of primitive types

f9f5f89

Checkpoint: Restore fn to satisfy runtime-test.js

9081ffd

Checkpoint: Unify boxing for primitives

1e0ba4d

Checkpoint

d24c02b

Remove now-unused box methods

215cdd2

Don't store names for null method IDs

dcdb16a

Fix indentation damage

fcf398a

Add test

5e9cff3

Satisfy CI

2806916

Accept weaker promises

635aa03

lewing force-pushed the bindings-optimizations-1 branch from 0eb7a06 to 635aa03 Compare November 11, 2020 01:43

lewing merged commit 85cb1df into dotnet:master Nov 11, 2020

ghost locked as resolved and limited conversation to collaborators Dec 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WASM Bindings optimizations and fixes #41808

WASM Bindings optimizations and fixes #41808

kg commented Sep 3, 2020 •

edited

Loading

kg commented Sep 12, 2020

marek-safar Sep 14, 2020

marek-safar Sep 14, 2020

kg Sep 14, 2020

kg Sep 17, 2020

marek-safar Sep 19, 2020

marek-safar Sep 14, 2020

kg Sep 14, 2020

lewing Oct 15, 2020

kg commented Sep 19, 2020

lewing commented Oct 13, 2020 •

edited

Loading

lewing left a comment

lewing Oct 15, 2020

kg Oct 15, 2020

lewing Oct 15, 2020

lewing Oct 15, 2020

lewing commented Nov 11, 2020

WASM Bindings optimizations and fixes #41808

WASM Bindings optimizations and fixes #41808

Conversation

kg commented Sep 3, 2020 • edited Loading

kg commented Sep 12, 2020

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

kg commented Sep 19, 2020

lewing commented Oct 13, 2020 • edited Loading

lewing 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

lewing commented Nov 11, 2020

kg commented Sep 3, 2020 •

edited

Loading

lewing commented Oct 13, 2020 •

edited

Loading