[PATCH] D126344: [memprof] Keep and display symbol names in the RawMemProfReader.

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added a comment.

Thanks for the review.



================
Comment at: llvm/lib/ProfileData/RawMemProfReader.cpp:419
+        if (KeepSymbolName)
+          GuidToSymbolName.insert({Guid, DIFrame.FunctionName});
 
----------------
tejohnson wrote:
> snehasish wrote:
> > tejohnson wrote:
> > > Why not add the function name into the Frame here rather than waiting for the later call to idToFrame() and filling it from the map then?
> > The idea here is to save memory by not duplicating the symbol names since we can have many unique frames (a combination of symbol + location). Also the processing does not require the symbol name today, so we only populate it in the iterator interface where a single MemProfRecord object is instantiated at a time.
> Another alternative, to avoid the repeated map lookups later on, is to make SymbolName a StringRef and keep a StringSet of the unique strings for ownership.
While it would eliminate the map lookup, it would make it very easy to introduce lifetime issues, i.e. if the MemProfRecords outlive the RawMemProfReader the symbol name StringRef would be a dangling pointer. I'll leave it as is for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126344/new/

https://reviews.llvm.org/D126344