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

Split PortableThreadPool.WorkerThread start and loop body #84490

Merged

Conversation

lambdageek
Copy link
Member

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

We will need to start the threadpool worker threads on the browser in a special way, such that they can exit back to the JS event loop, and use callbacks to run the worker loop body.

The current PR splits into a separate file the logic for starting threadpool worker threads, and the outer loop that waits for the semaphore that signals that work is available for the worker. The loop body (to be shared with the callback-based approach in a future PR) remains in PortableThreadPool.WorkerThread.cs as several new toplevel functions.

Current PR is just refactoring existing code. No functional change.

For browser-wasm we will need to start the worker thread in a special
way, and use callbacks to run the loop body.

Current PR is just refactoring existing code. No functional change.
@ghost
Copy link

ghost commented Apr 7, 2023

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

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

We will need to start the threadpool worker threads on the browser in a special way, such that they can exit back to the JS event loop, and use callbacks to run the worker loop body.

The current PR splits into a separate file the logic for starting threadpool worker threads, and the outer loop that waits for the semaphore that signals that work is available for the worker. The loop body (to be shared with the callback-based approach in a future PR) remains in PortableThreadPool.WorkerThread.cs as several new toplevel functions.

Current PR is just refactoring existing code. No functional change.

Author: lambdageek
Assignees: -
Labels:

area-System.Threading

Milestone: -

@lambdageek
Copy link
Member Author

lambdageek commented Apr 7, 2023

For reviewers: I recommend turning on "hide whitespace" for this PR on GitHub

Screenshot 2023-04-07 at 16 06 01

@lambdageek
Copy link
Member Author

@kouvel could you take a look

threadAdjustmentLock.Release();
}
// if we get here new work came in and we're going to keep running
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Could just return false instead of the break statement above

Copy link
Member

@kouvel kouvel left a comment

Choose a reason for hiding this comment

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

Just a couple of minor suggestions, otherwise LGTM

@lambdageek
Copy link
Member Author

Failure is #84979

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

Successfully merging this pull request may close these issues.

2 participants