Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add perfmap debug directory entry to crossgen2 output as needed #58552

Merged
merged 5 commits into from
Sep 10, 2021
Merged

Add perfmap debug directory entry to crossgen2 output as needed #58552

merged 5 commits into from
Sep 10, 2021

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Sep 2, 2021 •
edited
Loading

This change in general contains:

  • Only pass perfmap argument for Linux SPC. We were always passing it down (not all that important)
  • Add PerfMap Debug Directory Entry spec to the COFF spec for SRM.
  • Enhance CrossGen2 to emit PerfMap debug directory record. Tested that I can correlate entries as needed on Windows.
    For example the header in the perfmap is:
FFFFFFFF 00 6819a895-6568-e40a-b8dc-cc7f97c73524
FFFFFFFE 00 1
FFFFFFFD 00 Windows
FFFFFFFC 00 X64
FFFFFFFC 00 CoreRT

and the entries in the PE are:

--- PE FILE: E:\repos\runtime\artifacts\bin\coreclr\windows.x64.Debug\System.Private.CoreLib.dll ---
CodeView
        System.Private.CoreLib.ni.pdb, Age = 1, 19c03592-b136-04bf-12f9-f1a7a2d1efc3
21
        System.Private.CoreLib.r2rmap, Signature = 95 a8 19 68 68 65 0a e4 b8 dc cc 7f 97 c7 35 24, Version = 1
CodeView
        E:\repos\runtime\artifacts\obj\coreclr\System.Private.CoreLib\x64\Debug\System.Private.CoreLib.pdb, Age = 1, 89aa3509-94b5-4f46-8f53-a0fab72a0639
Reproducible
  • Stop emitting an ni.pdb record when PDB is not requested (Linux scenario).

@hoyosjs
Copy link
Member Author

hoyosjs commented Sep 2, 2021

cc: @tommcdon @mangod9 @brianrob

@hoyosjs
Copy link
Member Author

hoyosjs commented Sep 2, 2021

This is related to #58468 on main.

@trylek
Copy link
Member

trylek commented Sep 2, 2021

In your snippet of the PerfMap file above,

FFFFFFFC 00 X64
FFFFFFFC 00 CoreRT

is the duplication of the key (-4) intentional?

trylek
trylek reviewed Sep 2, 2021
View reviewed changes
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mostly looks great to me, I believe I spotted a small copy & paste bug and I hope for feedback from Michal regarding the ABI but I guess you're the one who knows where this information is used. Thank you!

docs/design/specs/PE-COFF.md Show resolved Hide resolved
src/coreclr/tools/aot/ILCompiler.Diagnostics/PerfMapWriter.cs Outdated Show resolved Hide resolved
src/coreclr/tools/aot/ILCompiler.ReadyToRun/CodeGen/ReadyToRunObjectWriter.cs Outdated Show resolved Hide resolved
src/coreclr/tools/aot/ILCompiler.ReadyToRun/CodeGen/ReadyToRunObjectWriter.cs Show resolved Hide resolved
...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DebugDirectoryNode.cs Outdated Show resolved Hide resolved
src/coreclr/tools/r2rdump/R2RDump.cs Show resolved Hide resolved
@runfoapp runfoapp bot mentioned this pull request Sep 3, 2021
Closed
davidwrighton
davidwrighton requested changes Sep 3, 2021
View reviewed changes
Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not attempt to stabilize any constants in the typesystem proper. That is not the appopriate place for stable constants.

src/coreclr/tools/Common/TypeSystem/Common/TargetArchitecture.cs Outdated Show resolved Hide resolved
src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs Outdated Show resolved Hide resolved
src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs Outdated Show resolved Hide resolved
src/coreclr/tools/aot/ILCompiler.Diagnostics/PerfMapWriter.cs Show resolved Hide resolved
@karelz karelz mentioned this pull request Sep 6, 2021
Closed
@hoyosjs hoyosjs force-pushed the juhoyosa/fix-r2rmap-guid branch from 6a06a60 to dc8d8f1 Compare September 9, 2021 05:31
@hoyosjs hoyosjs marked this pull request as ready for review September 9, 2021 05:32
@hoyosjs
Enhance CrossGen2 to emit PerfMap debug directory entry
42fc682 
This is used to correlate PE's with their corresponding PerfMaps. For example the header in the perfmap could be:
```
FFFFFFFF 00 026D4D21B3EE3D93843FF7A964235822
FFFFFFFE 00 1
FFFFFFFD 00 1
FFFFFFFC 00 3
FFFFFFFB 00 1
```
And the PE will have the corresponding entries in the PE as:
```
PerfMap (Type 21):
        System.Private.CoreLib.ni.r2rmap, Signature = 026d4d21b3ee3d93843ff7a964235822, Version = 1
```
@hoyosjs
Emit headers for debug directory entries from the nodes themselves.
de38fc0
@hoyosjs
Add PerfMap Debug Directory Entry spec
72a5289
@hoyosjs
Extend perfmap and r2r documentation
7adb893
@hoyosjs hoyosjs force-pushed the juhoyosa/fix-r2rmap-guid branch from dc8d8f1 to 7adb893 Compare September 10, 2021 07:10
@hoyosjs hoyosjs mentioned this pull request Sep 10, 2021
Merged
@hoyosjs hoyosjs merged commit 561f89a into dotnet:main Sep 10, 2021
@hoyosjs hoyosjs deleted the juhoyosa/fix-r2rmap-guid branch September 10, 2021 20:19
@ghost ghost locked as resolved and limited conversation to collaborators Oct 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@trylek trylek trylek left review comments

@MichalStrehovsky MichalStrehovsky MichalStrehovsky left review comments

@davidwrighton davidwrighton davidwrighton approved these changes

Assignees
No one assigned
Labels
area-crossgen2-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hoyosjs @trylek @davidwrighton @MichalStrehovsky