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] Add a System.Runtime.InteropServices.JavaScript.BrowserSynchronizationContext #69409

Closed
Tracked by #68162
lambdageek opened this issue May 16, 2022 · 15 comments
Assignees
Milestone

Comments

@lambdageek
Copy link
Member

lambdageek commented May 16, 2022

Update this is no longer a public API proposal. instead, we should make an internal SynchronizationContext subclass that is installed by the runtime on the main thread.


Background and motivation

In order to enable multi-threaded WebAssembly programming, we want to allow users to continue to program using async/await. In the current single-threaded browser-wasm runtime, all asynchronous tasks run on the main browser thread implicitly as there is nowhere else to run them. Our multi-threaded WebAssembly programming model is built on WebWorkers - dedicated threads that do not share system memory with the main browser thread but may pass messages to it back and forth including WebAssembly main memory using a SharedArrayBuffer.

In multi-threaded WebAssembly, certain operations (for example code that manipulates the DOM of the current page) must run on the browser's main event loop, not in a WebWorker. As a result, it is necessary to add a SynchronizationContext subclass that allows asynchronous work to be posted back to the main browser thread.

API Proposal

using System.Threading;

namespace System.Runtime.InteropServices.JavaScript;

/// A synchronization context that queues work to run on the browser's main thread.
/// In a multi-threaded web application certain operations (such as manipulating the DOM) must be done on the
/// main thread and not in a WebWorker.  The BrowserSynchronizationContext allows asynchronous callbacks to
/// return to run their continuations on the main thread.
[SupportedOSPlatform("browser")]
internal sealed class BrowserSynchronizationContext : SynchronizationContext
{
    internal BrowserSynchronizationContext();
    public override SynchronizatonContext CreateCopy();
    public override void Post(SendOrPostCallback d, object state);
    //// Always throws NotSupportedException
    public override void Send(SendOrPostCallback d, object state);
}

API Usage

In a .razor page in Blazor:

<button @onclick="DoSomeWork">Start Work</button>
<text>Result is: @output</text>

@code {
    string output = "";
    public async Task DoSomeWork() {
        output = "pending";
        Debug.Assert (SynchronizationContext.Current instanceof BrowserSynchronizationContext);
        var result = await Task.Run(Model.SomeAsyncWork); // start some work on the threadpool
        output = result.ToString();  // return to the UI thread and update the model
    }
}

Alternative Designs

  1. We could create some kind of JavaScript shim that exposes proxy objects for manipulating the DOM from an arbitrary webworker and leave the C# layer oblivious. (We would have to do this for every JS API that is only available in the main browser thread)
  2. We could add a synchronization context class for posting work to arbitrary web workers' event loops. In that case the main thread would not be special:
        public class JSEventLoopSynchronizationContext : SynchronizationContext {
            public JSEventLoopSynchronizationContext (Thread t);
            public static JSEventLoopSynchronizationContext Browser;
            // same overrides as before
        }

We can also consider some other names:

  • some other namespace
  • DOMSynchronizationContext
  • MainEventLoopSynchronizationContext
  • JavaScriptSynchronizationContext

Other considerations:

  • the eventloop, tasks and microtasks are not a browser concept, they're a JS concept. So we may want a name that makes sense in Node environments, too.

Risks

No response

@lambdageek lambdageek added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.InteropServices.JavaScript labels May 16, 2022
@lambdageek lambdageek added this to the 7.0.0 milestone May 16, 2022
@lambdageek
Copy link
Member Author

/cc @lewing @kg

@lambdageek lambdageek added the arch-wasm WebAssembly architecture label May 16, 2022
@ghost
Copy link

ghost commented May 16, 2022

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

Issue Details

Background and motivation

In order to enable multi-threaded WebAssembly programming, we want to allow users to continue to program using async/await. In the current single-threaded browser-wasm runtime, all asynchronous tasks run on the main browser thread implicitly as there is nowhere else to run them. Our multi-threaded WebAssembly programming model is built on WebWorkers - dedicated threads that do not share system memory with the main browser thread but may pass messages to it back and forth including WebAssembly main memory using a SharedArrayBuffer.

