Instead of writing out InstrProfRecords which contain only 0 counts, skip them.
The Xcode team asked for this so that the (often sparse) profiles gathered from UI tests take up less space.
Paths
| Differential D16727
[Profiling] Write out sparse indexed profiles ClosedPublic Authored by vsk on Jan 29 2016, 8:41 AM.
Details Summary Instead of writing out InstrProfRecords which contain only 0 counts, skip them. The Xcode team asked for this so that the (often sparse) profiles gathered from UI tests take up less space.
Diff Detail Event Timelinevsk updated this object. Comment Actions Any reason why we can not just always emit sparse output ?
Comment Actions Making sparse the default would change the output of llvm-profdata show since zero count functions would no longer be listed. Is that a concern? In other words, could existing users of llvm-profdata be relying on it in any way?
Comment Actions If there is to be a new option for llvm-profdata then the *.rst documentation file needs to be updated too: Comment Actions I think we should keep the non-sparse mode. Users of llvm-profdata might rely on having records for functions which don't execute (though I imagine this is a small group). In addition, one of our tests assumes this (weight-instr.test). Having -sparse turned on by default seems fair to me.
Comment Actions I have one major concern with this patch: When the records for those zero count functions are skipped, the profile-use compilation may get confused -- those functions are now treated as 'unknown functions' which the compiler can not do anything about them. One of the main benefit of keeping zero count functions is that the compiler can do very aggressive size optimizations on them. If we go with this route, we need a way to store place holder records for the cold functions. If we do care about size of indexed profile data, name compression will be very efficient way. My pending patch does not touch Indexed profile format, but it can be easily extended to Indexed format (and also keeping the ability to read the old format files).
Comment Actions (Sorry - typo earlier, slingn's*.)
Comment Actions Good point. Would making -sparse opt-in help with this? That way if someone really cares about trimming down the profdata file (e.g if optimization is not a concern), they have one more option. Having this in addition to your name compression patch can't hurt. Comment Actions
This revision is now accepted and ready to land.Jan 29 2016, 2:34 PM Closed by commit rL259258: [Profiling] Add a -sparse mode to llvm-profdata merge (authored by vedantk). · Explain WhyJan 29 2016, 2:58 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 46419 docs/CommandGuide/llvm-profdata.rst
include/llvm/ProfileData/InstrProfWriter.h
lib/ProfileData/InstrProfWriter.cpp
test/tools/llvm-profdata/general.proftext
test/tools/llvm-profdata/weight-instr.test
tools/llvm-profdata/llvm-profdata.cpp
unittests/ProfileData/CoverageMappingTest.cpp
unittests/ProfileData/InstrProfTest.cpp
|
I'm also not sure about 'Useful' as the name - since zeros are useful - just not necessary to write out.
How about shouldEncodeData()?