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.

Sign up for GitHub

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

Jump to bottom

[browser][MT] deputy thread #98118

Merged
merged 9 commits into from
Feb 22, 2024
Merged

[browser][MT] deputy thread #98118

merged 9 commits into from
Feb 22, 2024

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Feb 7, 2024 •
edited
Loading

Threading modes

  • described in multi-threading.md
  • new JSProxyContext.ThreadBlockingMode flowing from config.
    • influences ThrowOnBlockingWaitOnJSInteropThread behavior
  • new JSProxyContext.ThreadInteropMode
    • influences sychronous calls to JSImport and JSExport
    • new JSProxyContext.IsPendingSynchronousCall
    • new BeforeSyncJSExport and AfterSyncJSExport to recognize start/end of incoming sync JSImport call.
    • new mono_wasm_invoke_jsexport_sync_send_cb calls it
    • mono_wasm_install_js_worker_interop_wrapper helps with the setup of it.
    • new places throw PNSE when the rules are breached
    • args.CallerNativeTID to see if the JSImport caller is the same thread.
      • only set runtimeHelpers.isPendingSynchronousCall when are same
    • get_caller_native_tid to read that
  • new JSProxyContext.MainThreadingMode
    • could be used to switch off deputy thread. Not recommended.
  • new enums JSThreadBlockingMode and JSThreadInteropMode and MainThreadingMode

Deputy creation

  • new mono_wasm_create_deputy_thread
    • create deputy thread and start mono VM on it
  • new mono_wasm_register_ui_thread
    • and later attach the UI thread to Mono. We hope to avoid this in the future.

testing

  • there is new flavor of System.Runtime.InteropServices.JavaScript.BackgroundExec.Tests.csproj
    • disabled for ST
    • to run tests which really need to run from non-JS thread.
    • new env variable IsWasmBackgroundExec passed to unit tests

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-VM-threading-mono os-browser Browser variant of arch-wasm labels Feb 7, 2024
@pavelsavara pavelsavara added this to the 9.0.0 milestone Feb 7, 2024
@pavelsavara pavelsavara self-assigned this Feb 7, 2024
@ghost
Copy link

ghost commented Feb 7, 2024

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

Issue Details

null

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-VM-threading-mono, os-browser
Milestone: 9.0.0

@build-analysis build-analysis bot mentioned this pull request Feb 7, 2024
Closed
@pavelsavara pavelsavara closed this Feb 9, 2024
@pavelsavara pavelsavara reopened this Feb 9, 2024
This was referenced Feb 10, 2024
Closed
Closed
@pavelsavara pavelsavara force-pushed the browser_mt_deputy branch 3 times, most recently from 145b59f to cf52083 Compare February 14, 2024 17:08
@pavelsavara

This comment was marked as outdated.

Sign in to view
This was referenced Feb 15, 2024
Closed
Open
@build-analysis build-analysis bot mentioned this pull request Feb 18, 2024
Closed
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@build-analysis build-analysis bot mentioned this pull request Feb 18, 2024
Closed
@pavelsavara pavelsavara force-pushed the browser_mt_deputy branch 3 times, most recently from 6fe71bb to edf321d Compare February 19, 2024 17:32
@build-analysis build-analysis bot mentioned this pull request Feb 19, 2024
Closed
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

maraf
maraf reviewed Feb 21, 2024
View reviewed changes
...vices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSHostImplementation.Types.cs
NoSyncJSInterop = 0,
// allow non-re-entrant synchronous blocking calls to and from JS on JSWebWorker on threads with JS interop, like JSWebWorker and Main thread.
// calling synchronous JSImport on thread pool or new threads is allowed.
SimpleSynchronousJSInterop = 1,
Copy link
Member

Choose a reason for hiding this comment

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

I think we should come with a better name (before api review?)

Copy link
Member Author

Choose a reason for hiding this comment

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

@lewing also didn't like it. Please come up with suggestions.

Copy link
Member

@maraf maraf Feb 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm voting for longer and more descriptive name, "simple" has too much meanings

Copy link
Member

Choose a reason for hiding this comment

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

  • SingleSynchronousCallChainJSInterop
  • SingleSynchronousCallJSInterop

Copy link
Member Author

Choose a reason for hiding this comment

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

SingleSynchronousCallJSInterop is not bad, I like it

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW: those would not be visible on C# API, just JS config

@pavelsavara pavelsavara mentioned this pull request Feb 21, 2024
Open
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara merged commit 771afaf into dotnet:main Feb 22, 2024
146 of 149 checks passed
@pavelsavara pavelsavara deleted the browser_mt_deputy branch February 22, 2024 08:39
@pavelsavara pavelsavara changed the title [browser][MT] deputy [browser][MT] deputy thread Feb 22, 2024
This was referenced Feb 24, 2024
Closed
Closed
Closed
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@maraf maraf maraf approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@lambdageek lambdageek Awaiting requested review from lambdageek lambdageek is a code owner

@vargaz vargaz Awaiting requested review from vargaz

@marek-safar marek-safar Awaiting requested review from marek-safar marek-safar is a code owner

@akoeplinger akoeplinger Awaiting requested review from akoeplinger akoeplinger is a code owner

Assignees

@pavelsavara pavelsavara

Labels
arch-wasm WebAssembly architecture area-VM-threading-mono os-browser Browser variant of arch-wasm
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@pavelsavara @maraf