[PATCH] D78901: [Support] Get process statistics in ExecuteAndWait and Wait

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 5 inline comments as done.
sepavloff added inline comments.


================
Comment at: llvm/include/llvm/Support/Program.h:60
+    std::chrono::microseconds UserTime;
+    unsigned long long PeakMemory = 0;
+
----------------
aganea wrote:
> I would go for a `uint64_t` because it's shorter to read (and to parse for the preprocessor), but that's my personal preference.
No problem. Replaced.


================
Comment at: llvm/include/llvm/Support/Program.h:69
+    /// Checks if this objects contains information.
+    bool isSet() const { return PeakMemory != 0; }
+  };
----------------
aganea wrote:
> This screams for `Optional`, see D78903.
Indeed, thank you.


================
Comment at: llvm/lib/Support/Unix/Program.inc:356
 
   // Parent process: Wait for the child process to terminate.
   int status;
----------------
aganea wrote:
> `ProcStat` is not cleared here, in case of an early exit.
Added call of `reset`.


================
Comment at: llvm/lib/Support/Windows/Program.inc:407
+  if (ProcStat)
+    ProcStat->clear();
   DWORD WaitStatus = WaitForSingleObject(PI.Process, milliSecondsToWait);
----------------
aganea wrote:
> You can simply say: `*ProcStat = {};`
Used `Optional<>::reset()` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78901