In multi-threaded WebAssembly, certain operations (for example code that manipulates the DOM of the current page) must run on the browser's main event loop, not in a WebWorker. As a result, it is necessary to add a SynchronizationContext subclass that allows asynchronous work to be posted back to the main browser thread.

API Proposal

using System.Threading;

namespace System.Runtime.InteropServices.JavaScript;

/// A synchronization context that queues work to run on the browser's main thread.
/// In a multi-threaded web application certain operations (such as manipulating the DOM) must be done on the
/// main thread and not in a WebWorker.  The BrowserSynchronizationContext allows asynchronous callbacks to
/// return to run their continuations on the main thread.
[SupportedOSPlatform("browser")]
public sealed class BrowserSynchronizationContext : SynchronizationContext
{
    public BrowserSynchronizationContext();
    public override SynchronizatonContext CreateCopy();
    public override void Post(SendOrPostCallback d, object state);
    //// Always throws NotSupportedException
    public override void Send(SendOrPostCallback d, object state);
}

API Usage

In a .razor page in Blazor:

<button @onclick="DoSomeWork">Start Work</button>
<text>Result is: @output</text>

@code {
    string output = "";
    public async Task DoSomeWork() {
        output = "pending";
        Debug.Assert (SynchronizationContext.Current instanceof BrowserSynchronizationContext);
        var result = await Task.Run(Model.SomeAsyncWork); // start some work on the threadpool
        output = result.ToString();  // return to the UI thread and update the model
    }
}

Alternative Designs

  1. We could create some kind of JavaScript shim that exposes proxy objects for manipulating the DOM from an arbitrary webworker and leave the C# layer oblivious. (We would have to do this for every JS API that is only available in the main browser thread)
  2. We could add a synchronization context class for posting work to arbitrary web workers' event loops. In that case the main thread would not be special:
        public class JSEventLoopSynchronizationContext : SynchronizationContext {
            public JSEventLoopSynchronizationContext (Thread t);
            public static JSEventLoopSynchronizationContext Browser;
            // same overrides as before
        }

We can also consider some other names:

  • some other namespace
  • DOMSynchronizationContext
  • MainEventLoopSynchronizationContext
  • JavaScriptSynchronizationContext

Other considerations:

  • the eventloop, tasks and microtasks are not a browser concept, they're a JS concept. So we may want a name that makes sense in Node environments, too.

Risks

No response

Author: lambdageek
Assignees: -
Labels:

api-suggestion, arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: 7.0.0

@stephentoub
Copy link
Member

stephentoub commented May 16, 2022

Who installs the synchronization context? I'm wondering if it needs to be public or if it can just be an implementation detail; most of them are the latter.

@lambdageek
Copy link
Member Author

Who installs the synchronization context? I'm wondering if it needs to be public or if it can just be an implementation detail; most of them are the latter.

Oh interesting, I hadn't realized that. I think that's reasonable for the main thread.

I guess if we later decide to add a synchronization context that will post work to any web worker's event loop, that one would probably need to be public (or at least we might want to add some method that takes a Thread and returns a SynchronizationContext somewhere).


Going to rewrite this issue to focus on just adding an internal synch context that is installed by the runtime

@lambdageek lambdageek removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 17, 2022
@lambdageek lambdageek changed the title [API Proposal]: System.Runtime.InteropServices.JavaScript.BrowserSynchronizationContext [wasm-mt] Add a System.Runtime.InteropServices.JavaScript.BrowserSynchronizationContext May 17, 2022
@radical
Copy link
Member

radical commented Aug 12, 2022

@kg Moving this to 8.0 . Please change it back, if that's incorrect.

@radical radical modified the milestones: 7.0.0, 8.0.0 Aug 12, 2022
@kg
Copy link
Member

kg commented Aug 12, 2022

I think this needs to be in 7.

@kg kg modified the milestones: 8.0.0, 7.0.0 Aug 12, 2022
@kg
Copy link
Member

kg commented Aug 12, 2022

Fixed by #72652

@kg kg closed this as completed Aug 12, 2022
@kg kg reopened this Aug 18, 2022
@kg kg modified the milestones: 7.0.0, Future Aug 18, 2022
@kg
Copy link
Member

kg commented Aug 18, 2022

