Subzero: Fix timers for multithreaded translation.

src/IceGlobalContext.h

Index: src/IceGlobalContext.h
diff --git a/src/IceGlobalContext.h b/src/IceGlobalContext.h
index ac321e68860704d746e07f75b322c9e4adfc11df..8b45cf7345d9ff47758b2492486b0387e84553c0 100644
--- a/src/IceGlobalContext.h
+++ b/src/IceGlobalContext.h
@@ -83,6 +83,35 @@ class GlobalContext {
     uint32_t Fills;
   };
 
+  // TimerList is a vector of TimerStack objects, with extra methods
+  // to initialize and merge these vectors.
+  class TimerList : public std::vector<TimerStack> {
+  public:
+    // initInto() initializes a target list of timers based on the
+    // current list.  In particular, it creates the same number of
+    // timers, in the same order, with the same names, but initially
+    // empty of timing data.
+    void initInto(TimerList &Dest) const {
+      if (!ALLOW_DUMP)
+        return;
+      Dest.clear();
+      for (const TimerStack &Stack : *this) {
+        Dest.push_back(TimerStack(Stack.getName()));
+      }
+    }
+    void mergeFrom(TimerList &Src) {
+      if (!ALLOW_DUMP)
+        return;
+      assert(size() == Src.size());
+      size_type i = 0;
+      for (TimerStack &Stack : *this) {
+        assert(Stack.getName() == Src[i].getName());
+        Stack.mergeFrom(Src[i]);
+        ++i;
+      }
+    }
+  };
+
   // ThreadContext contains thread-local data.  This data can be
   // combined/reduced as needed after all threads complete.
   class ThreadContext {
@@ -92,7 +121,7 @@ class GlobalContext {
   public:
     ThreadContext() {}
     CodeStats StatsFunction;
-    std::vector<TimerStack> Timers;
+    TimerList Timers;
   };
 
 public:
@@ -211,14 +240,20 @@ public:
   // These are predefined TimerStackIdT values.
   enum TimerStackKind { TSK_Default = 0, TSK_Funcs, TSK_Num };
 
+  // newTimerStackID() creates a new TimerStack in the global space.
+  // It does not affect any TimerStack objects in TLS.
   TimerStackIdT newTimerStackID(const IceString &Name);
+  // dumpTimers() dumps the global timer data.  As such, one probably
+  // wants to call mergeTimerStacks() as a prerequisite.
+  void dumpTimers(TimerStackIdT StackID = TSK_Default,
+                  bool DumpCumulative = true);
+  // The following methods affect only the calling thread's TLS timer
+  // data.
   TimerIdT getTimerID(TimerStackIdT StackID, const IceString &Name);
   void pushTimer(TimerIdT ID, TimerStackIdT StackID = TSK_Default);
   void popTimer(TimerIdT ID, TimerStackIdT StackID = TSK_Default);
   void resetTimer(TimerStackIdT StackID);
   void setTimerName(TimerStackIdT StackID, const IceString &NewName);
-  void dumpTimers(TimerStackIdT StackID = TSK_Default,
-                  bool DumpCumulative = true);
 
   // Adds a newly parsed and constructed function to the Cfg work
   // queue.  Notifies any idle workers that a new function is
@@ -235,8 +270,10 @@ public:
 
   void startWorkerThreads() {
     size_t NumWorkers = getFlags().NumTranslationThreads;
+    auto Timers = getTimers();
     for (size_t i = 0; i < NumWorkers; ++i) {
       ThreadContext *WorkerTLS = new ThreadContext();
+      Timers->initInto(WorkerTLS->Timers);
       AllThreadContexts.push_back(WorkerTLS);
       TranslationThreads.push_back(std::thread(
           &GlobalContext::translateFunctionsWrapper, this, WorkerTLS));
@@ -254,6 +291,11 @@ public:
     }
     TranslationThreads.clear();
     // TODO(stichnot): join the emitter thread.
+    if (ALLOW_DUMP) {
+      auto Timers = getTimers();
+      for (ThreadContext *TLS : AllThreadContexts)
+        Timers->mergeFrom(TLS->Timers);
+    }
   }
 
   // Translation thread startup routine.
@@ -301,7 +343,7 @@ private:
   ICE_CACHELINE_BOUNDARY;
   // Managed by getTimers()
   GlobalLockType TimerLock;
-  std::vector<TimerStack> Timers;
+  TimerList Timers;
 
   ICE_CACHELINE_BOUNDARY;
   // StrLock is a global lock on the dump and emit output streams.
@@ -331,8 +373,8 @@ private:
   LockedPtr<CodeStats> getStatsCumulative() {
     return LockedPtr<CodeStats>(&StatsCumulative, &StatsLock);
   }
-  LockedPtr<std::vector<TimerStack>> getTimers() {
-    return LockedPtr<std::vector<TimerStack>>(&Timers, &TimerLock);
+  LockedPtr<TimerList> getTimers() {
+    return LockedPtr<TimerList>(&Timers, &TimerLock);
   }
 
   std::vector<ThreadContext *> AllThreadContexts;
@@ -357,24 +399,35 @@ class TimerMarker {
   TimerMarker &operator=(const TimerMarker &) = delete;
 
 public:
-  TimerMarker(TimerIdT ID, GlobalContext *Ctx)
-      : ID(ID), Ctx(Ctx), Active(false) {
+  TimerMarker(TimerIdT ID, GlobalContext *Ctx,
+              TimerStackIdT StackID = GlobalContext::TSK_Default)
+      : ID(ID), Ctx(Ctx), StackID(StackID), Active(false) {

[jvoung (off chromium), 2015/01/30 18:42:56]

I wonder if this ctor should be defined out of line now. Then it could be next
to the Cfg version.

[Jim Stichnoth, 2015/01/30 20:22:25]

I originally wanted both ctors (and the dtor) to be inline, so that with
ALLOW_DUMP #defined to 1 in the minimal build, the code would collapse to
nothing.  That unfortunately requires including IceCfg.h which is kind of icky,
so the Cfg version is defined out of line.

I think I rearranged things to get the best of both worlds.  The ALLOW_DUMP
portions of both ctors are inline, and the rest is done out of line.

[jvoung (off chromium), 2015/01/30 21:04:44]

I see okay.

     if (ALLOW_DUMP) {
-      Active = Ctx->getFlags().SubzeroTimingEnabled;
+      switch (StackID) {
+      case GlobalContext::TSK_Default:
+        Active = Ctx->getFlags().SubzeroTimingEnabled;
+        break;
+      case GlobalContext::TSK_Funcs:
+        Active = Ctx->getFlags().TimeEachFunction;
+      default:
+        break;
+      }
       if (Active)
-        Ctx->pushTimer(ID);
+        Ctx->pushTimer(ID, StackID);
     }
   }
-  TimerMarker(TimerIdT ID, const Cfg *Func);
+  TimerMarker(TimerIdT ID, const Cfg *Func,
+              TimerStackIdT StackID = GlobalContext::TSK_Default);
 
   ~TimerMarker() {
     if (ALLOW_DUMP && Active)
-      Ctx->popTimer(ID);
+      Ctx->popTimer(ID, StackID);
   }
 
 private:
-  TimerIdT ID;
+  const TimerIdT ID;
   GlobalContext *const Ctx;
+  const TimerStackIdT StackID;
   bool Active;
 };