Crossgen2 perfmap format version support

[trylek]

As of

#53792

Crossgen2 newly supports a new command-line option --perfmap-format-version that currently has two legal values, 0 and 1, where 0 corresponds to the "old style Crossgen1" perfmap naming format and 1 is the "new style Crossgen2" naming format. In particular, historically Crossgen1 used to name perfmap files like this:

<assembly-name>.ni.{<MVID of the MSIL file>}.map

This is insufficient and cumbersome in Crossgen2 composite mode where multiple assemblies get compiled into a single native executable; moreover the pre-existing scheme was insufficient anyway as symbol indexation needs to take targeting OS and architecture into consideration and that wasn't reflected in the MVID. The new naming in version 1 is like this:

<assembly-name>.ni.r2rmap

The current POR is that we drop the {MVID} bit and change .map to .r2rmap to avoid clashes with traditional .map files; this will need adjusting the following SDK source files:

https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/ResolveReadyToRunCompilers.cs
https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs
https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs

We also need to adjust the script calling them:

https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

These changes should let SDK make Crossgen2 produce the new perfmap format version 1 and ultimately switch over installer build in the runtime repo to produce the new perfmap style files. According to my understanding the bulk of the follow-up work will involve consuming the metadata in the perfmap file for the purpose of symbol indexation; I believe that should be tracked by a separate work item.

This change may need a counterpart perfview change and possibly additional downstream changes I'm not yet aware of; these will probably merit creating additional specific work items as we discover them.

Thanks

Tomas

/cc @dotnet/crossgen-contrib
@dotnet/dotnet-diag
@tommcdon
@brianrob
@hoyosjs
@mikem8361

Work Needed

• Consume the changes in R2R map generation.
  • SDK needs to consume the flag for new perf format. Add SDK support for perfmap format version sdk#19132
  • Runtime needs to also consume them in their copy of the tasks. Emit V1 of perfmap files in ShareFramework packages #57195
  • ASP needs to consume this in their own custom CrossGen2 runner. Package r2rmap files in crossgened SFX symbol package aspnetcore#35288
• Packaging and indexing.
  • Shared framework SDK needs to glob the new map files so they get properly packaged in symbol transport. Support packing new R2R maps to SFX packages arcade#7738
  • Symbol indexing needs to be updated to get an update in the spec and in the publishing/download tools.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[marcpopMSFT]

@tommcdon to triage and assign if this is needed in net 6.

[davmason]

Tom is out on vacation until August. The perfview changes can happen out of band, we need to know and track any runtime work that happens because we have about a month left.

@trylek @hoyosjs @mikem8361 - how much work is necessary to be in the runtime to fully support the scenario?

[trylek]

I believe we need basically two changes:

1. An SDK change tweaking PrepareReadyToRunCompilation et al and the Microsoft.NET.Publish.targets to accept a new property optionally defining the perfmap version format. SDK should be already consuming Crossgen2 Preview 7 that supports this option. When downlevel targeting .NET 5, it must assume the old naming scheme.

2. In the runtime repo, there are two places that need adjusting: S.P.Corelib build in the coreclr partition and framework crossgenning in the installer partition. These must be conscious changes correlated with the downstream symbol indexation logic; moreover they are blocked on consuming the updated SDK supporting the new property.

Based on these initial assumptions I believe the two changes will first have a chance to combine upon the RC fork in August even though earlier testing should be possible in the SDK repo that receives regular runtime updates.

Thanks

Tomas

[davmason]

Thanks Tomas! Do we have the appropriate work items tracking this? Tom is out and I'm generally unaware of this work item. It doesn't sound like any work from the diagnostics team is needed for 6, but will be out of band in perfview. Is that right?

[trylek]

Well, this was originally intended to be "the work item" tracking the outstanding work but I can easily create two separate ones for the SDK and runtime parts. I can easily make the two fixes in accordance with Tom's suggestion in our related e-mail thread on the issue but it would be great it @hoyosjs and @brianrob could confirm I'm not missing something in the overall plan. I assume that the "work from the diagnostics team" will largely involve setting up the perfmap symbol indexation mechanism but I think it's a completely new feature not necessarily tied to the .NET 6 release.

[hoyosjs]

The symuploader work needs to happen + pipe it through arcade and then have it flow up if we want to index runtime at build time. Outside of that there's this SDK work and any work needed to have it be consumed (and some pending work in the SFX sdk).

[trylek]

I have merged in the SDK change adding support for perfmap format version. @hoyosjs / @brianrob, any suggestions regarding the next step? Should we file a new issue to track the symbol upload / indexation logic in arcade? I'm afraid that the remaining work is pretty arcane for me, it would be great if someone more familiar with this logic could pick it up (apart from the fact that I'm OOF for one more week next week).

[hoyosjs]

You can put it there as there's some work that needs to be done there to the SFX framework too (The globbing needs to be updated). Then runtime and WindowsDesktop use that to crossgen the shared framework I believe. ASP will need to happen separately as they don't use that. Also @trylek, do you know if runtime uses the live crossgen2, or does it use the one in the SDK? Because if so this becomes post-RC1 work.

[hoyosjs]

Added the details of the missing work items to consume this. Except for the last one, I thing all are needed if we are going to get this for .NET 6.

[tommcdon]

@mangod we are transferring this issue to crossgen2 as we discovered there is work left on it. Juan will be updating the checklist with the remaining work

[mangod9]

so this still requires work in 6?

[tommcdon]

@mangod yes this is net6 bug fix work. @hoyosjs is authoring a pr

[mangod9]

@hoyosjs assume this can now be closed based on your PR?

[mangod9]

Closing this issue for now.