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] streamline Task/Promise marshalling - atttemp 2 #93648

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

pavelsavara
Copy link
Member

Second attempt to #93010

This streamlines passing Promise/Task from two cross-boundary calls per promise argument to one.

  • none when it's passed
  • one when it's resolved, rejected or garbage collected

Changes

  • drop create_task_callback
  • simplify complete_task
  • simplify mono_wasm_marshal_promise and rename it to mono_wasm_resolve_or_reject_promise
  • introduced GCVHandle
    • it's like GCHandle in that it keeps proxy of C# object alive while JS owns it.
    • but it's allocated ahead of time on JS side
  • introduced JSVHandle
    • it's like JSHandle in that it keeps proxy of JS object alive while C# owns it.
    • but it's allocated ahead of time on C# side
  • improved GetTaskResultDynamic to handle void Task
  • introduced internal JSObject.DisposeLocal
    • we can dispose both sides during resolve/reject call without extra dispose call

Interop "stack frame"

  • fixed IntPtr and handles to be I32 rather than U32
  • changed MarshalerType from I32 to U8 size
  • added MarshalerType.TaskResolved and MarshalerType.TaskRejected
    • this allows us to pass better metadata in the same JSMarshalerArgument slot
  • moved JSMarshalerArgument.ElementType to different offset
    • to not clash with handles
  • JSMarshalerArgument.ElementType now contains type of the promise's result
    • this is necessary for marshaling with dynamic type
  • changed JSBindingType layout accordingly.
    • In future this could also help with nested generic types.
  • bumped schema version to 2

Other

  • better type for WeakRefInternal<T>
  • interop unit test no longer ForceDisposeProxies in the middle of the program because it can't balance C# side of handles

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

ghost commented Oct 18, 2023

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

Issue Details

Second attempt to #93010

This streamlines passing Promise/Task from two cross-boundary calls per promise argument to one.

  • none when it's passed
  • one when it's resolved, rejected or garbage collected

Changes

  • drop create_task_callback
  • simplify complete_task
  • simplify mono_wasm_marshal_promise and rename it to mono_wasm_resolve_or_reject_promise
  • introduced GCVHandle
    • it's like GCHandle in that it keeps proxy of C# object alive while JS owns it.
    • but it's allocated ahead of time on JS side
  • introduced JSVHandle
    • it's like JSHandle in that it keeps proxy of JS object alive while C# owns it.
    • but it's allocated ahead of time on C# side
  • improved GetTaskResultDynamic to handle void Task
  • introduced internal JSObject.DisposeLocal
    • we can dispose both sides during resolve/reject call without extra dispose call

Interop "stack frame"

  • fixed IntPtr and handles to be I32 rather than U32
  • changed MarshalerType from I32 to U8 size
  • added MarshalerType.TaskResolved and MarshalerType.TaskRejected
    • this allows us to pass better metadata in the same JSMarshalerArgument slot
  • moved JSMarshalerArgument.ElementType to different offset
    • to not clash with handles
  • JSMarshalerArgument.ElementType now contains type of the promise's result
    • this is necessary for marshaling with dynamic type
  • changed JSBindingType layout accordingly.
    • In future this could also help with nested generic types.
  • bumped schema version to 2

Other

  • better type for WeakRefInternal<T>
  • interop unit test no longer ForceDisposeProxies in the middle of the program because it can't balance C# side of handles
Author: pavelsavara
Assignees: pavelsavara
Labels:

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

Milestone: 9.0.0

@pavelsavara pavelsavara changed the title second attempt https://github.com/dotnet/runtime/pull/93010 [browser] streamline Task/Promise marshalling - atttemp 2 Oct 18, 2023
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara marked this pull request as ready for review October 19, 2023 06:47
@pavelsavara pavelsavara requested review from lewing and kg as code owners October 19, 2023 06:47
@pavelsavara pavelsavara requested a review from maraf October 19, 2023 06:48
@pavelsavara pavelsavara merged commit de9aef3 into dotnet:main Oct 19, 2023
128 of 131 checks passed
@pavelsavara pavelsavara deleted the browser_promises_again branch October 19, 2023 13:21
@ghost ghost locked as resolved and limited conversation to collaborators Nov 18, 2023
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