This is an archive of the discontinued LLVM Phabricator instance.

[dsymutil] Print statistics about debug_info reduction in verbose mode.
ClosedPublic

Authored by JDevlieghere on May 6 2020, 12:12 PM.

Details

Summary

This patch adds statistics about the the contribution of each object file to the linked debug info to verbose mode. After linking it prints a table as illustrated below with the object file, the old debug info size, the new debug info size and the percentage difference. The table is sorted by the output size, so the object files contributing the most to the link are listed first.

-------------------------------------------------------------------------------
Object File                                    Bytes Input -> Output     Change
-------------------------------------------------------------------------------
basic2.macho.x86_64.o                                  210 -> 165        24.00%
basic3.macho.x86_64.o                                  177 -> 150        16.51%
basic1.macho.x86_64.o                                  125 -> 129        3.15%
-------------------------------------------------------------------------------
Total                                                  512 -> 444        14.23%
-------------------------------------------------------------------------------

Diff Detail

Event Timeline

JDevlieghere created this revision.May 6 2020, 12:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2020, 12:12 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
JDevlieghere marked an inline comment as done.
friss accepted this revision.May 6 2020, 1:20 PM

LGTM, with one potential suggestion:

llvm/lib/DWARFLinker/DWARFLinker.cpp
2533

Should we be using a different --stat flag for this? The verbose flag will output so much info on a real link...

This revision is now accepted and ready to land.May 6 2020, 1:20 PM
JDevlieghere marked 2 inline comments as done.May 6 2020, 1:34 PM
JDevlieghere added inline comments.
llvm/lib/DWARFLinker/DWARFLinker.cpp
2533

Not to mention that verbose mode implies single threaded, which is the reason I put it under verbose. If we want to use --stats we'll need a way to synchronize the printing across architectures for fat binaries. Do you think that's worth it?

friss added inline comments.May 6 2020, 1:52 PM
llvm/lib/DWARFLinker/DWARFLinker.cpp
2533

Do we need to synchronize? Maybe we should just print the current architecture at the beginning of the line to disambiguate?

JDevlieghere marked 3 inline comments as done.May 6 2020, 2:00 PM
JDevlieghere added inline comments.
llvm/lib/DWARFLinker/DWARFLinker.cpp
2533

We do, the output is all intertwined otherwise. I actually implemented the --stats thing you suggested before moving it under verbose for exactly this reason.

JDevlieghere marked an inline comment as done.

Make statistics a new option.

JDevlieghere edited the summary of this revision. (Show Details)May 6 2020, 4:39 PM
JDevlieghere edited the summary of this revision. (Show Details)

Fix aligment

This revision was automatically updated to reflect the committed changes.
avl added a comment.May 7 2020, 3:45 AM

Probably, it makes sense to not put printing code into DWARFLinker? i.e. add callbacks to DwarfFile:

class DwarfFile {
...

std::function <(StringRef FileName, StringRef TableName, uint64_t TableFinalSize)> StatisticsCallback;

};

or to add callback to DWARFlinker:

/// Report statistics.
void setStatisticCallback ( std::function <(StringRef FileName, StringRef TableName, uint64_t TableFinalSize)> StatisticsCallback );

So, that DWARFlinker reports statistics, but analyzing and displaying would be done separately.
In this case in dsymutil`s DwarfLinkerForBinary.cpp. What do you think?

aprantl added inline comments.May 7 2020, 9:52 AM
llvm/docs/CommandGuide/dsymutil.rst
102

typo: object

Does this count .debug_info only, or also the other sections?

How does type uniquing factor into this? Are we counting the input or the actual contribution?

llvm/include/llvm/DWARFLinker/DWARFLinker.h
561

Can you please document the return value?

llvm/lib/DWARFLinker/DWARFLinker.cpp
42

of the debug_info *section*?

2543

using a float to store the result of an operation on 64-bit integers sounds risky to me. Should we at least use a double?

2545

I'd feel more comfortable if we replaced this with
if (Input == 0 && Output == 0) or with if (Sum < epsilon) or something that doesn't rely on floating point landing on exact zero

basic3.macho.x86_64.o                                  177 -> 150        16.51%
basic1.macho.x86_64.o                                  125 -> 129        3.15%

Could we right-align the numbers?