[PATCH] D120860: [memprof] Filter out callstack frames which cannot be symbolized.

Thu Mar 3 16:40:00 PST 2022

snehasish added a comment.

PTAL, thanks!

================
Comment at: llvm/lib/ProfileData/RawMemProfReader.cpp:336
+    auto &CallStack = Entry.getSecond();
+    CallStack.erase(std::remove_if(CallStack.begin(), CallStack.end(),
+                                   [&AllVAddrsToDiscard](const uint64_t A) {
----------------
tejohnson wrote:
> Rather than scan CallStack again, can you keep a flag initialized to false above the prior loop, set to true if any entries are symbolized, and check that here?
In practice all callstacks have at least one item to discard which originates from a shared library, i.e. the call to main from glibc at `__libc_start_main in libc-start.c` so adding a flag here doesn't help. 

An alternative way to filter would be to use the procmaps to identify all addresses within the main binary and only retain those while reading the raw profile, however we don't record build ids on linux during profiling today (sanitizer_procmaps_linux does not support it). We could use a heuristic to identify the main binary but matching it to the profiled binary (passed in during `llvm-profdata merge`) build id would be best. 

================
Comment at: llvm/test/tools/llvm-profdata/memprof-basic.test:45
 CHECK-NEXT:     NumSegments: 9
 CHECK-NEXT:     NumMibInfo: 3
 CHECK-NEXT:     NumStackOffsets: 3
----------------
tejohnson wrote:
> Should this now be 2? Ditto for NumStackOffsets below.
This is coming from the raw profile without symbolization and pruning. If you don't feel strongly, I'll update the summary code in a separate patch. I've added a TODO in RawMemProfReader.cpp for now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120860