Support fiber API with JSPI - #27638
Conversation
|
I'm kind of surprised we only have one test for this. I'm guessing this is not very widely used. |
sbc100
left a comment
There was a problem hiding this comment.
Very impressive how seemingly simple this is.
|
@Akaricchi are you still using fibers in emscripten? |
Yes, they are fundamental for Taisei Project's web port; in particular they are used by koishi as a coroutine backend. Koishi has a basic test, but it's known to be insufficient. I'll give this PR a try later. If you want to test it yourself, you can check Taisei's github actions workflows to see how to build it for emscripten. Coroutines are only used by the game logic right now, so you'd actually have to play for a bit (or idle in the main menu to trigger demo playback; mind that those desync on the master branch). |
|
@Akaricchi I remember back when you added fibre support you were pretty vigilant about getting max performance. I would be curious if you get a chance if you could confirm if the JSPI version is fast (hopefully it is) for you, and by how much? |
|
Sure, I'll run some benchmarks when/if I get this to work with Taisei. The performance of the current asyncify-based version is objectively pretty damn bad when compared to native, but it ended up being acceptable for Taisei. I expect this to be fine too, unless it does something egregious like yielding to the browser event loop for every swap. The code seemed fine at a glance, but I don't know the specifics of how JSPI works. |
JSPI does require a micro-task for every swap, that is kind of fundamentally how it works. This is not the same things as the full browser event loop though, and we would expect it to be cheaper overall than ASYNCIFY (which has its own runtime and code size impacts). |
|
|
||
| swap(oldFiber, newFiber) { | ||
| return new Promise((resolve) => { | ||
| Fibers.fiberResolvers.set(oldFiber, resolve); |
There was a problem hiding this comment.
I'm not sure I like this dependency on the fiber struct's address. The docs state:
This structure represents a Fiber context continuation. The runtime does not keep references to these objects, they only contain information needed to perform the context switch. The switch operation updates some of the contents, however.
and that is true for the asyncify version. So it's possible to, e.g. realloc() an array of fibers without breaking anything. Perhaps you can fix this by reusing the rewind_id field of asyncify_data_t (embedded into emscripten_fiber_t), e.g. allocate an integer handle for each resolve and associate that instead of the address.
What happens when a fiber is discarded and never resumed though? Is there a zombie entry stuck in the map then?
There was a problem hiding this comment.
I've updated the pr to use an id tied to the fiber.
What happens when a fiber is discarded and never resumed though? Is there a zombie entry stuck in the map then?
Yes, currently if a suspended fiber is abandoned and never resumed, its resolver remains in Fibers.fiberResolvers and its WebAssembly continuation stays suspended in the engine.
For JSPI, we could add a emscripten_fiber_destroy or something to handle this case.
There was a problem hiding this comment.
Yeah this needs to be addressed.
Taisei keeps a permanent pool of reusable tasks around. Each task contains a fiber struct and an allocated stack. When a running task ends (either naturally by returning from its entry point, or cancelled by something else while suspended), it's simply marked as dead and added to the freelist. Eventually it will be reused when a new task needs to run, this code is called to re-initialize the fiber. Basically, it just assigns a new entry point for the fiber before swapping to it. I believe that's where the old continuation would leak.
You can pretty easily detect this. When swapping to a fiber that has an entry point set, check if it has a non-zero rewind_id, and if so, remove its entry from the map. That would fix Taisei (and similar pooling strategies) without client code changes.
But you'd probably still need a cleanup function for code that doesn't pool fibers like that, or calls emscripten_fiber_init to re-init them, or needs to be able to clean up its fibers completely. Note that doing a similar check in emscripten_fiber_init is unsafe, as it may be called on uninitialized memory.
The fiber API currently only supports Asyncify. JSPI allows suspending and resuming WebAssembly execution via native stack switching without the code size and performance overhead of Asyncify bytecode instrumentation. Mark asyncify_stack parameters as _Nullable in fiber.h since an Asyncify stack buffer is unnecessary under JSPI. Update the fiber test to run under both Asyncify and JSPI.
80f1f58 to
e4e0ea0
Compare
e4e0ea0 to
97d9951
Compare
The fiber API currently only supports Asyncify. JSPI allows suspending and resuming WebAssembly execution via native stack switching without the code size and performance overhead of Asyncify bytecode instrumentation.
Mark asyncify_stack parameters as _Nullable in fiber.h since an Asyncify stack buffer is unnecessary under JSPI.
Update the fiber test to run under both Asyncify and JSPI.