Delete extra layers #116405
Delete extra layers #116405
Conversation
jkotas
commented
Jun 7, 2025
jkotas
commented
- Delegate extra layer of metadata APIs
- Delegate extra layer of coreclr directory caching
There was a problem hiding this comment.
Pull Request Overview
This PR removes several extra metadata API layers and redundant shim files to simplify coreclr’s internal caching and assembly metadata access.
- Removed redundant metadata export and shimload headers and functions.
- Replaced multiple metadata API wrappers with direct calls to coreclr methods.
- Cleaned up ancillary code in debugger, binder, and metadata components.
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/assembly.cpp | Removed unneeded metadataexports include. |
| src/coreclr/vm/appdomain.hpp | Removed extra IsBaseLibrary/IsBaseLibrarySatellite functions. |
| src/coreclr/vm/appdomain.cpp | Simplified system directory retrieval and base library path setup. |
| src/coreclr/md/compiler/regmeta.cpp | Consolidated metadata reopening functions and fixed a typo. |
| src/coreclr/inc/* | Removed shimload and metadataexports headers. |
| src/coreclr/debug/* | Removed references to removed metadata API layers. |
| src/coreclr/binder/utils.cpp | Added forward declaration for RuntimeFileNotFound. |
| src/coreclr/ilasm/* and src/coreclr/dlls/mscoree/* | Removed unused includes for removed metadata layers. |
Comments suppressed due to low confidence (2)
src/coreclr/vm/appdomain.cpp:884
- The removal of the conditional block that appends the trailing directory separator may lead to incorrect path concatenation if GetClrModuleDirectory does not guarantee a trailing separator. Please confirm that GetClrModuleDirectory always returns a canonicalized directory with a trailing separator.
m_BaseLibrary.Append(g_pwBaseLibrary);
src/coreclr/debug/di/module.cpp:1040
- The updated call to MDReOpenMetaDataWithMemory replaces ReOpenMetaDataWithMemoryEx; please verify that this new API fully replicates the old behavior, especially in terms of flags and error handling.
hr = MDReOpenMetaDataWithMemory(m_pIMImport, pLocalMetaDataPtr, dwMetaDataSize, ofTakeOwnership );
|
Tagging subscribers to this area: @mangod9 |
| PRECONDITION(CheckPointer(ppv)); | ||
| } CONTRACTL_END; | ||
|
|
||
| return GetMDInternalInterface(pData, cbData, flags, riid, ppv); |
There was a problem hiding this comment.
The exact same thing was exposed under two different names. None of this is exported in CoreCLR, so it is fine to delete the extra layer.
Delegate extra layer of coreclr directory caching
| @@ -140,7 +140,6 @@ LEAF_END RhpGcPoll, _TEXT | |||
|
|
|||
| NESTED_ENTRY RhpGcPollRare, _TEXT | |||
| PUSH_COOP_PINVOKE_FRAME rcx | |||
| END_PROLOGUE | |||
There was a problem hiding this comment.
Fixing unrelated build warning (END_PROLOGUE is called by PUSH_COOP_PINVOKE_FRAME, so this one is redundant)
There was a problem hiding this comment.
There are two more like these:
runtime/src/coreclr/runtime/amd64/AllocFast.S
Lines 80 to 81 in 0f2bfae
runtime/src/coreclr/runtime/amd64/AllocFast.S
Lines 260 to 261 in 0f2bfae
There was a problem hiding this comment.
Those should obviously be fixed too even if it makes no codegen difference (END_PROLOGUE is no-op on non-Windows).
Co-authored-by: Aaron Robinson <[email protected]>
