-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Integrate mibc mono config extension #71657
Integrate mibc mono config extension #71657
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
1e4edb1
to
bd6e6a0
Compare
It looks like the Runtime value still shows up as |
@mdh1418, your change to add 0x4 to the With regard to making the value "Mono" show up properly, you will want to update https://github.com/microsoft/perfview/blob/main/src/TraceEvent/Parsers/ClrTraceEventParser.cs#L12694. This will cover PerfView and TraceEvent, and then any tools that pick-up new versions of TraceEvent. |
Yes, I believe the Sku value 0x4 makes it to the .nettrace through a rundown event runtime/src/mono/mono/eventpipe/ep-rt-mono.c Line 2872 in cc0ccbe
runtime/src/coreclr/tools/dotnet-pgo/Program.cs Lines 1776 to 1777 in cc0ccbe
Thanks, I'll make changes to ClrTraceEventParser and see how it reflects locally! |
@mdh1418 sounds good. What you've described sounds right to me. You will want to make the change to TraceEvent locally and then patch in the updated DLL where dotnet-pgo picks it up. You can do this all locally, and assuming it works, then you'd just need to take a new version of TraceEvent into dotnet-pgo. |
@brianrob I was able to verify that the MibcConfig section of the .mibc profile reflected I can put up a PR on perfview with the single line change and we can update the TraceEventVersion used in this repo to whichever version that PR makes it to. |
Sounds great! |
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.
LGTM! Couple of nits.
1cf769f
to
c9efb02
Compare
Follow up to #71359.
With .mibc profiles now containing a MibcConfig, there are a couple of places that can use some modifications to better integrate this change.
4
)This PR makes the following changes:
0x4
as"Mono"
(follow up on Add a config to MIBC format #71359 (comment))