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

Alexandre Ganea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 27 11:17:49 PDT 2020


aganea added inline comments.


================
Comment at: llvm/include/llvm/Support/Program.h:60
+    std::chrono::microseconds UserTime;
+    unsigned long long PeakMemory = 0;
+
----------------
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.


================
Comment at: llvm/include/llvm/Support/Program.h:63
+    /// Put the object into the state, in which it does not contain information.
+    void clear() {
+      TotalTime = UserTime = std::chrono::microseconds::zero();
----------------
I don't think you need `clear()`, as stated below.


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


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


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78901





More information about the llvm-commits mailing list