Skip to content

[JSPI] Automatically add promising JSPI wrappers to thread entry points. #22891

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 13, 2024
Merged

[JSPI] Automatically add promising JSPI wrappers to thread entry points. #22891

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7bca7aa
[JSPI] Automatically add promising JSPI wrappers to thread entry points.
brendandahl Nov 8, 2024
231c37d
Update test/core/test_pthread_join_and_asyncify.c
brendandahl Nov 12, 2024
f0ad577
Merge branch 'main' into jspi-proxy-to-pthread
brendandahl Nov 12, 2024
fa0b352
Add option to generate async dyncall to fix wasm64.
brendandahl Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/library_pthread.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,7 @@ var LibraryPThread = {
'$runtimeKeepaliveCounter',
#endif
],
$invokeEntryPoint: (ptr, arg) => {
$invokeEntryPoint: {{{ asyncIf(ASYNCIFY == 2) }}} (ptr, arg) => {
#if PTHREADS_DEBUG
dbg(`invokeEntryPoint: ${ptrToString(ptr)}`);
#endif
Expand Down Expand Up @@ -1079,7 +1079,11 @@ var LibraryPThread = {
// *ThreadMain(void *arg) form, or try linking with the Emscripten linker
// flag -sEMULATE_FUNCTION_POINTER_CASTS to add in emulation for this x86
// ABI extension.
#if ASYNCIFY == 2
var result = WebAssembly.promising({{{ makeDynCall('pp', 'ptr') }}})(arg);
#else
var result = {{{ makeDynCall('pp', 'ptr') }}}(arg);
#endif
#if STACK_OVERFLOW_CHECK
checkStackCookie();
#endif
Expand All @@ -1098,10 +1102,9 @@ var LibraryPThread = {
#endif
}
#if ASYNCIFY == 2
Promise.resolve(result).then(finish);
#else
finish(result);
result = await result;
#endif
finish(result);
},

#if MAIN_MODULE
Expand Down
4 changes: 2 additions & 2 deletions src/runtime_pthread.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ if (ENVIRONMENT_IS_PTHREAD) {
// notified about them.
self.onunhandledrejection = (e) => { throw e.reason || e; };

function handleMessage(e) {
{{{ asyncIf(ASYNCIFY == 2) }}} function handleMessage(e) {
Copy link
Collaborator

@sbc100 sbc100 Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like these fixes would apply to JSPI using in the main function of any pthread, not just the PROXY_TO_PTHREAD mode. Is there something specific to PROXY_TO_PTHREAD here?

Copy link
Collaborator Author

@brendandahl brendandahl Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's not totally specific to PROXY_TO_PTHREAD, more it showed up as an issue there since the entry point for the main proxy is not the users actual main.

With a user defined entry point there were ways of handling this (see core.test_pthread_join_and_asyncify). I think I can actually change that test now to not require the user to specify that the entry function is JSPI. Hower, this makes me wonder if we should automatically JSPI wrap thread entry functions or require the user to specify that they need JSPI. FWIW, we currently auto JSPI wrap main.

Copy link
Collaborator

@sbc100 sbc100 Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So an alternative solution to this one would be to auto-wrap _main_thread which is used in _emscripten_proxy_main ?

If the clang annotation was working we could use that there?

Copy link
Collaborator Author

@brendandahl brendandahl Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So an alternative solution to this one would be to auto-wrap _main_thread which is used in _emscripten_proxy_main ?

Yeah, we'd need EMSCRIPTEN_KEEPALIVE or add it to EXPORTED_FUNCTIONS and also add it to DEFAULT_ASYNCIFY_EXPORTS.

If the clang annotation was working we could use that there?

Yeah, I still need to fix binaryen to handle the annotation.

try {
var msgData = e['data'];
//dbg('msgData: ' + Object.keys(msgData));
Expand Down Expand Up @@ -201,7 +201,7 @@ if (ENVIRONMENT_IS_PTHREAD) {
}

try {
invokeEntryPoint(msgData.start_routine, msgData.arg);
{{{ awaitIf(ASYNCIFY == 2) }}} invokeEntryPoint(msgData.start_routine, msgData.arg);
} catch(ex) {
if (ex != 'unwind') {
// The pthread "crashed". Do not call `_emscripten_thread_exit` (which
Expand Down
6 changes: 4 additions & 2 deletions test/core/test_pthread_join_and_asyncify.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ EM_ASYNC_JS(int, async_call, (), {
return 42;
});

// TODO Remove EMSCRIPTEN_KEEPALIVE when support for async attributes is enabled.
EMSCRIPTEN_KEEPALIVE void *run_thread(void *args) {
void *run_thread(void *args) {
int ret = async_call();
assert(ret == 42);
return NULL;
}

int main() {
pthread_t id;
// Test that JSPI works on main thread.
emscripten_sleep(1);
// Also test that JSPI works on other threads.
pthread_create(&id, NULL, run_thread, NULL);
printf("joining thread!\n");
pthread_join(id, NULL);
Expand Down
2 changes: 1 addition & 1 deletion test/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8300,7 +8300,7 @@ def test_pthread_join_and_asyncify(self):
# TODO Test with ASYNCIFY=1 https://github.com/emscripten-core/emscripten/issues/17552
self.require_jspi()
self.do_runf('core/test_pthread_join_and_asyncify.c', 'joining thread!\njoined thread!',
emcc_args=['-sJSPI_EXPORTS=run_thread',
emcc_args=['-sJSPI',
'-sEXIT_RUNTIME=1',
'-pthread', '-sPROXY_TO_PTHREAD'])

Expand Down
Loading