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

[wasm] reduce MethodImplOptions.NoInlining #79457

Merged
merged 4 commits into from
Dec 12, 2022

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Dec 9, 2022

This removes [MethodImplOptions.NoInlining] from JS interop related infrastructure.

It works around #37955 by setting value of out parameters on the native side.
It also removes unnecessary result marshaling in mono_wasm_set_object_property_ref

Related #71425

@pavelsavara pavelsavara added this to the 8.0.0 milestone Dec 9, 2022
@pavelsavara pavelsavara requested review from kg, BrzVlad and maraf December 9, 2022 16:07
@pavelsavara pavelsavara self-assigned this Dec 9, 2022
@ghost
Copy link

ghost commented Dec 9, 2022

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

Issue Details

This removes [MethodImplOptions.NoInlining] from JS interop related infrastructure.

It works around #37955 by setting value of out parameters on the native side.
It also removes unnecessary result marshaling in mono_wasm_set_object_property_ref

Author: pavelsavara
Assignees: pavelsavara
Labels:

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

Milestone: 8.0.0

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@pavelsavara pavelsavara marked this pull request as ready for review December 10, 2022 11:00
@pavelsavara pavelsavara requested a review from lewing as a code owner December 10, 2022 11:00
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

@radical am I causing the WBT failures by changes in this PR ?

@radical
Copy link
Member

radical commented Dec 12, 2022

Unrelated to your changes.

@ilonatommy
Copy link
Member

Looks good.

@pavelsavara pavelsavara merged commit fda0b35 into dotnet:main Dec 12, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 11, 2023
@pavelsavara pavelsavara deleted the wasm_interop_allow_inline branch September 2, 2024 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants