-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[wasm-mt] Allow JSImport promises to run on managed threadpool worker threads #83838
[wasm-mt] Allow JSImport promises to run on managed threadpool worker threads #83838
Conversation
42a2a17
to
f780f76
Compare
I pushed a hack that kind of works around the fact that |
This is getting closer to being usable. Remaining required things:
Remaining nice to haves:
|
381bcdf
to
1c098fd
Compare
This should fix these errors: [wasm test] [23:10:04] dbug: Reached wasm exit [wasm test] [23:10:04] info: node:internal/process/promises:246 [wasm test] [23:10:04] info: triggerUncaughtException(err, true /* fromPromise */); [wasm test] [23:10:04] info: ^ [wasm test] [23:10:04] info: [wasm test] [23:10:04] info: [UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "#<ExitStatus>".] { [wasm test] [23:10:04] info: code: 'ERR_UNHANDLED_REJECTION' [wasm test] [23:10:04] info: } [wasm test] [23:10:04] info: [wasm test] [23:10:04] info: Node.js v17.3.1 [wasm test] [23:10:04] info: Process node.exe exited with 1
…Pool.WorkerThread
This is a LIFO semaphore with an asynchronous wait that triggers callbacks on the JS event loop in case of Release or timeout.
58c5536
to
812524f
Compare
Set WorkerThread.IsIOPending when the current thread has unsettled JS interop promises. When IsIOPending is true, the worker will not exit even if it has no more work to do. Instead it will repeatedly wait for more work to arrive or for all promises to settle.
the delay is longer that the threadpool worker's semaphore timeout, in order to validate that the worker stays alive while there are unsettled promises
cffb1f9
to
c8afaba
Compare
Verified that flooding a threadpool worker with work doesn't prevent it from servicing the JS event loop. Approach:
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsDepends on #83998 - need Emscripten 3.1.33 or later (specifically this commit: emscripten-core/emscripten@0c2f589) in order to work correctly. Otherwise thread keepalive is not tracked correctly and pthreads will exit before they're supposed to. This PR allows managed thread pool worker threads to call out to asynchronous JS APIs. This PR implements the following:
|
This seems potentially a problem, IIRC it's common for stuff like Parallel.For to also use the main thread for computation when it forks off workers, or for applications to block the main thread waiting for workers to finish doing stuff. I remember at one point you were looking into setting up messageports between all of our workers, is there an obstacle to that or did it just not happen? |
Don't do that. synchronous waits on the main thread continue to be problematic on the browser. For an event driven UI app, I don't actually believe that it is common to block the main thread waiting for async work to complete, as that would lock up the UI.
Point to point messageports require bouncing through the main thread to set them up on demand. (creating a pthread requires proxying through the main thread, too). It seemed too expensive to set up all N^2 of them upfront (when we create each thread). So I wasn't entirely sure we need to do it, unless it's completely unavoidable. It's not entirely accurate actually that the emscripten's |
Should I slice this up into smaller PRs that might be easier to review? Something like:
|
Might be a good idea. |
Created #84489 to summarize the work to be landed. Will close this PR in favor of the smaller pieces. |
Depends on #83998 - we need Emscripten 3.1.33 or later (specifically this commit: emscripten-core/emscripten@0c2f589) in order to work correctly. Otherwise thread keepalive is not tracked correctly and pthreads will exit before they're supposed to.
This PR allows managed thread pool worker threads to call out to asynchronous JS APIs.
Contributes to #77287, #68162, #76956
This PR implements the following:
PortableThreadPool.WorkerThread.cs
file intoPortableThreadPool.WorkerThread.NonBrowser.cs
that containsPortableThreadPool.WorkerThread.CreateWorkerThread()
and the thread body functionWorkerThreadStart()
for normal non-JS platforms that repeatedly waits on aLowLevelLifoSemaphore
that and either begins dispatching work by calling (WorkerDoWork
) or trying to stop the worker thread by callingWorkerTimedOutMaybeStop
if the semaphore is not released (ie the worker has been idle for some time) before a timeout is reached.PortableThreadPool.WorkerThread.cs
- common code from the threadpool worker thread's loop. The main loop is broken up into two pieces:WorkerDoWork
- repeatedly callsTakeActiveRequest()
andThreadPoolWorkQueue.Dispatch()
to process queued work items; andWorkerTimedOutMaybeStop
which is called when the worker has been idle and no new work has been signaled and which tries to stop the worker or else go around the main loop one more time if new work shows up (or there's unfinished async IO).PortableThreadPool.WorkerThread.Browser.Threads.Mono.cs
file that implements theCreateWorkerThread
andWorkerThreadStart
functions for a JS-based async event loop. Instead of looping in a managedwhile
loop, the thread instead returns to the JS event loop after installing callbacks that are called when the threadpool semaphore is released or after a timeout. In the meantime the JS event queue can run code on the worker in order to settle promises.LowLevelLifoSemaphore
that uses asynchronous callbacks for success and timeout.WebWorkerEventLoop.StartExitable
and uses aWebWorkerEventLoop.KeepaliveToken
to return from the thread start function to the JS event loop while keeping the POSIX thread alive. This, together with the callback-based semaphore allows the thread to stay alive while JS events are resolved. When the semaphore is released, the wait callback is called and we process managed threadpool work.LowLevelLifoSemaphore
into two versions:Wait
method,LowLevelLifoSemaphore.CreateAsyncWait()
) that cannot useWait
but can use the callback-basedPrepareAsyncWait
method.Wait
andPrepareAsyncWait
for managing the semaphore's counts is shared.LifoSemaphoreAsyncWait
struct that shares some common code with the "normal"LifoSemaphore
(viaLifoSemaphoreBase
).mono_lifo_semaphore_asyncwait_prepare_wait
andmono_lifo_semaphore_asyncwait_release
we use two emscripten APIs:emscripten_set_timeout
to schedule a C callback to run on the current thread after a certain amount of time has passed;emscripten_dispatch_to_thread_async
to allow one thread to queue a C callback to run on anotherpthread
(note this depends on the main browser thread being responsive enough to relay JS messages from one thread to another).prepare_wait
._js_owned_object_table
isgc-handles.ts
is non-empty: that is if there are any JS promises that will eventually callback to a C# Task on the current worker, even if the worker doesn't have any active threadpool work, it will keep the pthread alive until all the JS promises settle (and remove themselves from_js_owned_object_table
.