emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.73k stars 3.3k forks source link

docs: Improve Asyncify documentation about ignore-indirect & fiber.h #11681

Open stelcheck opened 4 years ago

stelcheck commented 4 years ago

Edit: I've also set SDL_SetHint(SDL_HINT_EMSCRIPTEN_ASYNCIFY, "0"); to try to disable other potential sources of asynchronous events.

Ref: https://github.com/Wizcorp/byuu-web/blob/master/libco/emscripten_fiber.c

I've been working on the project above for a few months, and early on in the process we've tried to refactor some of the code to avoid context switching overall as it appeared to have an unacceptable overhead. However, each core we add requires tedious porting work, work I'm eager to get rid of if possible.

I've recently read that said overhead can be mitigated by being more specific about what functions can unwind the stack; therefore, I've experimented with using build flags such as -s ASYNCIFY_IGNORE_INDIRECT, but unfortunately I've only been met with exception thrown: RuntimeError: unreachable so far (the reported method where the error occurs appear to differ based on what I feed in ASYNCIFY_IMPORTS/ASYNCIFY_ADD - however, none of those function hold async code).

I have one other place in the code where we use async code, but I am not currently using this code path; as it stand, only the fiber.h API (and perhaps emscripten_set_main_loop?) should be calling code asynchronously.

My questions would be as follow:

  1. Does it make sense to try to optimize the above referred code through ASYNCIFY_IGNORE_INDIRECT or other means?
  2. If the answer to the above is yes, what would be the best way to go about it?
  3. Are there any caveats to using emscripten_set_main_loop which would block the kind of optimizations I'm attempting?
kripken commented 4 years ago

You should be able to use ignore-indirect if you can add all the necessary functions to the list. That includes emscripten_set_main_loop - you need to add the main loop function to the list as well. In general, if X calls Y indirectly, and if you manually add both X and Y to the list, then that call will work.

You can find which functions are needed by looking at a stack trace when a sleep occurs (in the browser debugger, or manually adding one, etc.).

stelcheck commented 4 years ago

It looks like emscripten_set_main_loop is already added to imports, so I assume nothing else needs to be done there.

By logging a stacktrace on sleep, I assume you mean at the moment of calling asynchronous code (say, emscripten_fiber_swap)?

stelcheck commented 4 years ago

I've tried to add all the functions in the error stack to ASYNCIFY_ADD, but now I'm at a point where https://github.com/emscripten-core/emscripten/blob/8715a1bb4a6b614e3de6b842f8e57e054c2ba265/src/library_async.js#L631 is where the error is being reported. I'm assuming I must be missing something.

Edit: Looks like adding dynCall_* made things move further.

kripken commented 4 years ago

By logging a stacktrace on sleep, I assume you mean at the moment of calling asynchronous code (say, emscripten_fiber_swap)?

Yes.

And yes, dynCall functions need to be added also - they are just normal functions from asyncify's point of view. Perhaps we could improve the docs there?

stelcheck commented 4 years ago

It would definitely help. It might also help to provide more explicit/formatted output with async assert/async debug; with some more direct feedback in debug mode, it would probably help keep the documentation succinct on the subject.

kripken commented 4 years ago

Sounds good, I renamed the issue to focus on improving the docs, if you or someone else can make a PR that would be great!

If you don't know exactly what to add in the docs, a draft PR with a sketch would be good too, and we can help improve it.

stelcheck commented 4 years ago

I'd be happy to tackle this.

How about:

  1. One paragraph with a code sample and an error stack sample to describe how to use the information (probably recommend using --profiling on compile too?)
  2. One paragraph elaborating on the pseudo-calls to be added (dynCall_*, anything else?)
  3. Linkbacks where required (I don't think there is anything specific to add for fiber.h, or any other async API for that matter, but let me know if there are such cases)
kripken commented 4 years ago

Sounds good @stelcheck !

curiousdannii commented 4 years ago

There's no ADVISE mode like there was for EMTERPRETIFY?

kripken commented 4 years ago

@curiousdannii no, but the lists options are more flexible, so maybe it's not needed?

curiousdannii commented 4 years ago

Not knowing the internals, it does sound more flexible, but on the other hand if you've ever had to recommend adding a stack trace then your new verbose option at https://github.com/WebAssembly/binaryen/pull/3022 sounds like it will be useful.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.