-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Remove HELPER_METHOD_FRAMEs from built-in marshalers #97432
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
Remove HELPER_METHOD_FRAMEs from built-in marshalers #97432
Conversation
…n managed code instead of FCALLs
17b9d4f
to
2fab541
Compare
2fab541
to
5e59fc9
Compare
…or marshalers that already have the state type defined in managed code.
…le, fix test failures.
…he custom marshaler object itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM when green.
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
src/coreclr/vm/ilmarshalers.cpp
Outdated
|
||
BASEARRAYREF arrayRef = (BASEARRAYREF) *pManagedHome; | ||
BASEARRAYREF arrayRef = NULL; | ||
GCPROTECT_BEGIN(arrayRef); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: You do not need to GCPROTECT the copy of arg in situations like this. It is cheaper to do pManagedHome.Get
multiple times instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
…/runtime into helper-method-frame-removal
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Failures are known. Merging this in. |
The built-in marshalers that were implemented in native code used FCalls. Change them to use QCalls or (in the case of some simple methods that don't marshal and the
ICustomMarshaler
marshaler), move the implementation to managed code.