Skip to content

Remove HelperMethodFrames (HMF) from Reflection code paths #108535

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
AaronRobinsonMSFT merged 24 commits into from
Oct 8, 2024
Merged

Remove HelperMethodFrames (HMF) from Reflection code paths #108535

AaronRobinsonMSFT merged 24 commits into from
Oct 8, 2024

Conversation

@AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Oct 3, 2024 •
edited
Loading

Remove RuntimeFieldHandle.GetName() FCall.
Remove RuntimeMethodHandle.GetName() FCall.
Cleanup RuntimeTypeHandle.GetUtf8NameInternal().
Convert TypedReference.MakeTypedReference() to mostly managed.
Convert RuntimeFieldHandle.GetValueDirect() to QCall.
Convert RuntimeFieldHandle.SetValueDirect() to QCall.
Removed unused FCThrow* macros.

@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 10.0.0 milestone Oct 3, 2024
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Oct 3, 2024

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

lambdageek
lambdageek reviewed Oct 3, 2024
View reviewed changes
@build-analysis build-analysis bot mentioned this pull request Oct 4, 2024
Closed
3 tasks
This was referenced Oct 4, 2024
Open
Open
jkotas
jkotas reviewed Oct 4, 2024
View reviewed changes
huoyaoyuan
huoyaoyuan reviewed Oct 4, 2024
View reviewed changes
@AaronRobinsonMSFT
Move CoreCLR and Mono's TypedReference.MakeTypedReference to mostly m…
8e1bed7 
…anaged.

CoreCLR now supports EnC with TypedReference.
This was referenced Oct 5, 2024
Open
Closed
jkotas
jkotas reviewed Oct 5, 2024
View reviewed changes
This was referenced Oct 5, 2024
Open
Closed
Closed
@build-analysis build-analysis bot mentioned this pull request Oct 6, 2024
Open
jkotas
jkotas approved these changes Oct 6, 2024
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

CoreCLR part LGTM. Thank you!

@lambdageek Could you please take a look at the Mono specific part?

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Oct 7, 2024

@lambdageek This should be green now. I needed to add a new ICALL to get the MonoClass* to construct the TypedReference.

lambdageek
lambdageek approved these changes Oct 7, 2024
View reviewed changes
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

Mono bits LGTM. (although I have a couple questions).

You can add an EnC test for TypedReferece.MakeTypedReference near here

runtime/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs

Line 446 in ba28992

This was referenced Oct 8, 2024
Open
Closed
Closed
@AaronRobinsonMSFT AaronRobinsonMSFT mentioned this pull request Oct 8, 2024
Closed
@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Oct 8, 2024

/ba-g Innerloop timeout

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 0f04f6d into dotnet:main Oct 8, 2024
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the hmf_remove_02 branch October 8, 2024 18:48
@am11
Copy link
Member

am11 commented Oct 9, 2024

Excited to see how the QCall magic will work its charm on the tricky reflection endpoints (especially curious about what will replace the HELPER_METHOD_FRAME_BEGIN_RET_PROTECT machinery)! 🚀 Just here on the sidelines as a big fan, cheering for the big conversion showdown! Keep up the awesome work, Aaron! Only 100 more HMFs and 80 FCThrows to go! 😎

@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lambdageek lambdageek lambdageek approved these changes

@jkotas jkotas jkotas approved these changes

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

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

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

+1 more reviewer

@huoyaoyuan huoyaoyuan huoyaoyuan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@AaronRobinsonMSFT AaronRobinsonMSFT

Labels

area-VM-coreclr

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@AaronRobinsonMSFT @am11 @lambdageek @huoyaoyuan @jkotas