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

[wasm-mt] Allow JSImport promises to run on managed threadpool worker threads #83838

Closed

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Mar 23, 2023

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:

  • Refactors the PortableThreadPool.WorkerThread.cs file into
    • PortableThreadPool.WorkerThread.NonBrowser.cs that contains PortableThreadPool.WorkerThread.CreateWorkerThread() and the thread body function WorkerThreadStart() for normal non-JS platforms that repeatedly waits on a LowLevelLifoSemaphore that and either begins dispatching work by calling (WorkerDoWork) or trying to stop the worker thread by calling WorkerTimedOutMaybeStop 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 calls TakeActiveRequest() and ThreadPoolWorkQueue.Dispatch() to process queued work items; and WorkerTimedOutMaybeStop 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).
  • Creates a new PortableThreadPool.WorkerThread.Browser.Threads.Mono.cs file that implements the CreateWorkerThread and WorkerThreadStart functions for a JS-based async event loop. Instead of looping in a managed while 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.
    • The main difference from non-Browser is that we use a new version of LowLevelLifoSemaphore that uses asynchronous callbacks for success and timeout.
    • Additionally the thread is started using WebWorkerEventLoop.StartExitable and uses a WebWorkerEventLoop.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.
    • If the semaphore times out, the timeout callback is called and we decide if there is any pending async IO (in the form of unsettled JS interop promises) and either schedule another semaphore wait or destroy the keepalive token allowing the thread to exit if the keepalive count reaches zero.
  • Refactors LowLevelLifoSemaphore into two versions:
    • the "normal" version (created with the public constructor) that can be used with a synchronous Wait method,
    • and the "async wait" version (created with LowLevelLifoSemaphore.CreateAsyncWait()) that cannot use Wait but can use the callback-based PrepareAsyncWait method.
    • The common code for Wait and PrepareAsyncWait for managing the semaphore's counts is shared.
  • Implements the underlying native LifoSemaphoreAsyncWait struct that shares some common code with the "normal" LifoSemaphore (via LifoSemaphoreBase).
    • To implement mono_lifo_semaphore_asyncwait_prepare_wait and mono_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 another pthread (note this depends on the main browser thread being responsive enough to relay JS messages from one thread to another).
    • When a thread starts to wait we create a wait queue entry and simultaneously start a timeout and return from prepare_wait.
    • To release the semaphore, we select a wait entry and dispatch its success callback to run on the waiting thread. When the success or timeout callbacks run, they unlink the wait entry (and the success callback cancels the timeout callback) and call the user-provided success/timeout functions.
    • (There is a case where the async success is queued but the timeout fires first that is handled by the timeout callback first checking if a success has been queued to run, too, in which case the timeout silently becomes a no-op)
  • The managed threadpool worker keeps its pthread alive as long as the _js_owned_object_table is gc-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.

@lambdageek
Copy link
Member Author

I pushed a hack that kind of works around the fact that emscripten_runtime_keepalive_push/pop are no-ops. But it's super fragile. Mostly it's just here to let me keep working on this branch while we take another Emscripten bump

@lambdageek
Copy link
Member Author

lambdageek commented Mar 27, 2023

This is getting closer to being usable.

Remaining required things:

  • make sure we keep track of unsettled JS promises and set IsIOPending appropriately - disallow a worker to exit if it is waiting for a slow JS promise
  • verify that if we flood a worker with work it will still periodically yield to JS. OR else we need to rewrite WorkerDoWork (while (TakeActiveRequest(...))) to periodically yield

Remaining nice to haves:

  • Share more code with the normal WorkerThread code - share the loopy logic
  • make the LowLevelJSSemphore use the managed spinlock logic and maybe make it a special variant of the normal LowLevelLifoSemaphore managed class (with a different native impl, and an additional JS-friendly wait method)
  • Figure out if there's some way to make Task.Wait() throw an InvalidOperationException if the thing at the head of the chain is a JS Promise from the same thread.

radekdoulik and others added 6 commits March 28, 2023 13:54
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
This is a LIFO semaphore with an asynchronous wait that triggers
callbacks on the JS event loop in case of Release or timeout.
@lambdageek lambdageek force-pushed the feature-wasm-threadpool-worker branch from 58c5536 to 812524f Compare March 31, 2023 03:22
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
@lambdageek lambdageek force-pushed the feature-wasm-threadpool-worker branch from cffb1f9 to c8afaba Compare April 3, 2023 20:00
@lambdageek
Copy link
Member Author

Verified that flooding a threadpool worker with work doesn't prevent it from servicing the JS event loop.

Approach:

  1. ThreadPool.SetMaxThreads(2,2)
  2. Create a FloodWork task that uses Task.Run to start two more copies of itself and then does an expensive synchronous computation before awaiting its children.
  3. Start a task that calls out to globalThis.fetch() and then calls and response.text() after a (JS setTimeout-based) delay of a few seconds.
  4. From the main thread, race FloodWork and the background fetch
  5. cancel the flood
  6. observe that we got the expected string back from response.text()

