[PATCH] D107800: [CSSPGO][llvm-profgen] Cap context stack to reduce memory usage

Wed Aug 11 15:08:49 PDT 2021

hoy accepted this revision.
hoy added inline comments.

================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:73

+int CSProfileGenerator::MaxContextDepth = -1;
+
----------------
wlei wrote:
> spupyrev wrote:
> > The default value of "no trimming" sounds strange to me. Do we know if trimming may affect performance? If yes, we shouldn't trim. If no, we should have a reasonable bound here.
> > Do we know if trimming may affect performance?
> It depends on how compiler consumes the context, if the profile context is [a @ b @ c @ d] but compiler only inline d to c and stop inlining at c, then the [a @ b @] can be trimmed. Another things is the profile size tradeoff, before we did merge and trim cold profile after all samples is generated. This patch tried to move the merging ahead as we encountered the memory issues. it doesn't drop the sample.
> 
> >If no, we should have a reasonable bound here.
> Good question, but I don't know what's the reasonable bound yet. As you see the above comments, SPEC benchmark's max inline depth can vary from 5 to 26, the clang-10 binary even have a 51 max depth. and we didn't get memory issue for SPEC, so SPEC can set to -1.
> 
> so it's the tradeoff between many things, what's in my mind is to default it `-1` and we can tune it on-demand. what do you think?
> 
> 
> 
Default -1 (means no trimming) sounds good to me.  An otherwise decent default value would need quite some experiments to justify.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107800