@stephentoub Hi Stephen, I've been looking into the little hidden interactions between TaskContinuation and custom synchronization contexts and it's very unclear to me how we should go about properly integrating everything here. We need a custom sync context (or something that achieves the equivalent) because things like WebSockets and timers have strong thread affinity in the browser. But from doing testing on my own and reading the code, it looks like having a sync context registered at all will punt all running workloads into a degraded mode where they never run continuations inline. Is this less of a problem than it appears?

@stephentoub
Copy link
Member

Can you share an example of the kind of code you're concerned about with:
"it looks like having a sync context registered at all will punt all running workloads into a degraded mode where they never run continuations inline. Is this less of a problem than it appears?"

If the continuation must run back on the original context, then the implementation has little choice but to dutifully post back to the original context. If the antecedent operation completes on the same context, however, there is an optimization in the await infrastructure that will avoid posting back; that just requires that SynchronizationContext.Current is the same object as was current when the await was initiated, so if this is the concern, you'd want to make sure that you're not artificially cloning your context object and instead try to have the same SynchronizationContext instance always used for a given target context/thread/environment.

@stephentoub
Copy link
Member

(If instead the issue is that you see continuations being queued back to the original context but doing so is unnecessary, that's why .ConfigureAwait(false) exists.)

@kg
Copy link
Member

kg commented Aug 18, 2022

The issue is that in my testing, this logic results in all continuations being posted to the threadpool even if they were ConfigureAwait(false), specifically because we have a sync context.

https://cs.github.com/dotnet/runtime/blob/17154bd7b8f21d6d8d6fca71b89d7dcb705ec32b/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TaskContinuation.cs#L610

The presence of a sync context is being used as a heuristic for "continuations should never run here".

@stephentoub
Copy link
Member

The presence of a sync context is being used as a heuristic for "continuations should never run here".

Ah, yes, that old gem. Though to clarify slightly, it's really "should never run here if the continuation isn't associated with that same sync context".

In general I wouldn't expect it to be a big deal because you should only hit that on the first continuation in a series. After that, subsequent awaits won't see the original sync context because you won't be on a thread that has it set. Unless, are you setting that sync context on the thread pool threads?

@kg
Copy link
Member

kg commented Aug 18, 2022

The presence of a sync context is being used as a heuristic for "continuations should never run here".

Ah, yes, that old gem. Though to clarify slightly, it's really "should never run here if the continuation isn't associated with that same sync context".

In general I wouldn't expect it to be a big deal because you should only hit that on the first continuation in a series. After that, subsequent awaits won't see the original sync context because you won't be on a thread that has it set. Unless, are you setting that sync context on the thread pool threads?

OK, my hope was that it would only hurt the first thing in the series so it wouldn't matter much - I just see so much ContinueAwait(false) in the BCL that this had me worried we were going to significantly regress performance by introducing a sync context and it made me wonder if we should treat it as a radioactive type. But I don't see any real alternative to SyncContext for making sure things with thread affinity can make it back to the main browser thread.

Things are weird as-is since right now the wasm 'thread pool' runs everything on the main thread, so we get to skip all the overhead this introduces until threads get turned on and suddenly there's a sync context and a thread pool...

And 'if the continuation isn't associated with that same sync context' in this case means 'if the continuation was issued from another sync context OR has no captured context because it was ConfigureAwait(false), it can't run here'?

@stephentoub
Copy link
Member

And 'if the continuation isn't associated with that same sync context' in this case means 'if the continuation was issued from another sync context OR has no captured context because it was ConfigureAwait(false), it can't run here'?

Correct. This was put in a decade ago as a defense against doing too much unrelated work on the UI thread of WinForms and WPF client apps. If we were doing it over, I don't think we'd add it, but I expect we'd see a fair number of breaks if we removed the checks now (we could consider it, or consider it only for wasm if it really proved problematic). As an aside, when we added support for ValueTask, we explicitly didn't add in this check, so you shouldn't see such forced hops if you await new ValueTask<int>(taskOfInt).ConfigureAwait(false) instead.

@lambdageek
Copy link
Member Author

@kg I think this issue is done. Feel free to reopen, or create new issues if we need to revisit

@ghost ghost locked as resolved and limited conversation to collaborators Oct 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants