[lld] 3508c1d - [LLD] Make scoped timers thread safe

[Reid Kleckner via llvm-commits]

Author: Reid Kleckner
Date: 2020-05-20T16:16:08-07:00
New Revision: 3508c1d8fbe9b053d32330b6995c88c57a82e33f

URL: https://github.com/llvm/llvm-project/commit/3508c1d8fbe9b053d32330b6995c88c57a82e33f
DIFF: https://github.com/llvm/llvm-project/commit/3508c1d8fbe9b053d32330b6995c88c57a82e33f.diff

LOG: [LLD] Make scoped timers thread safe

Summary:
This is a pre-requisite to parallelizing PDB symbol and type merging.
Currently this timer usage would not be thread safe.

Reviewers: aganea, MaskRay

Subscribers: jfb, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D80298

Added: 
    

Modified: 
    lld/COFF/Driver.cpp
    lld/Common/Timer.cpp
    lld/include/lld/Common/Timer.h

Removed: 
    


################################################################################
diff  --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 03b8ce0a6bdc..7372505bb616 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1108,6 +1108,8 @@ Optional<std::string> getReproduceFile(const opt::InputArgList &args) {
 }
 
 void LinkerDriver::link(ArrayRef<const char *> argsArr) {
+  ScopedTimer rootTimer(Timer::root());
+
   // Needed for LTO.
   InitializeAllTargetInfos();
   InitializeAllTargets();
@@ -1166,7 +1168,6 @@ void LinkerDriver::link(ArrayRef<const char *> argsArr) {
 
   config->showSummary = args.hasArg(OPT_summary);
 
-  ScopedTimer t(Timer::root());
   // Handle --version, which is an lld extension. This option is a bit odd
   // because it doesn't start with "/", but we deliberately chose "--" to
   // avoid conflict with /version and for compatibility with clang-cl.
@@ -2042,7 +2043,7 @@ void LinkerDriver::link(ArrayRef<const char *> argsArr) {
   writeResult();
 
   // Stop early so we can print the results.
-  Timer::root().stop();
+  rootTimer.stop();
   if (config->showTiming)
     Timer::root().print();
 }

diff  --git a/lld/Common/Timer.cpp b/lld/Common/Timer.cpp
index 3848b6d8820e..ea221fd86f3e 100644
--- a/lld/Common/Timer.cpp
+++ b/lld/Common/Timer.cpp
@@ -13,29 +13,22 @@
 using namespace lld;
 using namespace llvm;
 
-ScopedTimer::ScopedTimer(Timer &t) : t(&t) { t.start(); }
+ScopedTimer::ScopedTimer(Timer &t) : t(&t) {
+  startTime = std::chrono::high_resolution_clock::now();
+}
 
 void ScopedTimer::stop() {
   if (!t)
     return;
-  t->stop();
+  t->addToTotal(std::chrono::high_resolution_clock::now() - startTime);
   t = nullptr;
 }
 
 ScopedTimer::~ScopedTimer() { stop(); }
 
-Timer::Timer(llvm::StringRef name) : name(std::string(name)), parent(nullptr) {}
-Timer::Timer(llvm::StringRef name, Timer &parent)
-    : name(std::string(name)), parent(&parent) {}
-
-void Timer::start() {
-  if (parent && total.count() == 0)
-    parent->children.push_back(this);
-  startTime = std::chrono::high_resolution_clock::now();
-}
-
-void Timer::stop() {
-  total += (std::chrono::high_resolution_clock::now() - startTime);
+Timer::Timer(llvm::StringRef name) : name(std::string(name)) {}
+Timer::Timer(llvm::StringRef name, Timer &parent) : name(std::string(name)) {
+  parent.children.push_back(this);
 }
 
 Timer &Timer::root() {
@@ -49,7 +42,8 @@ void Timer::print() {
   // We want to print the grand total under all the intermediate phases, so we
   // print all children first, then print the total under that.
   for (const auto &child : children)
-    child->print(1, totalDuration);
+    if (child->total > 0)
+      child->print(1, totalDuration);
 
   message(std::string(49, '-'));
 
@@ -58,7 +52,7 @@ void Timer::print() {
 
 double Timer::millis() const {
   return std::chrono::duration_cast<std::chrono::duration<double, std::milli>>(
-             total)
+             std::chrono::nanoseconds(total))
       .count();
 }
 
@@ -74,6 +68,7 @@ void Timer::print(int depth, double totalDuration, bool recurse) const {
 
   if (recurse) {
     for (const auto &child : children)
-      child->print(depth + 1, totalDuration);
+      if (child->total > 0)
+        child->print(depth + 1, totalDuration);
   }
 }

diff  --git a/lld/include/lld/Common/Timer.h b/lld/include/lld/Common/Timer.h
index 4a298b04a30b..95e811a2f9f0 100644
--- a/lld/include/lld/Common/Timer.h
+++ b/lld/include/lld/Common/Timer.h
@@ -12,6 +12,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include <assert.h>
+#include <atomic>
 #include <chrono>
 #include <map>
 #include <memory>
@@ -27,6 +28,8 @@ struct ScopedTimer {
 
   void stop();
 
+  std::chrono::time_point<std::chrono::high_resolution_clock> startTime;
+
   Timer *t = nullptr;
 };
 
@@ -36,8 +39,7 @@ class Timer {
 
   static Timer &root();
 
-  void start();
-  void stop();
+  void addToTotal(std::chrono::nanoseconds time) { total += time.count(); }
   void print();
 
   double millis() const;
@@ -46,11 +48,9 @@ class Timer {
   explicit Timer(llvm::StringRef name);
   void print(int depth, double totalDuration, bool recurse = true) const;
 
-  std::chrono::time_point<std::chrono::high_resolution_clock> startTime;
-  std::chrono::nanoseconds total;
+  std::atomic<std::chrono::nanoseconds::rep> total;
   std::vector<Timer *> children;
   std::string name;
-  Timer *parent;
 };
 
 } // namespace lld