@lambdageek lambdageek changed the title [NO MERGE][wasm-threads] JSImport promises on background threads [wasm-mt] Allow JSImport promises to run on managed threadpool worker threads Apr 4, 2023
@lambdageek lambdageek added arch-wasm WebAssembly architecture and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) labels Apr 4, 2023
@ghost
Copy link

ghost commented Apr 4, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Depends 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:

  • Refactors the PortableThreadPool.WorkerThread.cs file into
    • PortableThreadPool.WorkerThread.NonBrowser.cs that contains PortableThreadPool.WorkerThread.CreateWorkerThread() and the thread body function WorkerThreadStart() for normal non-JS platforms that repeatedly waits on a LowLevelLifoSemaphore that and either begins dispatching work by calling (WorkerDoWork) or trying to stop the worker thread by calling WorkerTimedOutMaybeStop 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 calls TakeActiveRequest() and ThreadPoolWorkQueue.Dispatch() to process queued work items; and WorkerTimedOutMaybeStop 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).
  • Creates a new PortableThreadPool.WorkerThread.Browser.Threads.Mono.cs file that implements the CreateWorkerThread and WorkerThreadStart functions for a JS-based async event loop. Instead of looping in a managed while 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.
    • The main difference from non-Browser is that we use a new version of LowLevelLifoSemaphore that uses asynchronous callbacks for success and timeout.
    • Additionally the thread is started using WebWorkerEventLoop.StartExitable and uses a WebWorkerEventLoop.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.
    • If the semaphore times out, the timeout callback is called and we decide if there is any pending async IO (in the form of unsettled JS interop promises) and either schedule another semaphore wait or destroy the keepalive token allowing the thread to exit if the keepalive count reaches zero.
  • Refactors LowLevelLifoSemaphore into two versions:
    • the "normal" version (created with the public constructor) that can be used with a synchronous Wait method,
    • and the "async wait" version (created with LowLevelLifoSemaphore.CreateAsyncWait()) that cannot use Wait but can use the callback-based PrepareAsyncWait method.
    • The common code for Wait and PrepareAsyncWait for managing the semaphore's counts is shared.
  • Implements the underlying native LifoSemaphoreAsyncWait struct that shares some common code with the "normal" LifoSemaphore (via LifoSemaphoreBase).
    • To implement mono_lifo_semaphore_asyncwait_prepare_wait and mono_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 another pthread (note this depends on the main browser thread being responsive enough to relay JS messages from one thread to another).
    • When a thread starts to wait we create a wait queue entry and simultaneously start a timeout and return from prepare_wait.
    • To release the semaphore, we select a wait entry and dispatch its success callback to run on the waiting thread. When the success or timeout callbacks run, they unlink the wait entry (and the success callback cancels the timeout callback) and call the user-provided success/timeout functions.
    • (There is a case where the async success is queued but the timeout fires first that is handled by the timeout callback first checking if a success has been queued to run, too, in which case the timeout silently becomes a no-op)
  • The managed threadpool worker keeps its pthread alive as long as the _js_owned_object_table is gc-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.
Author: lambdageek
Assignees: lambdageek
Labels:

arch-wasm, area-VM-threading-mono

Milestone: -

@lambdageek lambdageek marked this pull request as ready for review April 4, 2023 15:09
@lambdageek lambdageek requested a review from kouvel April 4, 2023 15:09
@kg
Copy link
Member

kg commented Apr 4, 2023

this depends on the main browser thread being responsive enough to relay JS messages from one thread to another

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?

@lambdageek
Copy link
Member Author

this depends on the main browser thread being responsive enough to relay JS messages from one thread to another

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.

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.

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?

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 emscripten_dispatch_to_thread_async requires the main thread to be in the JS event loop, always. because it's ferrying C data structures around, I believe the async work queues do get populated and pumped in other places as long as the worker thread is doing some emscripten syscalls. But if the worker thread is completely idle (or not interacting with emscripten at all), I believe the main thread needs to post it a message in order for the work to get dequeued and processed.

@lambdageek
Copy link
Member Author

lambdageek commented Apr 5, 2023

Should I slice this up into smaller PRs that might be easier to review? Something like:

  • Refactoring of LifoSemaphore, but no implementation of AsyncWait
  • Refactoring of ThreadPool.WorkerThread, but no implementation of the JS event loop version
  • Adding the async wait lifo semaphore
  • Adding the keepalive API and the JS eventloop ThreadPool.WorkerThread and updated smoketest

@kg
Copy link
Member

kg commented Apr 5, 2023

Should I slice this up into smaller PRs that might be easier to review? Something like:

* Refactoring of LifoSemaphore, but no implementation of AsyncWait

* Refactoring of ThreadPool.WorkerThread, but no implementation of the JS event loop version

* Adding the async wait lifo semaphore

* Adding the keepalive API and the JS eventloop ThreadPool.WorkerThread and updated smoketest

Might be a good idea.

@lambdageek
Copy link
Member Author

Created #84489 to summarize the work to be landed. Will close this PR in favor of the smaller pieces.

@lambdageek
Copy link
Member Author

Closing this in favor of #84494 which is the last part of #84489

@lambdageek lambdageek closed this Apr 7, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-VM-threading-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants