[llvm-profgen] Do not cache the frame location stack during computing inlined context size

[wlei]

Summary

In computeInlinedContextSizeForRange, the offset of range is only used one time, there is no need to cache the frame location stack.
Measured on one internal service binary, this can save 2GB memory usage and reduce a small run time (avoid one hash search).

[hoy]

Nice catch!

llvm/tools/llvm-profgen/ProfiledBinary.h

503

How about extend this function with a parameter for caching or not?

[wlei]

llvm/tools/llvm-profgen/ProfiledBinary.h

503

It seems the return types of the two functions should be different, the one without caching should return the value type instead of the reference as we don't store it to a map.

[hoy]

llvm/tools/llvm-profgen/ProfiledBinary.h

503

I see. Then it makes sense to leave them separate.

How about flip the names, i.e

getFrameLocationStackWithoutCaching -> getFrameLocationStack

getFrameLocationStack -> getCachedFrameLocation

Caching is expansive so we want users to be explicitly aware of it. Also perhaps in the future we want to use two containers for cached contexts, one for UseProbeDiscriminator==true and one for UseProbeDiscriminator==false.

[wlei]

llvm/tools/llvm-profgen/ProfiledBinary.h

503

Renamed, thanks for the suggestion!

[wlei]

renaming according to reviewer's suggestion

[hoy]

llvm/tools/llvm-profgen/ProfiledBinary.h

509

nit: getCachedFrameLocation -> getCachedFrameLocationStack

Sorry for missing it previously.

[wlei]

Updating D128859: [llvm-profgen] Do not cache the frame location stack during computing inlined context size

[wenlei]

Good find, thanks for the memory improvement!

llvm/tools/llvm-profgen/ProfiledBinary.h

public:
 
   uint32_t getFuncSizeForContext(const ContextTrieNode *ContextNode) {
     return FuncSizeTracker.getFuncSizeForContext(ContextNode);
   }
 
   // Load the symbols from debug table and populate into symbol list.
   void populateSymbolListFromDWARF(ProfileSymbolList &SymbolList);
 
+  SampleContextFrameVector
+  getFrameLocationStackWithoutCaching(uint64_t Offset,
+                                      bool UseProbeDiscriminator = false) {
+    InstructionPointer IP(this, Offset);
+    return symbolize(IP, true, UseProbeDiscriminator);
+  }
+
   const SampleContextFrameVector &
   getFrameLocationStack(uint64_t Offset, bool UseProbeDiscriminator = false) {
     auto I = Offset2LocStackMap.emplace(Offset, SampleContextFrameVector());
-    if (I.second) {
-      InstructionPointer IP(this, Offset);
-      I.first->second = symbolize(IP, true, UseProbeDiscriminator);
-    }
+    if (I.second)
+      I.first->second =
+          getFrameLocationStackWithoutCaching(Offset, UseProbeDiscriminator);
     return I.first->second;
   }
 
   Optional<SampleContextFrame> getInlineLeafFrameLoc(uint64_t Offset) {
     const auto &Stack = getFrameLocationStack(Offset);
     if (Stack.empty())
       return {};
     return Stack.back();
   }
 

llvm/tools/llvm-profgen/ProfiledBinary.cpp

if (IP.Address != RangeBegin)
     WithColor::warning() << "Invalid start instruction at "
                          << format("%8" PRIx64, RangeBegin) << "\n";
 
   if (IP.Address >= RangeEnd)
     return;
 
   do {
     uint64_t Offset = virtualAddrToOffset(IP.Address);
-    const SampleContextFrameVector &SymbolizedCallStack =
-        getFrameLocationStack(Offset, UsePseudoProbes);
+    SampleContextFrameVector SymbolizedCallStack =
+        getFrameLocationStackWithoutCaching(Offset, UsePseudoProbes);
     uint64_t Size = Offset2InstSizeMap[Offset];
 
     // Record instruction size for the corresponding context
     FuncSizeTracker.addInstructionForContext(SymbolizedCallStack, Size);
 
   } while (IP.advance() && IP.Address < RangeEnd);
 }