[PATCH] D78027: [TimeProfiler] Emit real process ID and thread names

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb marked 2 inline comments as done.
broadwaylamb added inline comments.


================
Comment at: llvm/lib/Support/TimeProfiler.cpp:77
 
+static std::string getThreadName() {
+  SmallString<64> Name;
----------------
MaskRay wrote:
> You can define ThreadName as `SmallString<0>` to avoid this function.
Then the `ThreadName` field will lose `const`-ness, and I will also need to call `llvm::get_thread_name` in the constructor body, not the member initialization list. Would that be OK?


================
Comment at: llvm/lib/Support/TimeProfiler.cpp:249
+
+    for (const auto &TTP : ThreadTimeTraceProfilerInstances) {
+      writeMetadataEvent("thread_name", TTP->Tid, TTP->ThreadName);
----------------
MaskRay wrote:
> Delete `{}` for simple statements. In LLVM code, such `{}` is not common.
The code above uses `{}`.

According to http://llvm.org/docs/CodingStandards.html:

>If you are extending, enhancing, or bug fixing already implemented code, use the style that is already being used so that the source is uniform and easy to follow.


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

https://reviews.llvm.org/D78027