-
Notifications
You must be signed in to change notification settings - Fork 524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Suggestion] [sokol_app] proxy javascript function accessing DOM to the main thread #999
base: master
Are you sure you want to change the base?
Conversation
## What does this PR do This commit changes the javascript defined by sokol to allow proxying them when necessary and enable the use of the proxy_to_pthread option. Specificallt, it replaces a few call to the `EM_JS` macro to the `EM_JS_MAIN_THREAD` ## What problem does it solve? When compiled with `proxy_to_pthread` ([generally recommended](https://emscripten.org/docs/porting/pthreads.html#blocking-on-the-main-browser-thread) by emscripten documentation), the "main" app thread is running separately from the browser event loop. This guarantee the browser won't become unresponsive because of a deadlock. This option is particularly convenient when porting code that might block for any event on the main thread. With this option, the main app thread is not able to access a few javascript features such as the `window` object. The solution for that is called [proxying]()https://emscripten.org/docs/porting/pthreads.html#proxying), where javascript function calls are move to the main thread and proxied on the worker thread. # Potential caveat Since there is no utility `EM_JS_MAIN_THREAD` in emscripten (that would define a javascript function callable from c++ proxied to main), I had to define the corresponding function in C and make them call inline asm via the `MAIN_THREAD_EM_ASM`. This means that the generated function in the javascript side won't be named with the name defined in C, instead they will have random name. Hence, if those function were intended to be called from the javascript side, this PR would be a problem. I don't see any call like this so far, but the fact that they were not profixed with an underscore as other internal function make me doubt.
Hmm, at first glance this change is a bit too radical for my liking. Do you know if MAIN_THREAD_EM_ASM intoduces overhead if the C side is already running on the main thread? |
I suspected this would require a bit more discussion, hence why I kept this PR for the end. :) I think the ability to run with the Unfortunately, I don't have good answer for the overhead, and did not find anything on the emscripten github repo before posting the PR. I find the emscripten comment on // Runs the given JavaScript code synchronously on the main browser thread, and returns no value back.
// Call this function for example to access DOM elements in a pthread when building with -s USE_PTHREADS=1.
// Avoid calling this function in performance sensitive code, because this will effectively sleep the
// calling thread until the main browser thread is able to service the proxied function call. If you have
// multiple MAIN_THREAD_EM_ASM() code blocks to call in succession, it will likely be much faster to
// coalesce all the calls to a single MAIN_THREAD_EM_ASM() block. If you do not need synchronization nor
// a return value back, consider using the function MAIN_THREAD_ASYNC_EM_ASM() instead, which will not block.
// In single-threaded builds (including proxy-to-worker), MAIN_THREAD_EM_ASM*()
// functions are direct aliases to the corresponding EM_ASM*() family of functions.
#define MAIN_THREAD_EM_ASM(code, ...) Another informative bit is looking at the generated javascript for the inline ASM call:
So, In term of overhead I see two potential sources:
I think if 2 is a problem, another option could be to detect and dispatch on the C side instead and avoid inline ASM: EM_JS(void, cfunc_impl, (void));
void cfunc(void) {
if (!emscripten_is_main_runtime_thread()) {
cfunc_impl();
} else {
emscripten_sync_run_in_main_runtime_thread(SIG, cfunc_impl, args)
}
} but honestly I don't know enough about the internal of emscripten and javascript to see if that could lead to a measurable difference overhead. Note that I realized my PR is incomplete, but my guess was that the functions needing to be changed were not performance sensitive:
although my current implementation is likely not the most efficient and could be improved either with |
This is the PR mentioned in #997 and is the last change I had locally that I would like to suggest upstream.
What does this PR do?
This commit changes the javascript functions defined by sokol_app.h to allow proxying them to the main thread when necessary and enable the use of the emscripten
proxy_to_pthread
option. Specifically, it replaces a few call to theEM_JS
macro to theEM_JS_MAIN_THREAD
What problem does it solve?
When compiled with
proxy_to_pthread
(generally recommended by emscripten documentation), the "main" app thread is running separately from the browser event loop. This guarantee the browser won't become unresponsive because of a deadlock. This option is particularly convenient when porting code that might block for any event on the main thread.With this option, the main app thread is not able to access a few javascript features such as the DOM
window
object. The emscripten solution for that is called proxyinghttps://emscripten.org/docs/porting/pthreads.html#proxying), where javascript function calls are moved to the browser event loop and the return value eventually sent back to the worker thread.Potential caveat
Since there is no utility
EM_JS_MAIN_THREAD
in emscripten (that would define a javascript function callable from c++ proxied to main), I had to define the corresponding function in pure C and make them call inline asm via theMAIN_THREAD_EM_ASM
.Example of such a change:
becomes
This means that the generated function in the javascript side won't be named with the name defined in C, instead they will have random name.
Hence, if those function were intended to be called from the javascript side, this PR would be a problem. I don't see any call like this so far, but the fact that they were not prefixed with an underscore, as other internal function, make me doubt.