[PATCH] D118677: [llvm-profgen] Clean up unnecessary memory reservations between phases.

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/llvm-profgen.cpp:161
   Generator->generateProfile();
+  Reader.release();
   Generator->write();
----------------
hoy wrote:
> wenlei wrote:
> > nit: using a lexical scope for this purpose might be better. 
> Lexical scope is nice, but in this case `Reader` and `Generator` have overlapping lifetimes, i.e, we need `Reader` to be released before `Generator->write();` but after `Generator->generateProfile();`. So one lexical scope including both objects may not work.
> 
> Actually maybe we can define `Reader->getSampleCounters()` as a `unique_ptr` too, so that when it is transferred to `Generator`, `Generator` will take its ownership and release it when appropriate.
> Lexical scope is nice, but in this case Reader and Generator have overlapping lifetimes, i.e, we need Reader to be released before Generator->write(); but after Generator->generateProfile();. So one lexical scope including both objects may not work. 

I was thinking about hoisting the unique_ptr for Generator above, initialized with null, then set to the real thing after reader is available. This way the lexical scope only applies to reader.

> Actually maybe we can define Reader->getSampleCounters() as a unique_ptr too, so that when it is transferred to Generator, Generator will take its ownership and release it when appropriate. 

I'm not sure if this is a good idea. With this, we can leave Reader in a corrupted state after calling the getter, which is probably not a good API design. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118677