KEMBAR78
Use emit-based invoke for all of WASM including DI by steveharter · Pull Request #85065 · dotnet/runtime · GitHub
Skip to content

Use emit-based invoke for all of WASM including DI #85065

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 4 commits into from
Apr 24, 2023
Merged

Use emit-based invoke for all of WASM including DI #85065

merged 4 commits into from
Apr 24, 2023

Conversation

@steveharter
Copy link
Contributor

@steveharter steveharter commented Apr 19, 2023 •
edited
Loading

Makes the newer emit-based reflection added to Mono in .NET 8.0 also support Blazor\WASM.

Also addresses main requirement for #66153 which is to allow ASP.NET to use DI's ActivatorUtilities.CreateFactory without a ~259K size regression (measuring size of .br files).

The perf results for using an emit-based invoke instead of interpreted are not as good as the 2x-3x we saw for Core and Mono, but the expected overall improvement should be ~20-30% with the largest gain by far being a zero-arg constructor which is ~2.2x faster. Once the new fast invoke APIs are ready (which are based on emit), those are expected to further improve most cases; these will be used by DI once available.

Measurements below are in ms and for 1,000,000 iterations. Also includes the object[] parameters alloc for MethodBase.Invoke() for cases that have parameters.

Test Before After Ratio
constructor (0 args) 430 195 2.21
constructor (3 args) 2015 1660 1.21
constructor (5 args) 2585 2293 1.13
method (0 args) 1008 718 1.40
method (3 args) 2601 2322 1.12
method (5 args) 3218 2918 1.10
set property 1730 1420 1.22
get property 111 108 1.03

@steveharter steveharter self-assigned this Apr 19, 2023
@ghost
Copy link

ghost commented Apr 19, 2023

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

Makes the newer emit-based reflection added to Mono in .NET 8.0 also support Blazor\WASM. (perf numbers to come in a bit).

Also addresses main requirement for #66153 which is to allow ASP.NET to use DI's ActivatorUtilities.CreateFactory without a ~259K size regression (measuring size of .br files).

Author: steveharter
Assignees: steveharter
Labels:

area-System.Reflection
Milestone: -

@steveharter steveharter added this to the 8.0.0 milestone Apr 19, 2023
@ghost
Copy link

ghost commented Apr 19, 2023

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

Makes the newer emit-based reflection added to Mono in .NET 8.0 also support Blazor\WASM. (perf numbers to come in a bit).

Also addresses main requirement for #66153 which is to allow ASP.NET to use DI's ActivatorUtilities.CreateFactory without a ~259K size regression (measuring size of .br files).

Author: steveharter
Assignees: steveharter
Labels:

area-System.Reflection, tenet-performance
Milestone: -

@lambdageek lambdageek requested a review from maraf April 19, 2023 19:53
@lambdageek lambdageek added the arch-wasm WebAssembly architecture label Apr 19, 2023
@ghost
Copy link

ghost commented Apr 19, 2023

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

Issue Details

Makes the newer emit-based reflection added to Mono in .NET 8.0 also support Blazor\WASM. (perf numbers to come in a bit).

Also addresses main requirement for #66153 which is to allow ASP.NET to use DI's ActivatorUtilities.CreateFactory without a ~259K size regression (measuring size of .br files).

Author: steveharter
Assignees: steveharter
Labels:

arch-wasm, area-System.Reflection, tenet-performance
Milestone: 8.0.0

@lambdageek lambdageek requested a review from ivanpovazan April 19, 2023 19:54
@ghost
Copy link

ghost commented Apr 19, 2023

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Makes the newer emit-based reflection added to Mono in .NET 8.0 also support Blazor\WASM. (perf numbers to come in a bit).

Also addresses main requirement for #66153 which is to allow ASP.NET to use DI's ActivatorUtilities.CreateFactory without a ~259K size regression (measuring size of .br files).

Author: steveharter
Assignees: steveharter
Labels:

arch-wasm, area-System.Reflection, tenet-performance, os-android
Milestone: 8.0.0

@lewing lewing requested review from pavelsavara and vargaz April 20, 2023 01:14
@steveharter steveharter marked this pull request as ready for review April 21, 2023 17:46
@build-analysis build-analysis bot mentioned this pull request Apr 21, 2023
Closed
buyaa-n
buyaa-n approved these changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

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

LGTM, the comment here was helpful for understanding better

@steveharter steveharter merged commit d11ed57 into dotnet:main Apr 24, 2023
@steveharter steveharter deleted the BlazorDi branch April 24, 2023 14:09
@steveharter steveharter mentioned this pull request Apr 24, 2023
Closed
3 tasks
@build-analysis build-analysis bot mentioned this pull request Apr 25, 2023
Closed
This was referenced May 2, 2023
Open
Open
@ghost ghost locked as resolved and limited conversation to collaborators May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lambdageek lambdageek lambdageek left review comments

@maraf maraf Awaiting requested review from maraf

@ivanpovazan ivanpovazan Awaiting requested review from ivanpovazan

@pavelsavara pavelsavara Awaiting requested review from pavelsavara

@vargaz vargaz Awaiting requested review from vargaz

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

@jkotas jkotas Awaiting requested review from jkotas

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

+1 more reviewer

@buyaa-n buyaa-n buyaa-n approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@steveharter steveharter

Labels

arch-wasm WebAssembly architecture area-System.Reflection tenet-performance Performance related issue

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@steveharter @lambdageek @buyaa-n