Skip to content

[browser] introduce JSProxyContext #95959

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
Merged
merged 26 commits into from
Dec 20, 2023
Merged

[browser] introduce JSProxyContext #95959

merged 26 commits into from
Dec 20, 2023

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Dec 13, 2023 •
edited
Loading

  • introduce JSProxyContext which collects all proxies and handles relevant to particular WebWorker/UI thread
  • introduce thread local JSProxyContext.OperationStack to know the context of the current operation.
    • most of the time, this matches the current thread
    • sometimes, we pass JSObject instance to JSImport. Such instance already has thread affinity
    • we will use JSProxyContext.CaptureContextFromParameter to capture the target thread to call into.
    • The dispatch to it is next PR.
  • move all handle related code into JSProxyContext
  • use target JSProxyContext instance as a lock around operations with handles.
  • introduced JSObject.ProxyContext also for non-MT build. It simplifies the #if FEATURE_WASM_THREADS hell.
  • make CancelablePromise more async
  • drop IsThreadCaptured it was bit misleading
  • new mono_wasm_invoke_method_raw which doesn't require existing JSProxyContext, for early calls.
  • new push_operation, pop_operation to be called before JSExport calls
  • renamed install_synchronization_context -> install_main_synchronization_context

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Dec 13, 2023
@pavelsavara pavelsavara added this to the 9.0.0 milestone Dec 13, 2023
@pavelsavara pavelsavara self-assigned this Dec 13, 2023
@ghost
Copy link

ghost commented Dec 13, 2023

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

Issue Details
  • introduce JSProxyContext which collects all proxies and handles relevant to particular WebWorker/UI thread
  • introduce thread local JSProxyContext.CapturedInstance to know the context of the current operation.
    • most of the time, this matches the current thread
    • sometimes, we pass JSObject instance to JSImport. Such instance already has thread affinity and we will use it to capture the target thread to call into.
  • move all handle related code into JSProxyContext
  • use target JSProxyContext instance as a lock around operations with handles.
  • introduced JSObject.ProxyContext also for non-MT build. It simplifies the #if FEATURE_WASM_THREADS hell.
  • make CancelablePromise more async
  • drop IsThreadCaptured it was bit misleading
  • new mono_wasm_invoke_method_raw which doesn't require existing JSProxyContext, for early calls.
  • new capture_proxy_context to be called before JSExport call
  • renamed install_synchronization_context -> install_main_synchronization_context
Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript, os-browser
Milestone: 9.0.0

@pavelsavara pavelsavara mentioned this pull request Dec 13, 2023
Merged
@pavelsavara
Copy link
Member Author

pavelsavara commented Dec 13, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 13, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara marked this pull request as ready for review December 13, 2023 16:43
@pavelsavara
Copy link
Member Author

pavelsavara commented Dec 17, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 17, 2023

Azure Pipelines successfully started running 1 pipeline(s).

maraf
maraf approved these changes Dec 18, 2023
View reviewed changes
@pavelsavara
remove complexity of PendingOperation maintanance
f56baa1 
 - new JSMarshalerArgumentImpl.ContextHandle field
 - all calls from JS side will set JSMarshalerArgumentImpl.ContextHandle with current thread context
     - via set_arg_proxy_context
 - all calls to JS where runtime calls stackalloc JSMarshalerArgument will set JSMarshalerArgumentImpl.ContextHandle with Context of promise or function affinity
    - it will use InitializeWithContext, JSImportNoCapture
    - these are Task and Function callbacks to JS
 - JSMarshalerArgument.ToManaged always uses new JSMarshalerArgument.ToManagedContext
 - JSMarshalerArgument.ToManagedContext always uses JSMarshalerArgumentImpl.ContextHandle, no magic
 - JSMarshalerArgument.ToJS always uses new JSMarshalerArgument.ToJSContext
 - JSMarshalerArgument.ToJSContext during JSImport uses JSProxyContext.CurrentOperationContext otherwise JSMarshalerArgumentImpl.ContextHandle
@pavelsavara
Copy link
Member Author

pavelsavara commented Dec 18, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 18, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

pavelsavara commented Dec 18, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 18, 2023

Azure Pipelines successfully started running 1 pipeline(s).

kg
kg reviewed Dec 18, 2023
View reviewed changes
kg
kg reviewed Dec 18, 2023
View reviewed changes
kg
kg reviewed Dec 18, 2023
View reviewed changes
@pavelsavara
feedback
9b8e3ad 
cancellation thread safe
Active issue dotnet#96173
@pavelsavara
Copy link
Member Author

pavelsavara commented Dec 19, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 19, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

pavelsavara commented Dec 19, 2023 •
edited
Loading

Filed new issues
#96173
#96190
#96191

@pavelsavara
Copy link
Member Author

pavelsavara commented Dec 19, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 19, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara merged commit 8bd23f0 into dotnet:main Dec 20, 2023
@pavelsavara pavelsavara deleted the browser_proxy_context branch December 20, 2023 07:57
This was referenced Jan 3, 2024
Closed
Open
@github-actions github-actions bot locked and limited conversation to collaborators Jan 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@kg kg kg approved these changes

@maraf maraf maraf approved these changes

@lambdageek lambdageek Awaiting requested review from lambdageek

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

@marek-safar marek-safar Awaiting requested review from marek-safar

Assignees

@pavelsavara pavelsavara

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm

Projects

None yet

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@pavelsavara @kg @maraf