Make LLVM timer reprintable: that is, make more than one print action on the same timer feasible

[george.karpenkov]

Summary

Currently, each LLVM timer can be only printed once, as the act of printing clear the timer.
I don't think this is always a good idea, as printing twice might be a requirement (e.g. dumping statistics in a serialized form as well as printing it to stdout, or dumping statistics in multiple places).
In the analyzer we are actually hitting those limitations (e.g. it would be nice to have statistics in the produced HTML report as well as on the command line).

Moreover, the current printing mechanism implicitly assumes that the timer is stopped -- and prints zero otherwise.
This patch relaxes this assumption (as I don't think printing a running timer should be a giant problem or should crash), but if it's an issue I could change it to make the assumption explicit by asserting !isRunning.

[MatzeB]

I agree that a function named printXXX() should not also clear the timer.
As far as I understand it this shouldn't change any behavior in the existing clang/llvm code, so LGTM.

[graydon]

Unfortunately this change is causing trouble over in swift where we _did_ rely on the fact that printing timers cleared them. In particular: we wanted to control printing the timers as json to our own chosen stats stream on shutdown (not the llvm one) and once printed, we did not expect them to print restart _again_ to stderr when llvm shuts down.

Is there some way we can either revert this change, or add a method that explicitly clears all timers?

[george.karpenkov]

@graydon could you just explicitly .clear() them?

[graydon]

In D43136#1199781, @george.karpenkov wrote:

@graydon could you just explicitly .clear() them?

There is no public API with which to do so (or indeed, to access them at all). I would be happy to otherwise. Shall I add such an API?

[george.karpenkov]

@graydon Ah right, .clear() is public on a Timer, but not exposed through TimerGroup.
I'm fine with adding such an API.
I'm not sure whether anyone else would object ( @MatzeB @ab @thegameg ?)

[thegameg]

In D43136#1199886, @george.karpenkov wrote:

I'm not sure whether anyone else would object ( @MatzeB @ab @thegameg ?)

Sounds good to me.

llvm/trunk/lib/Support/Timer.cpp

void TimerGroup::PrintQueuedTimers(raw_ostream &OS) {
   TimersToPrint.clear();
 }
 
 void TimerGroup::prepareToPrintList() {
   // See if any of our timers were started, if so add them to TimersToPrint and
   // reset them.
   for (Timer *T = FirstTimer; T; T = T->Next) {
     if (!T->hasTriggered()) continue;
+    bool WasRunning = T->isRunning();
+    if (WasRunning)
+      T->stopTimer();
+
     TimersToPrint.emplace_back(T->Time, T->Name, T->Description);
 
-    // Clear out the time.
-    T->clear();
+    if (WasRunning)
+      T->startTimer();
   }
 }
 
 void TimerGroup::print(raw_ostream &OS) {
   sys::SmartScopedLock<true> L(*TimerLock);
 
   prepareToPrintList();
 
for (const PrintRecord &R : TimersToPrint) {
     delim = ",\n";
 
     const TimeRecord &T = R.Time;
     printJSONValue(OS, R, ".wall", T.getWallTime());
     OS << delim;
     printJSONValue(OS, R, ".user", T.getUserTime());
     OS << delim;
     printJSONValue(OS, R, ".sys", T.getSystemTime());
+    if (T.getMemUsed()) {
+      OS << delim;
+      printJSONValue(OS, R, ".sys", T.getMemUsed());
+    }
   }
   TimersToPrint.clear();
   return delim;
 }
 
 const char *TimerGroup::printAllJSONValues(raw_ostream &OS, const char *delim) {
   sys::SmartScopedLock<true> L(*TimerLock);
   for (TimerGroup *TG = TimerGroupList; TG; TG = TG->Next)
     delim = TG->printJSONValues(OS, delim);
   return delim;
 }
 
 void TimerGroup::ConstructTimerLists() {
   (void)*NamedGroupedTimers;
 }