Skip to content

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

Merged
merged 3 commits into from
May 21, 2025

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented May 15, 2025

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

@elinor-fung

This comment was marked as outdated.

This comment was marked as outdated.

@elinor-fung

This comment was marked as outdated.

This comment was marked as outdated.

@elinor-fung
Copy link
Member Author

/azp run runtime-coreclr crossgen2 outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@elinor-fung elinor-fung marked this pull request as ready for review May 16, 2025 15:41
Copy link
Contributor

@Copilot Copilot AI left a 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);
Copy link
Preview

Copilot AI May 16, 2025

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.

Suggested change
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.

@elinor-fung
Copy link
Member Author

crossgen2-comparison failure exists in main - filed #115661

@elinor-fung
Copy link
Member Author

elinor-fung commented May 20, 2025

For the AndroidSampleApp with composite R2R, this cuts the amount we copy/use for its images by ~3.4MB (from ~8.3MB copied to ~4.9MB). I'm testing on emulator and did not see a difference in measured startup time.

@elinor-fung elinor-fung merged commit ef640d0 into dotnet:main May 21, 2025
141 of 143 checks passed
@elinor-fung elinor-fung deleted the r2r-emptySections branch May 21, 2025 23:20
@jkotas
Copy link
Member

jkotas commented May 22, 2025

The component assemblies don't have native code, so we can just use their flat layout without loading / mapping them to virtual addresses.

Do we still generate extra index sections like ReadyToRunSectionType::AvailableTypes for the component assemblies and are we able to use these extra index sections at runtime? (I am not able to tell by just looking at the code.)

@elinor-fung
Copy link
Member Author

My understanding is that OwnerCompositeExecutable should be the one and only section for component assemblies. That is what I saw when I was looking at dumped out R2R info for component assemblies and that is what we call out in the R2R format doc for the OwnerCompositeExecutable section. I also didn't see any other section types when I was looking at RewriteComponentFile.

SimaTian pushed a commit that referenced this pull request May 27, 2025
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants