emscripten icon indicating copy to clipboard operation
emscripten copied to clipboard

emscripten_request_animation_frame_loop(), emscripten_set_main_loop(), emscripten_set_timeout() etc. are not compatible with JSPI

Open juj opened this issue 1 year ago • 11 comments
trafficstars

I'm taking a peek at my WebGPU bindings JSPI support test, and I find that after updating to latest Emscripten, I now start getting this error:

Uncaught RuntimeError: attempting to suspend without a WebAssembly.promising export

image

I wonder if something might have changed in the JSPI implementation since an earlier Emscripten version, that my code is now out of date?

Here is how I handle JSPI:

https://github.com/juj/wasm_webgpu/blob/16bbfed542979a2f15f789cd0461cce7bb30e0e6/lib/lib_webgpu.js#L1427-L1449

i.e. I use a pattern

lib_webgpu.js
  wgpu_buffer_map_sync__sig: 'ii',
  wgpu_buffer_map_sync__async: true,
  wgpu_buffer_map_sync: function(buffer) {
    return Asyncify.handleAsync(() => {
      return wgpu[buffer].mapAsync(); // .mapAsync() returns a promise
  },

This used to work with both Asyncify and JSPI at some point - but maybe the syntax, or something else has changed?

It's been a while since I followed this feature. I tried to search https://emscripten.org/docs/porting/asyncify.html for documentation example of how to use Asyncify.handleAsync(), though there is only a EM_JS() based example, which I don't want to use. So I wonder if that changes anything, or if the code should work identically as a JS lib?

juj avatar Sep 03 '24 18:09 juj

A SINGLE_FILE build expressing the failure can be seen in https://oummg.com/dump/buffer_map_sync.html

juj avatar Sep 03 '24 18:09 juj

What is odd here is that the error occurs in the next run of the requestAnimationFrame event loop..

juj avatar Sep 03 '24 18:09 juj

Ah actually it is not the next run of the requestAnimationFrame event loop, but the current call.

So I need the WebAssembly.promising() call. I find this patch fixes the issue:

diff --git a/src/library_html5.js b/src/library_html5.js
index 4e73df86c..3b1c899d2 100644
--- a/src/library_html5.js
+++ b/src/library_html5.js
@@ -2472,7 +2472,11 @@ var LibraryHTML5 = {

   emscripten_request_animation_frame_loop: (cb, userData) => {
     function tick(timeStamp) {
+#if JSPI
+      if (WebAssembly.promising({{{ makeDynCall('idp', 'cb') }}})(timeStamp, userData)) {
+#else
       if ({{{ makeDynCall('idp', 'cb') }}}(timeStamp, userData)) {
+#endif
         requestAnimationFrame(tick);
       }
     }

What is the overhead of a WebAssembly.promising()? I ponder if we should add the above patch to all our settimeouts/setintervals/requestAnimationFrame/set_main_loop calls(), or even go as far as to add it to all {{{ makeDynCall() }}} calls?

Although I recall there are some nesting problems with mixed stacks of Wasm and JS calll frames, where JS -> Wasm -> JS -> Wasm -> JS calls need some kind of manual unwinding strategy to fully yield back from innermost wasm call to outermost JS scope?

juj avatar Sep 03 '24 19:09 juj

For now I removed use of emscripten_set_animation_frame_loop() as it is incompatible with JSPI. In wasm_webgpu bindings I added a custom function wgpu_request_animation_frame() that works in a manner that is JSPI-aware, and holds rAF()s until JSPI suspend is finished.

I wonder if we will want to do something like that for all top level event handler functions.. or not quite sure what would be the best general strategy to handle this - this feels a bit challenging requirement for the user to need to know if their code execution might lead to a JSPI suspend before calling into Wasm? Would we want to build a double API set for JSPI vs non-JSPI functions?

juj avatar Sep 03 '24 19:09 juj

I observe that calling WebAssembly.promising() generates temporary garbage, so to avoid calling that function each frame to wrap the same function pointer, I wanted to optimize the wrapping to only occur once, to avoid runaway JS garbage.

In https://github.com/juj/wasm_webgpu/commit/a363e1a39440fcd522d95b3ee9768390d6a7fce1

i.e.

https://github.com/juj/wasm_webgpu/blob/a363e1a39440fcd522d95b3ee9768390d6a7fce1/lib/lib_webgpu.js#L219-L229

I switched to a mechanism that would achieve that, so WebAssembly.promising() is called only once for the whole rAF loop.

However, oddly, after that the animation that it results in only works for some number of seconds, before breaking in that same error:

image

I've uploaded a test build of the above code at https://oummg.com/dump/buffer_map_sync_breaks_after_a_few_seconds.html

On my system it successfully animates for some number of seconds, before pausing in an exception.

Is there a restriction that the function object returned by WebAssembly.promising() may only be called once/be involved only in one suspend/resume event?

juj avatar Sep 03 '24 20:09 juj

The more I poke at https://oummg.com/dump/buffer_map_sync_breaks_after_a_few_seconds.html RuntimeError, I get a feeling like that would be a browser bug. I can't see any reason why execution of such a requestAnimationFrame loop should spontaneously stop like that.

juj avatar Sep 04 '24 18:09 juj

To respond to the original issue, JSPI has changed behind the scenes, but I'm not aware of anything that would have caused your example to break if it was working before with JSPI.

Is there a restriction that the function object returned by WebAssembly.promising() may only be called once/be involved only in one suspend/resume event?

No, you should be able to call it as many times as you want. FWIW, on my macbook m1 w/ chrome 128 I have no issue with your demo above.

brendandahl avatar Sep 04 '24 19:09 brendandahl

I reported the bug in Chromium bug tracker at https://issues.chromium.org/issues/364667545.

The difference between the working and failing runs is as small as

image

juj avatar Sep 04 '24 22:09 juj

Revised the title of this bug to focus on the Emscripten side challenge, namely that none of the callback-taking functions work with JSPI, unless one does a custom wrapper like e.g. here:

https://github.com/juj/wasm_webgpu/blob/280bbcfcef90b3ea6a515eeae8414b45aedabdeb/lib/lib_webgpu.js#L203-L220

juj avatar Sep 04 '24 22:09 juj

I've run into this a number of other places (e.g. starting threads). With JSPI we basically need to add wrappers around most entry points into wasm. Adding the wrapper everywhere by default won't work since a lot of code is not expecting a promise return value.

It still needs some work, but I'm hoping that we can add an annotation to functions that need the JSPI wrapper. Alternatively, we could make a preprocessor similar to makeDynCall that will auto add the wrapper when built with JSPI.

brendandahl avatar Sep 04 '24 22:09 brendandahl

makeSuspendingDynCall?

sbc100 avatar Sep 04 '24 23:09 sbc100

Found this issue based on a report on Matrix of the same issue. The error message is slightly different so I'll paste it here for searchability:

Uncaught SuspendError: trying to suspend without WebAssembly.promising
    at hello.wasm.legalfunc$emwgpuWaitAny (http://localhost:8081/out/webdbg-sanitizers/hello.wasm:wasm-function[2669]:0x16103d)

Based on @juj's workaround (link to latest version) I came up with an EM_JS version that works on the app side. It also fixes what looks like a bug in that code - the wrapped version of cb returns a Promise but it doesn't await before testing the return value (and Promise is always truthy). I think this also conveniently removes the need for _wgpuNumAsyncifiedOperationsPending, which I'm not sure is the right logic anyway? (It seems to block the main loop from running while any webgpu operation is pending, but in principle we only really care about the ones that are actually waited in the frame function - which is what await will wait for.)

Here's my solution. It works at least for my basic test case (wgpuInstanceWaitAny on a wgpuBufferMapAsync operation). https://github.com/kainino0x/webgpu-cross-platform-demo/blob/f5c69c6fccbb2584c1b6f9e559f9a41a38a9b5ad/main.cpp#L692-L704 (EDIT: updated link to a version where I fixed a related bug)

kainino0x avatar Jan 10 '25 01:01 kainino0x

Here is a not-so-generalized version that works on Asyncify in addition to JSPI: https://github.com/kainino0x/webgpu-cross-platform-demo/blob/c26ea3e29ed9f73f9b39bddf7964b482ce3c6964/main.cpp#L737-L758

kainino0x avatar Jan 10 '25 08:01 kainino0x

Here is a not-so-generalized version that works on Asyncify in addition to JSPI:

For the record, on Asyncify(=1), usually manifests as the loop just stopping when the callback suspends for the first time. (It returns 0 when it suspends, which is falsy, telling it to stop looping. I understood this at some point but I don't remember anymore why it returns 0.) However, this error from #24154 may have the same root cause. I'm not sure exactly what causes it.

my_app.html:46 [my_app] Calling wgpuInstanceWaitAny for wgpuBufferMapAsync future with timeout 1'500'000'000ull...
my_app.html:1 Uncaught (in promise) unwind

kainino0x avatar Apr 29 '25 02:04 kainino0x

However, this error from #22493 may have the same root cause.

Did you mean #24154?

fridenmf avatar Apr 29 '25 05:04 fridenmf

Oops, yes :)

kainino0x avatar Apr 30 '25 00:04 kainino0x