Add support for pre-compiling interop type maps in crossgen2#124352
Draft
jkoritzinsky wants to merge 15 commits intodotnet:mainfrom
Draft
Add support for pre-compiling interop type maps in crossgen2#124352jkoritzinsky wants to merge 15 commits intodotnet:mainfrom
jkoritzinsky wants to merge 15 commits intodotnet:mainfrom
Conversation
dotnet-policy-service
bot
added
the
linkable-framework
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds support for pre-compiling interop type maps in crossgen2/ReadyToRun images. Instead of processing TypeMap attributes at runtime, the compiler encodes them into native format hash tables similar to how NativeAOT works. The implementation introduces three new ReadyToRun sections (ExternalTypeMaps, ProxyTypeMaps, and TypeMapAssemblyTargets) that store pre-computed type mappings.
Changes:
- Adds native code to read pre-cached type map entries from R2R sections during runtime lookup
- Implements compiler infrastructure to generate type map hash tables in native format
- Modifies TypeMapMetadata collection to support two modes: Traverse (for NativeAOT) and Record (for R2R)
- Bumps ReadyToRun minor version to 18.1
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TypeMapLazyDictionary.cs | Adds QCall methods to query pre-cached type maps and integrates them into the lazy dictionary lookup path |
| src/coreclr/vm/readytoruninfo.h | Declares member variables and methods for the three new R2R sections |
| src/coreclr/vm/readytoruninfo.cpp | Implements hash table lookups for pre-cached external/proxy type maps and assembly targets with critical bugs |
| src/coreclr/vm/qcallentrypoints.cpp | Registers new QCall entry points for type map queries |
| src/coreclr/vm/nativeformatreader.h | Adds string decoding/comparison utilities for native format reader |
| src/coreclr/vm/corelib.h | Adds field mapping for CallbackContext structure |
| src/coreclr/vm/binder.cpp | Adds include for assemblynative.hpp |
| src/coreclr/vm/assemblynative.hpp | Updates CallbackContext native structure and declares new QCall signatures |
| src/coreclr/vm/assemblynative.cpp | Modifies ProcessAttributes to check for and process pre-cached type maps before falling back to attributes |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/TypeMapAssemblyTargetsNode.cs | Generates native hash table encoding assembly targets per type map group |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTypeMapManager.cs | R2R-specific type map manager that creates nodes for each module's type maps |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunProxyTypeMapNode.cs | Encodes proxy type map entries as native format hash tables |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunExternalTypeMapNode.cs | Encodes external type map entries as native format hash tables |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs | Initializes ImportReferenceProvider |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs | Changes return types from ISymbolNode to Import for consistency |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs | Integrates TypeMapManager into per-assembly R2R header generation |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ReadyToRunModuleSignature.cs | New signature type for module helper fixups |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ReadyToRunHeaderNode.cs | Renames HeaderNode to ReadyToRunHeaderNode and adds overload for Add method |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs | Removes incorrect assertion |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ImportReferenceProvider.cs | Provides unified interface for encoding type/module references as R2R import fixups |
| src/coreclr/tools/aot/ILCompiler/Program.cs | Updates TypeMapMetadata.CreateFromAssembly calls with new parameters |
| src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs | Updates TypeMapMetadata.CreateFromAssembly calls with new parameters |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj | Adds new compiler source files to project |
| src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj | Moves TypeMap files to shared Common location |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedTypeMapManager.cs | Refactors to use helper methods for node creation |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs | Updates TypeMapManager integration to use INativeFormatTypeReferenceProvider |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ProxyTypeMapNode.cs | Changes base class to SortableDependencyNode and updates signature |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InvalidProxyTypeMapNode.cs | Changes base class to SortableDependencyNode and updates signature |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InvalidExternalTypeMapNode.cs | Changes base class to SortableDependencyNode and updates signature |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExternalTypeMapNode.cs | Changes base class to SortableDependencyNode and updates signature |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AnalyzedProxyTypeMapNode.cs | Changes base class to SortableDependencyNode and updates signature |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AnalyzedExternalTypeMapNode.cs | Changes base class to SortableDependencyNode and updates signature |
| src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs | Bumps R2R minor version to 18.1 and adds new section type enum values |
| src/coreclr/tools/Common/Internal/Runtime/MetadataBlob.cs | Removes obsolete ExternalTypeMap/ProxyTypeMap blob enum values |
| src/coreclr/tools/Common/Compiler/TypeMapMetadata.cs | Extensive refactoring to support TypeMapAssemblyTargetsMode and pending map merging |
| src/coreclr/tools/Common/Compiler/TypeMapManager.cs | Moves to shared Common location and adds ReadyToRun-specific method |
| src/coreclr/tools/Common/Compiler/ProxyTypeMapObjectNode.cs | Updates to use TypeMapManager and INativeFormatTypeReferenceProvider |
| src/coreclr/tools/Common/Compiler/IProxyTypeMapNode.cs | Changes visibility to internal and conditionally compiles ToAnalysisBasedNode |
| src/coreclr/tools/Common/Compiler/INativeFormatTypeReferenceProvider.cs | New interface for abstracting type/method reference encoding |
| src/coreclr/tools/Common/Compiler/IExternalTypeMapNode.cs | Changes visibility to internal and conditionally compiles ToAnalysisBasedNode |
| src/coreclr/tools/Common/Compiler/ExternalTypeMapObjectNode.cs | Updates to use TypeMapManager and INativeFormatTypeReferenceProvider |
| src/coreclr/tools/Common/Compiler/DependencyAnalysis/SortableDependencyNode.cs | Renames ReadyToRunHeaderNode to GlobalHeaderNode in enum |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/TypeMapLazyDictionary.NativeAot.cs | Updates to use ReadyToRunSectionType instead of ReflectionMapBlob |
| src/coreclr/inc/readytorun.h | Adds new R2R section type enum values and version comment |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs | Adds test coverage for new UsedTypeMapUniverse type map group |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/TypeMapAssemblyTargetsNode.cs
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (3)
src/coreclr/tools/Common/Compiler/ProxyTypeMapObjectNode.cs:17
- This file has an unused
using System.Formats.Tar;import in the header. Even though it’s not referenced by the code here, it can still cause compile breaks if the namespace isn’t available for some target frameworks/configurations, and it adds noise. Please remove the unused using directive.
src/coreclr/tools/Common/Compiler/ExternalTypeMapObjectNode.cs:51 GetSectionnow always returnsObjectNodeSection.DataSection. Previously the section choice followed the external references table (oftenReadOnlyDataSectionon Windows/relative-pointer targets). Forcing writable data can have security/size implications and may change object layout unexpectedly. Consider keeping the prior behavior (e.g., mirror the common fixups table’s section selection) or explicitly justify why RW data is required.
src/coreclr/tools/Common/Compiler/ProxyTypeMapObjectNode.cs:55- Same as ExternalTypeMapObjectNode:
GetSectionis hardcoded toDataSection, which may unnecessarily make the typemap blob writable on platforms that previously placed it inReadOnlyDataSection. Consider restoring the previous section selection logic to avoid regressions in memory protections/layout.
Comment on lines
+13
to
+17
| public interface INativeFormatTypeReferenceProvider | ||
| { | ||
| internal Vertex EncodeReferenceToType(NativeWriter writer, TypeDesc type); | ||
| internal Vertex EncodeReferenceToMethod(NativeWriter writer, MethodDesc method); | ||
| } |
There was a problem hiding this comment.
INativeFormatTypeReferenceProvider is declared as a public interface, but its members are marked internal without providing default implementations. This won’t compile (interface members without bodies can’t have non-public accessibility). Make the interface itself internal, or make the members public (or provide default interface implementations if you intended non-public helpers).
…ap format and move NativeAOT to use the same R2R section identifier for its Type Map support
…or sharing between NativeAOT and R2R.
…tegration for type maps generally
We still have some issues with the hash-table writing (reading entries in some cases seems to result in incorrect reads).
…es with negative offsets. Fix issues in handling invalid maps. Fix an issue in R2RModuleSignature (though that approach may still have issues).
jkoritzinsky
force-pushed
the
typemap-r2r
branch
from
95d4085 to
9a46b19
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
src/coreclr/tools/Common/Compiler/ProxyTypeMapObjectNode.cs:47
- The
Sizeproperty is removed from the interface but still exists in the implementation. TheProxyTypeMapObjectNodeclass declarespublic int Size { get; private set; }at line 47, but after removing theINodeWithSizeinterface, this property is no longer used or set. Remove the unusedSizeproperty.
src/coreclr/tools/Common/Compiler/ExternalTypeMapObjectNode.cs:45 - The
Sizeproperty is removed from the interface but still exists in the implementation. TheExternalTypeMapObjectNodeclass declarespublic int Size { get; private set; }at line 45 and sets it at line 36, but after removing theINodeWithSizeinterface, this property is no longer used. Remove the unusedSizeproperty and the assignment at line 36.
Comment on lines
+201
to
+207
|
|
||
| _ecmaModuleFixupCache = new NodeCache<IEcmaModule, Import>(key => | ||
| { | ||
| return new PrecodeHelperImport( | ||
| _codegenNodeFactory, | ||
| new ReadyToRunModuleSignature(key)); | ||
| }); |
There was a problem hiding this comment.
Duplicate initialization of _ecmaModuleFixupCache. The cache is already initialized in CreateNodeCaches() at lines 188-193, but the same code block appears again inside the ContinuationTypeSymbol method at lines 202-207. This duplicate code is unreachable and should be removed.
Suggested change
| _ecmaModuleFixupCache = new NodeCache<IEcmaModule, Import>(key => | |
| { | |
| return new PrecodeHelperImport( | |
| _codegenNodeFactory, | |
| new ReadyToRunModuleSignature(key)); | |
| }); |
Comment on lines
+42
to
+51
| Vertex INativeFormatTypeReferenceProvider.EncodeReferenceToType(NativeWriter writer, TypeDesc type) | ||
| { | ||
| Import typeImport = GetImportToType(type); | ||
| return writer.GetTuple(writer.GetUnsignedConstant((uint)typeImport.Table.IndexFromBeginningOfArray), writer.GetUnsignedConstant((uint)typeImport.IndexFromBeginningOfArray)); | ||
| } | ||
| internal Vertex EncodeReferenceToType(NativeWriter writer, TypeDesc type) | ||
| { | ||
| Import typeImport = GetImportToType(type); | ||
| return writer.GetTuple(writer.GetUnsignedConstant((uint)typeImport.Table.IndexFromBeginningOfArray), writer.GetUnsignedConstant((uint)typeImport.IndexFromBeginningOfArray)); | ||
| } |
There was a problem hiding this comment.
Duplicate method EncodeReferenceToType with identical implementation. The explicit interface implementation INativeFormatTypeReferenceProvider.EncodeReferenceToType at lines 42-46 and the internal method at lines 47-51 have the exact same body. Remove one of them or clearly document why both are needed if there's a subtle difference.
Comment on lines
+25
to
+32
| private readonly RuntimeType _groupType; | ||
| private LazyExternalTypeDictionary? _externalTypeMap; | ||
| private LazyProxyTypeDictionary? _proxyTypeMap; | ||
| private ExceptionDispatchInfo? _creationException; | ||
|
|
||
| public CallbackContext(RuntimeType groupType) | ||
| { | ||
| _groupType = groupType; |
There was a problem hiding this comment.
The _groupType field in CallbackContext is added but never used in the managed code. While it's set in the constructor (line 32), the native code doesn't appear to read this field either (based on the CallbackContext definition in assemblynative.hpp which shows _groupType as OBJECTREF but no native code accesses it). If this field is intended for future use, add a comment explaining its purpose; otherwise, remove it to avoid confusion.
| @@ -253,7 +253,6 @@ private int ModuleToIndexInternal(IEcmaModule module) | |||
| } | |||
| else | |||
| { | |||
There was a problem hiding this comment.
The assertion that was removed provided validation that cross-module inlineable modules are being tracked. While the comment from PR #124202 explains this was removed because "module lookups can be used without actual image references" (for module fixups), removing the assertion entirely loses important validation for other cases. Consider replacing with a conditional assertion or a comment explaining when this invariant doesn't hold, to help future maintainers understand the expected behavior.
Suggested change
| { | |
| { | |
| // Module lookups can also be used in scenarios (e.g. certain module fixups) | |
| // where there is no actual image reference available. In those cases we | |
| // record a default MVID instead of enforcing that the module must have | |
| // been tracked as "indexable" earlier in the pipeline. |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use the native format to encode the information like in NAOT. For references to types, encode as an
(int, int)pair pointing to an R2R import fixup for a given type handle.This PR precompiles each assembly's type maps independently. It also precompiles the
TypeMapAssemblyTargetsattributes into a hash table mapping the group type to a sequence ofModule*, encoded via fixups with a similar entry formatting to type references (an(int, int)pair pointing to a "helper fixup" with the module override +READYTORUN_HELPER_Module).Fixes #123678
Depends on #124247
Depends on #124202