Move UMEntryThunk to StubPrecode infrastructure #112460
Conversation
davidwrighton
commented
Feb 12, 2025
•
davidwrighton
commented
- Use the StubPrecode logic to create UMEntryThunk instances
- Allocate them all on the Global LoaderAllocator
- Unify the creation of UMEntryThunks between the C++/CLI fixup generation path and the delegate creation path
- Remove ThunkHeapStubManager as it is now subsumed by RangeSectionStubManager
- Tweak code paths around all of this so that we don't have unnecessary indirections (This change will add 2 memory loads to the UMEntryThunk path on x86/x64, but that should be fairly immaterial compared to the cost of an actual reverse p/invoke)
- Use the StubPrecode logic to create UMEntryThunk instances - Allocate them all on the Global LoaderAllocator - Unify the creation of UMEntryThunks between the C++/CLI fixup generation path and the delegate creation path - Remove ThunkHeapStubManager as it is now subsumed by RangeSectionStubManager - Tweak code paths around all of this so that we don't have unnecessary indirections (This change will add 2 memory loads to the UMEntryThunk path on x86/x64, but that should be fairly immaterial compared to the cost of an actual reverse p/invoke)
…he UMEntryThunks to not successfully allocate/work
There was a problem hiding this comment.
Copilot reviewed 52 out of 52 changed files in this pull request and generated no comments.
davidwrighton
changed the title
| case PRECODE_STUB: | ||
| case PRECODE_NDIRECT_IMPORT: | ||
| #ifdef FEATURE_INTERPRETER | ||
| case PRECODE_INTERPRETER: |
There was a problem hiding this comment.
The interpreter precode actually doesn't store MethodDesc there, but rather the IR bytecode address. It is possible to get the MethodDesc via the CodeHeader stored right before the bytecode, but we should not come down this path for the interpreter precode, so I'd rather assert here and return nullptr for that case.
|
|
||
| #ifdef PROFILING_SUPPORTED | ||
| #ifdef PROFILING_SUPPORTED |
There was a problem hiding this comment.
A nit - I guess the #ifdef / #endif were indented by accident, right?
| PCODE Target; | ||
| BYTE Type; | ||
| TADDR Type; |
There was a problem hiding this comment.
Do we need TADDR here? It doesn't matter from the size perspective, but I was thinking we might possibly find some use for the remaining 3 bytes. But I am fine with having TADDR there.
| pUMEntryThunk = pPrecode->AsUMEntryThunk(); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| // Lookup the callsite in the hash, if found, we can map this call back to its managed function. |
There was a problem hiding this comment.
This comment needs updating. There is no lookup in the hash anymore.
| } | ||
| } | ||
| } | ||
| #endif // !DACCESS_COMPILE | ||
|
|
||
| OBJECTHANDLE GetObjectHandleSpeculative() const |
There was a problem hiding this comment.
What is the situation that the Speculative version allows that the non-speculative version does not allow?
There was a problem hiding this comment.
(It is not obvious to me why we need the speculative variant.)
| @@ -175,9 +175,6 @@ End | |||
| Crst DbgTransport | |||
| End | |||
|
|
|||
| Crst DelegateToFPtrHash | |||
There was a problem hiding this comment.
Renerate crsttypes_generated.h
