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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:503
   const SampleContextFrameVector &
   getFrameLocationStack(uint64_t Offset, bool UseProbeDiscriminator = false) {
     auto I = Offset2LocStackMap.emplace(Offset, SampleContextFrameVector());
----------------
wlei wrote:
> hoy wrote:
> > How about extend this function with a parameter for caching or not?
> 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. 
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128859