[PATCH] D118515: [llvm-profgen] On-demand track optimized-away inlinees for preinliner.
Hongtao Yu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 7 08:56:54 PST 2022
hoy added inline comments.
================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:608-609
+
+ // Flush the symbolizer to save memory.
+ Binary->flushSymbolizer();
}
----------------
wenlei wrote:
> It is weird to have profile generator flush symbolizer of ProfileBinary, how much saving is this comparing to on-demand context size tracking?
>
> I assume the symbolization here is only needed for dwarf base profile, can we free symbolizer for probe profile generation right after ProfiledBinary::load?
The symbolizer consumes quite some memory and it's needed in both probe and dwarf case to calculate code sizes for inlinees. Perhaps I should just reset the symbolizer pointer instead of flushing it.
================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:351
+ if (UsePseudoProbes && TrackFuncContextSize) {
+ for (const auto &Child : ProbeDecoder.getDummyInlineRoot().getChildren()) {
+ auto *Frame = Child.second.get();
----------------
wenlei wrote:
> Actually ProbeDecoder.getDummyInlineRoot().getChildren() is already a map, wondering can we make it possible to look up by function name without building an extra map?
>
> I can see that the key is Guid, ProbeId pair - we can get Guid from names, but ProbeId under dummy root is index. What was the reason for top level nodes to have different probe Id instead of a dummy probe Id? For top level, we don't expect same name to appear more than once.
Top level nodes should have a dummy probe id, i.e, 0. The key in ProbeDecoder.getDummyInlineRoot().getChildren() is the guid. and we need a guid to look up a node in the children map. Unfortunately, there isn't a func name to guid map available currently. What we have is the `GUID2FuncDescMap`. So I'm building a name to top level node map. Alternatively we can build a name to guid map here but that'll incur two hash lookups to get the node. Since either map is only used by llvm-profgen, I'm not placing the map to `MCPseudoProbe` which is also shared by the Bolt probe decorder.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118515/new/
https://reviews.llvm.org/D118515
More information about the llvm-commits
mailing list