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

[browser][MT] dispatch across threads via emscripten #97669

Merged
merged 8 commits into from
Feb 5, 2024

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jan 29, 2024

Dispatch

use emscripten job queue in the target thread instead of JSSynchronizationContext to call cross-thread JS functions.

This has following benefits

  • the target thread doesn't have to be managed thread.
    • this contributes to the goal of avoiding managed code on UI thread to avoid clashing with GC stop-the-world
  • smaller allocation of the job object
  • probably faster than interp of the JSSynchronizationContext
  • doesn't need to wait for schedule_background_exec -> setTimeout tick

Cross-thread asynchronous

All of them are fire-and-forget messages

  • ReleaseCSOwnedObjectPost
  • InvokeJSImportAsyncPost
  • ResolveOrRejectPromisePost & CancelPromisePost
    • this is complex case because there could be (rare) racing threads.
    • The one with JS objects is resolving the Task/Promise and another thread is triggering cancellation.
    • We need to keep the GCHandle of the Task/Promise alive so, that the async messages don't have use-after-free problem.
    • After this PR we don't have the use-after-free problem
    • But we leak the GCHandle instead, only when there is a race condition.
    • The problem will become even more complex, once we want to also resolve/reject the promise->task by async message.

Cross-thread synchronous

  • InvokeJSFunctionSend & InvokeJSImportSyncSend
    • both of them are blocking the caller in native code via GC friendly mono_coop_sem_wait
    • I speculate that target thread is unable to .Wait for too long
  • JSSynchronizationContext.Send now throws PNSE on JS threads via ThrowOnBlockingWaitOnJSInteropThread
  • ConsolePal.Clear - ICAL instead of JSImport - more in line the rest of with posix console pal.

Other

  • deleted legacy interop ICAL dead code

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

ghost commented Jan 29, 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-System.Runtime.InteropServices.JavaScript, os-browser

Milestone: 9.0.0

@pavelsavara

This comment was marked as resolved.

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara and others added 5 commits February 5, 2024 10:30
…stem/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs

Co-authored-by: Marek Fišera <mara@neptuo.com>
…stem/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs

Co-authored-by: Marek Fišera <mara@neptuo.com>
Co-authored-by: Marek Fišera <mara@neptuo.com>
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@maraf maraf left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@pavelsavara
Copy link
Member Author

I filled #97968

@pavelsavara pavelsavara merged commit 68a18eb into dotnet:main Feb 5, 2024
130 of 139 checks passed
@pavelsavara pavelsavara deleted the browser_mt_emscripten_dispatch branch February 5, 2024 12:08
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants