[PATCH] D139603: [llvm-profdata] Add option to cap profile output size

[William Junda Huang via Phabricator via llvm-commits]

huangjd added a comment.

In D139603#4038245 <https://reviews.llvm.org/D139603#4038245>, @dyung wrote:

> @huangjd the test you added seems to be failing on Windows. Can you take a look and revert if you need time to investigate?
>
> https://lab.llvm.org/buildbot/#/builders/216/builds/15534

How to do size check in a cross platform way?



================
Comment at: llvm/include/llvm/ProfileData/SampleProfWriter.h:45
+  SampleProfileMap &ProfileMap;
+  size_t OutputSizeLimit;
+
----------------
snehasish wrote:
> Perhaps specify that this is in bytes either in the variable name or a comment?
Clarified comment in constructor


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:981
+/// this class can be extended to satisfy such need.
+class FunctionPruningStrategy {
+  std::vector<NameFunctionSamples> Functions;
----------------
snehasish wrote:
> snehasish wrote:
> > How about moving this to SampleProfileWriter so that we can use this API in other tooling where we don't invoke llvm-profdata?
> Re-opening since I think we could move the RewriteProfileSizeLimit and CalculateNumFunctionsToRemove method to SampleProfileWriter as well so that this heuristic (and subsequent updates to it) can be reused in internal tooling directly. 
CalculateNumFunctionToRemove is moved inside FunctionPruningStrategy since it won't be used anywhere else.  It can be overriden if necessary 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139603