1581572 - Provide jit paths for js <-> wasm calls that pass or return anyref or funcref

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P3)

Description

[Lars T Hansen [:lth]]

Attachment: nongc.tar

The background here is https://github.com/WebAssembly/gc/issues/71, which reports that Firefox is more than 10x slower than Chrome on a polyfill for the GC functionality. I have a test case which I will attach (run simple.py to serve nongc.html on localhost:8000). This shows Firefox more than 3x slower than Chrome on a simple test; more investigation needed; this could be anything and need not even be anyref-related.

For Chrome (Canary?) you have to start it with --js-flags="--experimental-wasm-anyref"; for Firefox you probably have to use Nightly, where reftypes should be enabled by default.

Comment 1

[Lars T Hansen [:lth]]

Volker, in reference to the github issue you posted about the gc polyfill, I don't see the > 10x slowdown in firefox relative to chrome that you reported there. Are you willing to share the code you ran to get this result and more information about your environment (OS, hardware, methodology)? An attachment on this bug is fine (or email directly to me if you don't want the code to be public). Thanks.

Comment 2

[Volker Berlin]

Attachment: bench.zip

I hope I have not do some mistake, for example with some command line flags. I forgot ever that I set many flags in my environment. I will try to give you the needed information.

My Java code look like:

        @Export
        static double run() {
            int[] a = new int[1000_000];
            double time = currentTimeMillis();
            int i;
            for( i = 0; i < a.length; i++ ) {
                a[i] = i;
            }
            return currentTimeMillis() - time;
        }

        @Import( module = "nongc", name = "currentTimeMillis", js = "() => Date.now()")
        static double currentTimeMillis() {
            return 0;
        }

I think this should be 2 mio calls to import functions. I measure the time inside wasm to remove the time of load overhead.

With SpiderMonkey this needs 250-270 milliseconds.
With node (v8) this needs 20-22 milliseconds.

I run it on Windows 10. I use https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/jsshell-win64.zip from today.
For V8 I use node v12.4.0 with V8 version 7.4.288.27-node.18. My Hardware is intel i5 with 1.80 GHz.

Command lines:

• js --wasm-gc wasm9190323946455869402SpiderMonkeyTest.js
• cmd /C node --experimental-wasm-mv --experimental-wasm-se --experimental-wasm-sat-f2i-conversions --experimental-wasm-eh --experimental-wasm-anyref --experimental-wasm-bigint wasm8495614166900481792nodetest.js
• cmd /C node --experimental-wasm-mv --experimental-wasm-se --experimental-wasm-sat-f2i-conversions --experimental-wasm-eh --experimental-wasm-anyref --experimental-wasm-bigint wasm4451452015083876476WatTest.js
• js --wasm-gc C:\Users\Volker\AppData\Local\Temp\junit6526293398485513149\wasm4049694636877473615SpiderMonkeyWatTest.js
• cmd /C node --experimental-wasm-mv --experimental-wasm-se --experimental-wasm-sat-f2i-conversions --experimental-wasm-eh --experimental-wasm-anyref --experimental-wasm-bigint wasm1904334429783223594nodetest.js

Scripte and binary are attached. Do you need any else?

Comment 3

[Lars T Hansen [:lth]]

Thanks! I'll try to get back to this within the next week or so.

Comment 4

[Lars T Hansen [:lth]]

