-
Notifications
You must be signed in to change notification settings - Fork 5k
Allow using flat layout for component assemblies of composite R2R #115631
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
3413029
to
1d0094a
Compare
/azp run runtime-coreclr crossgen2 outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull Request Overview
This PR enables using a flat layout for component assemblies in composite ReadyToRun images by avoiding unnecessary mapping for assemblies without native code, while also improving section handling. Key changes include:
- Changing the logic for resolving the owner composite executable name in flat layouts.
- Adjusting the relocation and mapping behavior based on whether the image is a component assembly.
- Refactoring section serialization in the R2RPEBuilder to use a new SerializedSectionData structure and streamline section output.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/coreclr/vm/readytoruninfo.cpp | Revised owner composite name resolution and modified relocation checks for component assemblies. |
src/coreclr/vm/peimagelayout.cpp | Updated condition to load images into virtual addresses only when needed. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/SectionBuilder.cs | Added helper to check if a section has content. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/R2RPEBuilder.cs | Refactored section data handling and section serialization logic; made the class sealed. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs | Updated section mapping for R2R headers to use read‑only sections on non‑Windows. |
|
||
// Make sure the section has at least 1 byte, otherwise the PE emitter goes mad, | ||
// messes up the section map and corrups the output executable. | ||
Debug.Assert(extraData != null); |
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.
The change replaces a runtime fallback that ensured a non-empty BlobBuilder with a Debug.Assert, which only triggers in debug builds. In a release build, if _sectionBuilder.SerializeSection unexpectedly returns null or an empty blob, this could lead to invalid output; consider explicitly handling or guarding against null/empty extraData in production.
Debug.Assert(extraData != null); | |
Debug.Assert(extraData != null); | |
if (extraData == null || extraData.Count == 0) | |
{ | |
throw new InvalidOperationException($"SerializeSection returned a null or empty BlobBuilder for section '{name}'."); | |
} |
Copilot uses AI. Check for mistakes.
crossgen2-comparison failure exists in main - filed #115661 |
For the |
Do we still generate extra index sections like |
My understanding is that |
…15631) Allow using flat layout for component assemblies of composite R2R. The component assemblies don't have native code, so we can just use their flat layout without loading / mapping them to virtual addresses. For scenarios where we can't just map from a file for the load - for example, external data (Android), this avoids copying all the data of component assemblies. - Skip emitting empty sections in R2R images - When we had no data for a section, we'd emit a single-byte section. Avoid including the section altogether - Use read-only section for R2R headers - This was using writeable data on non-Windows. We do the mapping ourselves and don't need this to be writeable on any platform. The way we emit R2R PE images, the read-only data goes into .text, so this should not incur the penalty of an extra section. - Relax checks for component assembly initialization to stop requiring a loaded layout - Handle getting the owner composite name for a flat layout of a component assembly of composite R2R
Allow using flat layout for component assemblies of composite R2R. The component assemblies don't have native code, so we can just use their flat layout without loading / mapping them to virtual addresses. For scenarios where we can't just map from a file for the load - for example, external data (Android), this avoids copying all the data of component assemblies.