Skip to content

[browser][MT] Marshal resolved/unresolved tasks separately - #99347

Merged
pavelsavara merged 12 commits into
dotnet:mainfrom
pavelsavara:browser_marshal_resoved_tasks_parameters
Mar 7, 2024
Merged

[browser][MT] Marshal resolved/unresolved tasks separately#99347
pavelsavara merged 12 commits into
dotnet:mainfrom
pavelsavara:browser_marshal_resoved_tasks_parameters

Conversation

@pavelsavara

@pavelsavarapavelsavara commented Mar 6, 2024

Copy link
Copy Markdown
Member

This change will avoid sending 2 messages to target thread (UI) and instead will only send one.

  • when the target thread is the same as current thread and this is synchronous/blocking return from JSExport
  • when this is non-return parameter of JSImport <- this is new here

Otherwise will use new JSAsyncTaskScheduler

  • to make sure that the mono_wasm_resolve_or_reject_promise_post happens after mono_wasm_invoke_jsimport_async_post
  • I'm not sure why #99317 was not enough and didn't help it.

contributes to #98406

@pavelsavarapavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Mar 6, 2024
@pavelsavarapavelsavara added this to the 9.0.0 milestone Mar 6, 2024
@pavelsavara
pavelsavara requested a review from marafMarch 6, 2024 10:02
@pavelsavarapavelsavara self-assigned this Mar 6, 2024
@ghost

ghost commented Mar 6, 2024

Copy link
Copy Markdown

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

Issue Details

This change will avoid sending 2 messages to target thread (UI) and instead will only send one.

  • when the target thread is the same as current thread and this is synchronous/blocking return from JSExport
  • when this is non-return parameter of JSImport <- this is new here

It could still happen that Task completion (on TP thread) is racing with the task.IsCompleted condition here, so it may not fix all cases, but it will dramatically reduce the chances.

I'm not 100% sure why #99317 didn't help it.

contributes to #98406

Author:pavelsavara
Assignees:pavelsavara
Labels:

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

Milestone:9.0.0

@pavelsavarapavelsavara changed the title [browser][MT] Marshal resolved tasks immediately, when they are parametr of JSImport[browser][MT] Marshal resolved/unresolved tasks separatelyMar 6, 2024
@pavelsavara
pavelsavara marked this pull request as ready for review March 6, 2024 14:48
@pavelsavara
pavelsavara requested a review from lewing as a code ownerMarch 6, 2024 14:48
pavelsavaraand others added 4 commits March 6, 2024 17:36
…stem/Runtime/InteropServices/JavaScript/JSProxyContext.cs
Co-authored-by: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com>
…stem/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Task.cs
Co-authored-by: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com>
@pavelsavara

Copy link
Copy Markdown
MemberAuthor

Filled #99395

@pavelsavara

Copy link
Copy Markdown
MemberAuthor

CI issue is #98721

@pavelsavara
pavelsavara merged commit b0f6444 into dotnet:mainMar 7, 2024
@pavelsavara
pavelsavara deleted the browser_marshal_resoved_tasks_parameters branch March 7, 2024 09:45
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-System.Runtime.InteropServices.JavaScriptos-browserBrowser variant of arch-wasm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@pavelsavara@ilonatommy