-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Apply some obvious rich debug info optimizations #73373
Conversation
When rich debug info is enabled we send MethodDetails events for all inlinees as otherwise no information for those may have been sent (in case they have not been jitted). During rundown this was sending a lot of duplicate events.
* Compress rich debug info when stored in the runtime. For Avalonia.ILSpy which has ~33k methods, the memory overhead of rich debug info goes from 7 MB -> 2.1 MB * ETW events do not use the delta compression, but still optimize them a little by avoiding sending out the padding and unnecessarily large types. The average size of a rich debug info ETW event goes from 259 bytes to 219 bytes for ILSpy
I'll make sure to run the diagnostics tests on this and will verify manually that some basic debugging scenarios work as well. |
WriteToBuffer(&pBuffer, numMappings); | ||
for (uint32_t i = 0; i < numInlineTree; i++) | ||
{ | ||
WriteToBuffer(&pBuffer, inlineTree[i].Method); |
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.
Use of WriteToBuffer is a concern for me.
- The definition of these types does not have comments/description that the field's types are part of a contract that is never permitted to change.
- This is an inefficient encoding. For instance, ILOffset is unlikely to use the full 32 bits that is being sent here, and the Child/Sibling are highly unlikely to be large as well.
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.
We discussed offline. I'm ok with the current set of changes.
The inline ordinals may not be monotonically increasing and the existing DoEncodedDeltaU32 is unnecessarily inefficient for the cases where it isn't (plus, will assert).
Gets us an extra ~10% size reduction in my tests
I added a I've verified that some basic debugging/stepping scenarios work, and that the diagnostics tests pass, with both rich debug info on and off. |
Failure is #73247 |
When rich debug info is enabled we send MethodDetails events for all inlinees as otherwise no information for those may have been sent (in case they have not been jitted). During rundown this was sending a lot of duplicate events, so use a hash set to avoid this. On Avalonia.ILSpy which has around ~33k methods jitted, we go from ~94k MethodDetails events to ~43k MethodDetails events on rundown.
In addition, compress rich debug info when stored in the runtime. For the above scenario the memory overhead of rich debug info goes from 7 MB -> 2.1 MB.
Also optimize the format we use for the ETW events. While we do not use the same delta compression as the runtime storage, we now avoid sending out padding and also use a smaller type for the offset mappings source type. The average size of a rich debug info ETW event goes from 259 bytes to 219 bytes for the above scenario.
The net result is the following for two rundowns of Avalonia.ILSpy (I clicked around a bit and did a few actions before enabling the rundown. EventID(189) is the rich debug info event)

Before:
After:

There should not be any effect of this PR except when rich debug info is enabled.
I would like to get this in .NET 7. There is more work to be done here for .NET 8, e.g. by avoiding storing method handles when we can get it based on IL offset, but this fixes the most glaring issues.