Looking at the command lines, --wasm-gc disables the optimizing compiler in the SpiderMonkey shell (https://searchfox.org/mozilla-central/source/js/src/wasm/WasmCompile.cpp#104). So that's probably the reason why you're seeing such slowdowns. I think for your polyfill -- given that it is a polyfill -- that switch should not be needed, since the reftypes proposal is enabled by default in the shell.

Comment 5

[Volker Berlin]

In the meantime I have already try it without the flag --wasm-gc. Yes, this make some thinks faster but this test case was not faster. Seems that the call of import functions is identical optimized.

I have also try many other flags which make the things only slower. Only a switch for spectre make it 5% faster. But this such small that this can be a random effect.

Comment 6

[Lars T Hansen [:lth]]

Thanks, that's helpful. I'll pull it up in a profiler and see what I can learn.

Comment 7

[Lars T Hansen [:lth]]

OK, I can confirm this discrepancy with my tiny test case using node v12 vs the spidermonkey shell on x64 linux: 19ms in node, 145ms in the spidermonkey shell. The same is the time in the sm shell whether i'm using ion or baseline, so the running time is unsurprisingly dominated by the calls to the imports.

These imports are super-lightweight, so the time is probably dominated by the wasm->js boundary. The fact that something is an arrow function vs a normal function does not seem to matter, for example.

Comment 8

[Volker Berlin]

Thank you for updating me.

Comment 9

[Lars T Hansen [:lth]]

Benjamin says "Could it be because there's no fast JIT entry/exit, since there are anyref in the signature?" and I think this sounds very plausible, so something Ryan can look into when he has time. I'll attach my updated test archive along with an adaptation to nodejs. Note the test with nodejs requires version 12 -- version 10 crashes in the most awful way.

Comment 10

[Lars T Hansen [:lth]]

Attachment: nongc.tar

For a spidermonkey release shell, no options are needed (though one can always run with --wasm-compiler=ion), the test case is nongc.js.

For node 12, run with --experimental-wasm-anyref nongc-node.js

Comment 11

[Lars T Hansen [:lth]]

Almost certainly related to bug 1508560 ("remaining stubs work").

Comment 12

[Lars T Hansen [:lth]]

Also see https://bugzilla.mozilla.org/show_bug.cgi?id=1532556#c1.

Comment 13

[Lars T Hansen [:lth]]

Attachment: Bug 1581572 - Allow AnyRef on JS/wasm fast paths, part 1. r=rhunt,bbouvier

An AnyRef is an opaque pointer type to wasm but can hold any JS value;
the value is boxed when it enters wasm and unboxed when it enters JS.
This boxing and unboxing must also happen when wasm and JS interact
along the fast ("Jit") path, not just through the slow ("Interpreter")
path.

Comment 14

[Lars T Hansen [:lth]]

I should add that the offered patch fixes the problem reported in comment 0; Firefox time is now down to the same time as Node for the test case, because we get the fast jit path on the call to the import if the import accepts an anyref. Additionally, the patch allows a js->wasm call to be inlined if the wasm function returns an anyref. These two cases are common because a boxed wasm-side anyref can be unboxed into a js-side Value.

The other case, where a JS value may have to be boxed to a wasm-side anyref going into wasm as an argument on a JS->wasm call on as a return value on a wasm->JS call, will be a second patch.

(Separately, there's optimization work to be done to avoid boxing string values, see bug 1532556.)

Comment 15

[Lars T Hansen [:lth]]

Attachment: Bug 1581572 - Allow AnyRef on JS/wasm fast paths, part 2. r=rhunt,bbouvier

WIP. This allows an imported function (an exit) that returns anyref
to have a fast path; the Value that we receive from the import is
boxed into the wasm representation, which means allocating an object
for it if the wasm representation can't accomodate it directly
(currently only object or null).

Depends on D49396

Comment 16

[Lars T Hansen [:lth]]

Latest revisions remove the changes to support the inlined JS->wasm fast path; I want to handle all of those in a separate patch.

Comment 17

[Lars T Hansen [:lth]]

Attachment: Bug 1581572 - Allow AnyRef on JS/wasm fast paths, part 3. r=rhunt,bbouvier

Handle argument passing and returns for the inlined JS->wasm fast path.

Depends on D49728

Comment 18

[Lars T Hansen [:lth]]

Attachment: Bug 1581572 - Allow FuncRef on JS/wasm fast paths. r=rhunt

Generalizes the work on AnyRef to also allow FuncRef values to flow
out to JS through parameters on exits to JS or return values on
entries from JS.

Depends on D50031

Comment 19

[Lars T Hansen [:lth]]

Hmph, I was testing more broadly and I see there are some 64-bit-isms here. Updated patches coming, by and by.

Comment 20

[Lars T Hansen [:lth]]

Hm, it could look like visitWasmAnyRefFromJSObject() needs to create a safepoint, but it does not...

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3018975e696
Allow AnyRef on JS/wasm fast paths, part 1. r=rhunt,bbouvier
https://hg.mozilla.org/integration/autoland/rev/b1796f474af4
Allow AnyRef on JS/wasm fast paths, part 2. r=rhunt,bbouvier
https://hg.mozilla.org/integration/autoland/rev/4ba83138d78e
Allow AnyRef on JS/wasm fast paths, part 3. r=rhunt,bbouvier
https://hg.mozilla.org/integration/autoland/rev/b50479e36a54
Allow FuncRef on JS/wasm fast paths. r=rhunt

Comment 22

[Dorel Luca [:dluca]]

Backed out 6 changesets (bug 1581572, bug 1596026) for Spidermonkey failure in js/src/wasm/WasmStubs.cpp. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=276957819&repo=autoland

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=b50479e36a54536e902079df79efe42f12f63937

Backout:
https://hg.mozilla.org/integration/autoland/rev/522722c8c0e2ca314f00bff3f64398b237eb1385

Comment 23

[Lars T Hansen [:lth]]

Groan, why is this code even being compiled for nojit...

Comment 24

[Lars T Hansen [:lth]]

Problem with nojit only, needed stubs in masm.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37323f3b5d16
Allow AnyRef on JS/wasm fast paths, part 1. r=rhunt,bbouvier
https://hg.mozilla.org/integration/autoland/rev/636bc79ab015
Allow AnyRef on JS/wasm fast paths, part 2. r=rhunt,bbouvier
https://hg.mozilla.org/integration/autoland/rev/ee84a26cfc8f
Allow AnyRef on JS/wasm fast paths, part 3. r=rhunt,bbouvier
https://hg.mozilla.org/integration/autoland/rev/ef5aaf41caf7
Allow FuncRef on JS/wasm fast paths. r=rhunt

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/37323f3b5d16
https://hg.mozilla.org/mozilla-central/rev/636bc79ab015
https://hg.mozilla.org/mozilla-central/rev/ee84a26cfc8f
https://hg.mozilla.org/mozilla-central/rev/ef5aaf41caf7

Comment 27

[Volker Berlin]

Great, my original test is now a little faster than with Node/V8.

Do you interested on more performance bottleneck compared to V8 in new bug tickets?

Comment 28

[Lars T Hansen [:lth]]

(In reply to Volker Berlin from comment #27)

Do you interested on more performance bottleneck compared to V8 in new bug tickets?

Yes, always :-)

Comment 29

[Mu Tao]

Attachment: Bug 1581572 - [mips] Allow AnyRef on JS/wasm fast paths, part 1. r?lth

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9069638dadb5
[mips] Allow AnyRef on JS/wasm fast paths, part 1. r=lth

Comment 31

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/9069638dadb5