Skip to content

Print R2RDump Statistics in the Output File Instead of Console - #52278

Merged
ivdiazsa merged 3 commits into
dotnet:mainfrom
ivdiazsa:JewelRetrieval
May 20, 2021
Merged

Print R2RDump Statistics in the Output File Instead of Console#52278
ivdiazsa merged 3 commits into
dotnet:mainfrom
ivdiazsa:JewelRetrieval

Conversation

@ivdiazsa

@ivdiazsaivdiazsa commented May 4, 2021

Copy link
Copy Markdown
Contributor

This PR fixes#49249. When running R2RDump.exe/dll, the Fixup count statistics were shown in the console at the end. The intended behavior was to add them at the end of the resulting --out file instead.

This PR addresses this issue, and adds a little useful feature. Instead of hardcoding the table's format, its padding is now generated from the contents that will be printed in it. This allows it to always display properly, regardless of the fields and numbers stored, and therefore is now a self-maintaining component.

Before this change:

=========== Eager fixup counts across all methods ===========
Fixup | Count
Verify_FieldOffset | 655074
TypeHandle | 243894
Verify_TypeLayout | 122611
StringHandle | 106559
MethodDictionary | 5465
IndirectPInvokeTarget | 1007
TypeDictionary | 950
Check_InstructionSetSupport | 736
MethodHandle | 277
PInvokeTarget | 88
MethodEntry_DefToken | 12
FieldHandle | 8
-----------------------------------
Total | 1136681
=============================================================

After this change:

=========== Eager fixup counts across all methods ===========
Fixup | Count
Verify_FieldOffset | 655074
TypeHandle | 243894
Verify_TypeLayout | 122611
StringHandle | 106559
MethodDictionary | 5465
IndirectPInvokeTarget | 1007
TypeDictionary | 950
Check_InstructionSetSupport | 736
MethodHandle | 277
PInvokeTarget | 88
MethodEntry_DefToken | 12
FieldHandle | 8
-------------------------------------
Total | 1136681
=============================================================

@ghostghost added the area-ReadyToRun label May 4, 2021
@ivdiazsa
ivdiazsa requested review from mangod9 and trylekMay 4, 2021 23:37
Comment threadsrc/coreclr/tools/r2rdump/TextDumper.cs Outdated

@trylektrylek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very nice, thank you!

* In order to format the fixup counts results table, we need to
* know beforehand the size of each column. */

int longestFixupKind = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-blocking nit - you could rename this variable to "fixupPadding", initialize it to 5 and delete the line 595. If you're more comfortable with your formulation, feel free to keep it.

@runfoapprunfoappBot mentioned this pull request May 5, 2021
@ivdiazsa
ivdiazsa merged commit 80c81b3 into dotnet:mainMay 20, 2021
@ghostghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

R2RDump newly displays some weird fixup statistics summary, maybe caused by my CG2 map changes

3 participants

@ivdiazsa@trylek@mangod9