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

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProfWriter.cpp:111
+    StringBuffer.clear();
+    OutputStream.reset(new raw_svector_ostream(StringBuffer));
+    if (std::error_code EC = write(ProfileMap))
----------------
huangjd wrote:
> snehasish wrote:
> > Is this leaking the raw_svector_ostream objects?
> > 
> > Can we rewrite this as the following?
> > 
> > ```
> > raw_svector_ostream OS(StringBuffer);
> > OutputStream.reset(&OS);
> > ```
> > 
> > Also might be worth it to use a separate OutputStream object within the loop scope so that we don't have to worry about the swap before and after and the lifetimes of the raw_vector_ostream objects.
> unique_ptr.reset(new obj) is the correct usage. 
> https://en.cppreference.com/w/cpp/memory/unique_ptr/reset
Thanks, knowing the type makes it clear! Perhaps that is additional motivation for a separate, local OutputStream var?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141446