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

[mono] add internal WebWorkerEventLoop utility class #84492

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Apr 7, 2023

This is Part 3 of #84489 - landing support for async JS interop on threadpool threads in multi-threaded WebAssembly.

Provides two pieces of functionality:

  1. A keepalive token that can be used to prevent the current POSIX thread from terminating when it returns from its thread start function, or from an invocation from the JS event loop. When the last keepalive token is destroyed (assuming Emscripten isn't keeping the thread alive for other reasons) it will terminate as if by calling pthread_exit and the webworker will be made available to other threads

  2. A HasUnsettledInteropPromises property that peeks _js_owned_object_table to see if there are any promises created by the interop subsystem that have not been fulfilled or rejected yet.

@ghost
Copy link

ghost commented Apr 7, 2023

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

Issue Details

This is Part 3 of #84489 - landing support for async JS interop on threadpool threads in multi-threaded WebAssembly.

Provides two pieces of functionality:

  1. A keepalive token that can be used to prevent the current POSIX thread from terminating when it returns from its thread start function, or from an invocation from the JS event loop. When the last keepalive token is destroyed (assuming Emscripten isn't keeping the thread alive for other reasons) it will terminate as if by calling pthread_exit and the webworker will be made available to other threads

  2. A HasUnsettledInteropPromises property that peeks _js_owned_object_table to see if there are any promises created by the interop subsystem that have not been fulfilled or rejected yet.

Note that this has a soft dependency on #83998 - without the emscripten bump, emscripten_runtime_keepalive_push does not work correctly with our compilation options. As a result there's MONO_EMSCRIPTEN_KEEPALIVE_WORKAROUND_HACK which kind of attempts to work around the issue, but is imprecise and ideally we won't ship it.

Author: lambdageek
Assignees: lambdageek
Labels:

arch-wasm, area-Threading-mono

Milestone: -

Provides two pieces of functionality:

1. A keepalive token that can be used to prevent the current POSIX
thread from terminating when it returns from its thread start
function, or from an invocation from the JS event loop.  When the last
keepalive token is destroyed (assuming Emscripten isn't keeping the
thread alive for other reasons) it will terminate as if by calling
`pthread_exit` and the webworker will be made available to other
threads

2. A `HasUnsettledInteropPromises` property that peeks
`_js_owned_object_table` to see if there are any promises created by
the interop subsystem that have not been fulfilled or rejected yet.
for mono_wasm_eventloop_has_unsettled_interop_promises we
can't use the _js_owned_object_table size because it contains
other interop objects, not just promises
@lambdageek
Copy link
Member Author

@pavelsavara anything else I should address in this one?

@pavelsavara
Copy link
Member

I'm ok about the code, but I don't understand the big picture.
I guess this PR doesn't have usage of WebWorkerEventLoop in it and it would be in the follow-up PR part 4 ?
Why is the KeepaliveToken instance or multiple instances. Would simple counter suffice ?

@lambdageek
Copy link
Member Author

lambdageek commented Apr 20, 2023

I'm ok about the code, but I don't understand the big picture. I guess this PR doesn't have usage of
WebWorkerEventLoop in it and it would be in the follow-up PR part 4 ?

Yea it's used in the Part 4.

Why is the KeepaliveToken instance or multiple instances. Would simple counter suffice ?

I wanted to make something that is harder to misuse than the emscripten push/pop API. Also to mirror the JS setTimeout/clearTimeout API - so every caller will get a unique keepalive token. I kind of suspect this will eventually become a public API

What I like about the KeepaliveToken approach is that in async code it's too easy to call pop too many times (or zero times). With a KeepaliveToken you'll get an InvalidOperationException if you pop too many times, instead of an incorrect counter value.

@pavelsavara
Copy link
Member

Thanks for explanation.
I see how this is useful, assuming there would be more than one push/pop per worker and that from external code.

I kind of suspect this will eventually become a public API

I'm not convinced this should be public API. Once we have use case for it we could re-consider it. For now, LGTM

@lambdageek
Copy link
Member Author

WBT failure is #85010 (which needs a dotnet/sdk fix to flow down)

@lambdageek
Copy link
Member Author

debugger test timeout is #84101

@lambdageek lambdageek merged commit bd37436 into dotnet:main Apr 20, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants