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

[Lei Wang via Phabricator via llvm-commits]

wlei 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());
----------------
hoy wrote:
> 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.
Renamed, thanks for the suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128859