Skip to content
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

feat: port libuv threadpool #9

Merged
merged 9 commits into from
Nov 17, 2022
Merged

feat: port libuv threadpool #9

merged 9 commits into from
Nov 17, 2022

Conversation

toyobayashi
Copy link
Owner

Fixes #8

Comment on lines +188 to +210
// TODO(?): need help
// Neither emscripten_dispatch_to_thread_async nor MAIN_THREAD_ASYNC_EM_ASM
// invoke the async complete callback if there is a printf() in worker thread.
// This breaks "packages/test/pool" tests.
// Not sure what happens, maybe has deadlock,
// and not sure whether this is Emscripten bug or my incorrect usage.
// BTW emscripten_dispatch_to_thread_async seems
// not support __wasm64__ V_I64 signature yet

// pthread_t main_thread = emscripten_main_browser_thread_id();
// if (pthread_equal(main_thread, pthread_self())) {
// NEXT_TICK(callback, data);
// } else {
// emscripten_dispatch_to_thread_async(main_thread,
// __EMNAPI_ASYNC_SEND_CALLBACK_SIG,
// callback,
// NULL,
// data);
// // or
// // MAIN_THREAD_ASYNC_EM_ASM({
// // emnapiGetDynamicCalls.call_vp($0, $1);
// // }, callback, data);
// }
Copy link
Owner Author

Choose a reason for hiding this comment

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

@RReverser I left comments here. If you prefer that emnapi should not depend on PThread object, I would appreciate it if you could open another PR to provide help. Sorry for my broken English, forget it if there is some strange or offensive expression.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@RReverser c3a3cdb1 At last I found emscripten_proxy_async and em_proxying_queue_create works well, but using emscripten_proxy_get_system_queue() will also stuck if call printf in child thread. I could not figure out why.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you prefer that emnapi should not depend on PThread object, I would appreciate it if you could open another PR to provide help

Heh unfortunately I'm finding a bit hard to follow how threading works in this library, so not sure I can submit a PR for that yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW one other reason I want it to have its own pool of workers is that upstream there are talks of making it possible to use pthreads without a pthread pool, and I now have a working implementation of that at least for Node.js.

However, it seems that emnapi won't work with that approach in the current form since it manipulates internal arrays and waits until unusedWorkers is not empty. But, if Emscripten will use pthreads without a pthread pool, it will be empty at the start so emnapi will wait forever.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, looks like I had a slightly outdated dist version and you already removed some more usages. This is looking promising, going to give it a try with the pool-less version of Emscripten.

@toyobayashi toyobayashi merged commit 7e9e784 into main Nov 17, 2022
@toyobayashi toyobayashi deleted the feat-threadpool branch April 6, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current AsyncWorker scheduling conflict with threads used by other libraries
2 participants