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.

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

Include runtime flavor and platform information into .mibc Profile #71065

Closed
mdh1418 opened this issue Jun 21, 2022 · 11 comments · Fixed by #71359
Closed

Include runtime flavor and platform information into .mibc Profile #71065

mdh1418 opened this issue Jun 21, 2022 · 11 comments · Fixed by #71359
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@mdh1418
Copy link
Member

mdh1418 commented Jun 21, 2022

With the work to enable PGO-based Profiled AOT on Mono, mono based .mibc files can now be generated. It would be convenient if there was some way to identify the Runtime flavor, Platform OS + Arch for a corresponding .mibc file.

For instance, I had been testing the full pipeline of Profiled AOT on Mono, but unknowingly had generated a coreclr based .mibc (probably accidentally published a coreclr build and collected a .nettrace from the resulting executable). Only after comparing with another .mibc that is more clearly Mono based could we guess that the previous .mibc was a coreclr based one.

/cc: @lambdageek @davidwrighton @lateralusX @tommcdon

@dotnet-issue-labeler
Copy link

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.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 21, 2022
@ghost
Copy link

ghost commented Jun 21, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

With the work to enable PGO-based Profiled AOT on Mono, mono based .mibc files can now be generated. It would be convenient if there was some way to identify the Runtime flavor, Platform OS + Arch for a corresponding .mibc file.

For instance, I had been testing the full pipeline of Profiled AOT on Mono, but unknowingly had generated a coreclr based .mibc (probably accidentally published a coreclr build and collected a .nettrace from the resulting executable). Only after comparing with another .mibc that is more clearly Mono based could we guess that the previous .mibc was a coreclr based one.

/cc: @lambdageek @davidwrighton @lateralusX

Author: mdh1418
Assignees: -
Labels:

area-Diagnostics-coreclr, untriaged

Milestone: -

@lambdageek
Copy link
Member

There are a couple of subtasks here:

  • Extend the mibc format to include optional runtime information - at the very least profile platform/architecture and runtime flavor (coreclr vs mono vs ...). For example I wonder if collecting a profile on coreclr x64 and deploying to arm64 is worth at least a warning.
  • Extend the information in .nettrace files (rundown?) to include platform info and runtime flavor.
  • Make changes to the dotnet-pgo tool to shovel the platform info from .nettrace to .mibc

@lambdageek
Copy link
Member

I don't think we necessarily want a profile to become unusable if it's from the "wrong" runtime/platform, but it would be easier to diagnose profile issues if there was an option for the runtime to report that profile XYZ doesn't match platform/runtime ABC

@EgorBo
Copy link
Member

EgorBo commented Jun 21, 2022

I planned to try to extend the MIBC format too, was planning to add:

  1. versioning
  2. "is it app specific or some common profile" flag
  3. working set size

@tommcdon
Copy link
Member

Adding @davidwrighton @mangod9 who own the mibc format

@davidwrighton
Copy link
Member

This seems pretty good. Adding an extra method to the PGO data to encode this data should be straightforward. I'd like to see it structured as a key/value store. Probably with just strings for keys and values.

@noahfalk noahfalk added the enhancement Product code improvement that does NOT require public API changes/additions label Jun 22, 2022
@lateralusX
Copy link
Member

There are a couple of subtasks here:

  • Extend the mibc format to include optional runtime information - at the very least profile platform/architecture and runtime flavor (coreclr vs mono vs ...). For example I wonder if collecting a profile on coreclr x64 and deploying to arm64 is worth at least a warning.
  • Extend the information in .nettrace files (rundown?) to include platform info and runtime flavor.
  • Make changes to the dotnet-pgo tool to shovel the platform info from .nettrace to .mibc

When closing a session there is a process info event emitted, it includes command line, os info as well as CPU architecture, but currently not runtime flavor.

@tommcdon
Copy link
Member

I planned to try to extend the MIBC format too, was planning to add:

  1. versioning
  2. "is it app specific or some common profile" flag
  3. working set size

@EgorBo has agreed to take a look at adding the runtime flavor info to the MIBC file format. @EgorBo please let me know if any help/assistance is need with adding the runtime flavor to the process info event.

@EgorBo
Copy link
Member

EgorBo commented Jun 27, 2022

When closing a session there is a process info event emitted, it includes command line, os info as well as CPU architecture, but currently not runtime flavor.

Thanks, I see ProcessInfo event that contains OS and Arch.

Adding an extra method to the PGO data to encode this data should be straightforward. I'd like to see it structured as a key/value store. Probably with just strings for keys and values.

@davidwrighton do you mean like

ldstr "key1"
ldstr "value1"
pop
pop
ldstr "key2"
ldstr "value2"
pop
pop
...

?

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 28, 2022
@EgorBo
Copy link
Member

EgorBo commented Jun 28, 2022

#71359

@tommcdon tommcdon added this to the 7.0.0 milestone Jun 28, 2022
@tommcdon tommcdon removed the untriaged New issue has not been triaged by the area owner label Jun 28, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 1, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants