This is an archive of the discontinued LLVM Phabricator instance.

[PGO] Shorten profile symbol prefixes
ClosedPublic

Authored by davidxl on Dec 14 2015, 11:41 AM.

Details

Summary

Unlike profile data section names, profile symbol names (for data, counter and func names) are not part of runtime ABI. They can be changed to anything (given no name conflict) without changing profile format. Current naming scheme is to prefix the owning function name with long prefixes: llvm_profile_name_, llvm_profile_counters_, __llvm_profile_data_. For large C++ programs, this scheme can add to large overhead to the size of non-stripped binary and total input object size to the linker.

This patch shortens the prefixes without losing the symbol name 'readability'.

The impact of this change: Clang profile-instr binary size is reduced by 7M, and the total instrumented object size is reduced by ~40M. For larger apps, the savings can be easily hundreds of mega byes.

The test case changes are mechanical -- they are not included in the review.

Diff Detail

Repository
rL LLVM

Event Timeline

davidxl updated this revision to Diff 42743.Dec 14 2015, 11:41 AM
davidxl retitled this revision from to [PGO] Shorten profile symbol prefixes.
davidxl updated this object.
davidxl added reviewers: bogner, vsk.
davidxl added a subscriber: llvm-commits.

Added missing subscriber.

vsk edited edge metadata.Dec 14 2015, 12:06 PM

This is a good idea. There are quite a few tests in llvm and clang which need updating. The clang changes can come later, but I'd expect s/llvm_profile_/prf_/g-style changes to llvm/test along with this patch.

yes -- I already have the clang test changes ready and verified.

davidxl updated this revision to Diff 42750.Dec 14 2015, 12:10 PM
davidxl edited edge metadata.

Include LLVM test changes as requested.

vsk added a comment.Dec 14 2015, 12:18 PM

Does test/tools/llvm-cov/Inputs/binary-formats.macho32b pass too? I noticed it had a __llvm_prf_names__DATA section.

Every test passes. The name you mentioned is section name which is not changed (see the summary of the patch).

vsk added a comment.Dec 14 2015, 12:59 PM

This lgtm, but please hold off on committing this until everyone's had more time to look at it.

Nice. Thanks. The shortened names are indeed easier to read (in addition to the size savings).

This revision was automatically updated to reflect the committed changes.