[PATCH] D69043: [RFC] Adding time-trace to LLD?

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: lld/ELF/Driver.cpp:515
+
+  if (llvm::timeTraceProfilerEnabled()) {
+    SmallString<128> path(config->outputFile);
----------------
Could you possibly move this block (and the init block above) into a new file in `lld/Common/` so we can use it in the COFF driver as well?


================
Comment at: lld/ELF/Driver.cpp:527
+  }
+  return;
 }
----------------
Extraneous return.


================
Comment at: lld/ELF/Options.td:361
 
+def time_trace: F<"time-trace">, HelpText<"Record time trace">;
+
----------------
Given this option is a candidate for the other LLD drivers, I am wondering if we couldn't have a shared `lld/Common/CommonOpt.td`. @ruiu WDYT?


================
Comment at: llvm/lib/Support/TimeProfiler.cpp:70
   void begin(std::string Name, llvm::function_ref<std::string()> Detail) {
+    std::lock_guard<std::mutex> Lock(Mu);
     Stack.emplace_back(steady_clock::now(), TimePointType(), std::move(Name),
----------------
The lock is definitely not the way to go, it would skew all the timings especially on tens-of-cores machines. Like you're suggesting, make `TimeTraceProfilerInstance` a TLS, along with a new combiner upon the call to `timeTraceProfilerWrite()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69043