[PATCH] Move global state in timevar.c to a new "timer" class

[David Malcolm <dmalcolm at redhat dot com>]

I want to make the timevar infrastructure be more flexible,
to better serve some JIT use-cases.

The following is a "removal of global state" patch which gathers
together the state within timevar.c into a new "timer" class.

Within the classic cc1/cc1plus/etc use-case, this timer class
is a singleton.  For the JIT, I have a followup patch allowing
timers to be managed externally by client code, so that client code
can e.g. have multiple compiles in-process, and accumulate the
timings into different timers as appropriate (so e.g. a JavaScript
engine on one thread could accumulate its JIT-compilation timings
into one timer, whereas a Python JIT on another thread could
accumulate into another timer, and various other arrangements).

There is a global singleton for gcc's internals:

  timer *g_timer;

(which is guarded by the jit mutex for the jit case).

Currently the global bool "timevar_enable" is used for early bailout
from the timevar code, for the common needs-to-be-fast case where
timevar-tracking is disabled.  The patch eliminates this flag
throughout, replacing it with a test for g_timer being non-NULL
(it is only non-NULL if timing has been enabled).

The existing API implicitly uses "g_timer", so e.g.

  timevar_push (TV_SOMETHING);

which was:

  if (timevar_enable)
    timevar_push_1 (TV_SOMETHING);

effectively becomes:

  if (g_timer)
    g_timer->push (TV_SOMETHING);

A detail relating to timevar.c's get_time: "g_timer" is a singleton
as far as most of gcc is concerned, but I have a follow patch in
development in which there could be multiple timer instances in the
jit.  Hence the patch eliminated a check for timevar_enable in
"get_time" , *without* replacing it with a check for
g_timer != NULL, since with this patch get_time is only called from
methods of timer.  Doing so enables us to call methods on other timer
instances (e.g. to measure time spent waiting for the jit mutex).
There's also a test eliminated from timer::print, for similar reasons.

With timing disabled, this patch replaces the early reject
using a global bool with an early reject using a global ptr, and so
ought to be as fast:

test-benchmark.c, with timing disabled (the default case):
 Before introducing "class timer":
  100 iterations at optlevel 0 took a total of 6.150s (0.061s per iteration)
  100 iterations at optlevel 1 took a total of 6.780s (0.068s per iteration)
  100 iterations at optlevel 2 took a total of 6.990s (0.070s per iteration)
  100 iterations at optlevel 3 took a total of 8.590s (0.086s per iteration)
 After introducing "class timer:
  100 iterations at optlevel 0 took a total of 6.120s (0.061s per iteration)
  100 iterations at optlevel 1 took a total of 6.840s (0.068s per iteration)
  100 iterations at optlevel 2 took a total of 7.020s (0.070s per iteration)
  100 iterations at optlevel 3 took a total of 8.570s (0.086s per iteration)

With timing enabled, there's an extra indirection inside the
code in timevar.c, but this appears to have no significant cost,
for the jit.df/test-benchmark.c at least (though this test has just one
function):

test-benchmark.c, with timing enabled:
 Before introducing "class timer":
  100 iterations at optlevel 0 took a total of 6.110s (0.061s per iteration)
  100 iterations at optlevel 1 took a total of 6.970s (0.070s per iteration)
  100 iterations at optlevel 2 took a total of 7.080s (0.071s per iteration)
  100 iterations at optlevel 3 took a total of 8.820s (0.088s per iteration)
 After introducing "class timer":
  100 iterations at optlevel 0 took a total of 6.170s (0.062s per iteration)
  100 iterations at optlevel 1 took a total of 6.980s (0.070s per iteration)
  100 iterations at optlevel 2 took a total of 7.040s (0.070s per iteration)
  100 iterations at optlevel 3 took a total of 8.730s (0.087s per iteration)

The patch moves the structs "timevar_def" and "timevar_stack_def"
from timevar.c into timevar.h, hiding them as private within
class timer.  I had another version of this patch which kept them
within timevar.c, but to do this I had to use the "pimpl" idiom,
adding complexity and another layer of indirection.  It seemed best
to simply put them as private types within class timer.

Successfully bootstrapped&regrtested on x86_64-unknown-linux-gnu
(Fedora 20).

OK for trunk?

gcc/ChangeLog:
	* timevar.c (timevar_enable): Delete in favor of...
	(g_timer): New global.
	(struct timevar_def): Move to timevar.h inside class timer.
	(struct timevar_stack_def): Likewise.
	(timevars): Delete global in favor of field "m_timevars" within
	class timer in timevar.h
	(stack): Likewise, in favor of field "m_stack".
	(unused_stack_instances): Likewise, in favor of field
	"m_unused_stack_instances".
	(start_time): Likewise, in favor of field "m_start_time".
	(get_time): Eliminate check for timevar_enable.
	(timer::timer): New function, built from part of timevar_init.
	(timevar_init): Rewrite idempotency test from using
	"timevar_enable" bool to using dynamic allocation of "g_timer".
	Move rest of implementation into timer's constructor.
	(timevar_push_1): Rename to...
	(timer::push): ...this, adding "m_" prefixes to variables that
	are now fields of timer.
	(timevar_pop_1): Likewise, rename to...
	(timer::pop): ...this, and add "m_" prefixes.
	(timevar_start): Replace test for "timevar_enable" with one for
	"g_timer", and move bulk of implementation to...
	(timer::start): ...here, adding "m_" prefixes.
	(timevar_stop): Likewise, from here...
	(timer::stop): ...to here.
	(timevar_cond_start): Likewise, from here...
	(timer::cond_start): ...to here.
	(timevar_cond_stop): Likewise, from here...
	(timer::cond_stop): ...to here.
	(validate_phases): Rename to...
	(timer::validate_phases): ...this, and add "m_" prefixes.  Make
	locals "total" and "tv" const.
	(timevar_print): Rename to...
	(timer::print): ...this, and add "m_" prefixes.  Make locals
	"total" and "tv" const.  Eliminate test for timevar_enable.
	* timevar.h (timevar_enable): Eliminate.
	(g_timer): New declaration.
	(timevar_push_1): Eliminate.
	(timevar_pop_1): Eliminate.
	(timevar_print): Eliminate.
	(class timer): New class.
	(timevar_push): Rewrite to use g_timer.
	(timevar_pop): Likewise.
	* toplev.c (toplev::~toplev): Likewise.
---
 gcc/timevar.c | 207 +++++++++++++++++++++++++++-------------------------------
 gcc/timevar.h | 103 ++++++++++++++++++++++++++---
 gcc/toplev.c  |   7 +-
 3 files changed, 194 insertions(+), 123 deletions(-)

diff --git a/gcc/timevar.c b/gcc/timevar.c
index 123402d..76ad22a 100644
--- a/gcc/timevar.c
+++ b/gcc/timevar.c
@@ -99,10 +99,10 @@ static double clocks_to_msec;
 #define CLOCKS_TO_MSEC (1 / (double)CLOCKS_PER_SEC)
 #endif
 
-/* True if timevars should be used.  In GCC, this happens with
+/* Non-NULL if timevars should be used.  In GCC, this happens with
    the -ftime-report flag.  */
 
-bool timevar_enable;
+timer *g_timer;
 
 /* Total amount of memory allocated by garbage collector.  */
 
@@ -115,57 +115,6 @@ size_t timevar_ggc_mem_total;
 
 /* See timevar.h for an explanation of timing variables.  */
 
-/* A timing variable.  */
-
-struct timevar_def
-{
-  /* Elapsed time for this variable.  */
-  struct timevar_time_def elapsed;
-
-  /* If this variable is timed independently of the timing stack,
-     using timevar_start, this contains the start time.  */
-  struct timevar_time_def start_time;
-
-  /* The name of this timing variable.  */
-  const char *name;
-
-  /* Nonzero if this timing variable is running as a standalone
-     timer.  */
-  unsigned standalone : 1;
-
-  /* Nonzero if this timing variable was ever started or pushed onto
-     the timing stack.  */
-  unsigned used : 1;
-};
-
-/* An element on the timing stack.  Elapsed time is attributed to the
-   topmost timing variable on the stack.  */
-
-struct timevar_stack_def
-{
-  /* The timing variable at this stack level.  */
-  struct timevar_def *timevar;
-
-  /* The next lower timing variable context in the stack.  */
-  struct timevar_stack_def *next;
-};
-
-/* Declared timing variables.  Constructed from the contents of
-   timevar.def.  */
-static struct timevar_def timevars[TIMEVAR_LAST];
-
-/* The top of the timing stack.  */
-static struct timevar_stack_def *stack;
-
-/* A list of unused (i.e. allocated and subsequently popped)
-   timevar_stack_def instances.  */
-static struct timevar_stack_def *unused_stack_instances;
-
-/* The time at which the topmost element on the timing stack was
-   pushed.  Time elapsed since then is attributed to the topmost
-   element.  */
-static struct timevar_time_def start_time;
-
 static void get_time (struct timevar_time_def *);
 static void timevar_accumulate (struct timevar_time_def *,
 				struct timevar_time_def *,
@@ -183,9 +132,6 @@ get_time (struct timevar_time_def *now)
   now->wall = 0;
   now->ggc_mem = timevar_ggc_mem_total;
 
-  if (!timevar_enable)
-    return;
-
   {
 #ifdef USE_TIMES
     struct tms tms;
@@ -218,25 +164,24 @@ timevar_accumulate (struct timevar_time_def *timer,
   timer->ggc_mem += stop_time->ggc_mem - start_time->ggc_mem;
 }
 
-/* Initialize timing variables.  */
+/* Class timer's constructor.  */
 
-void
-timevar_init (void)
+timer::timer () :
+  m_stack (NULL),
+  m_unused_stack_instances (NULL),
+  m_start_time ()
 {
-  if (timevar_enable)
-    return;
-
-  timevar_enable = true;
-
   /* Zero all elapsed times.  */
-  memset (timevars, 0, sizeof (timevars));
+  memset (m_timevars, 0, sizeof (m_timevars));
 
   /* Initialize the names of timing variables.  */
 #define DEFTIMEVAR(identifier__, name__) \
-  timevars[identifier__].name = name__;
+  m_timevars[identifier__].name = name__;
 #include "timevar.def"
 #undef DEFTIMEVAR
 
+  /* Initialize configuration-specific state.
+     Ideally this would be one-time initialization.  */
 #ifdef USE_TIMES
   ticks_to_msec = TICKS_TO_MSEC;
 #endif
@@ -245,6 +190,17 @@ timevar_init (void)
 #endif
 }
 
+/* Initialize timing variables.  */
+
+void
+timevar_init (void)
+{
+  if (g_timer)
+    return;
+
+  g_timer = new timer ();
+}
+
 /* Push TIMEVAR onto the timing stack.  No further elapsed time is
    attributed to the previous topmost timing variable on the stack;
    subsequent elapsed time is attributed to TIMEVAR, until it is
@@ -253,9 +209,9 @@ timevar_init (void)
    TIMEVAR cannot be running as a standalone timer.  */
 
 void
-timevar_push_1 (timevar_id_t timevar)
+timer::push (timevar_id_t timevar)
 {
-  struct timevar_def *tv = &timevars[timevar];
+  struct timevar_def *tv = &m_timevars[timevar];
   struct timevar_stack_def *context;
   struct timevar_time_def now;
 
@@ -270,27 +226,27 @@ timevar_push_1 (timevar_id_t timevar)
 
   /* If the stack isn't empty, attribute the current elapsed time to
      the old topmost element.  */
-  if (stack)
-    timevar_accumulate (&stack->timevar->elapsed, &start_time, &now);
+  if (m_stack)
+    timevar_accumulate (&m_stack->timevar->elapsed, &m_start_time, &now);
 
   /* Reset the start time; from now on, time is attributed to
      TIMEVAR.  */
-  start_time = now;
+  m_start_time = now;
 
   /* See if we have a previously-allocated stack instance.  If so,
      take it off the list.  If not, malloc a new one.  */
-  if (unused_stack_instances != NULL)
+  if (m_unused_stack_instances != NULL)
     {
-      context = unused_stack_instances;
-      unused_stack_instances = unused_stack_instances->next;
+      context = m_unused_stack_instances;
+      m_unused_stack_instances = m_unused_stack_instances->next;
     }
   else
     context = XNEW (struct timevar_stack_def);
 
   /* Fill it in and put it on the stack.  */
   context->timevar = tv;
-  context->next = stack;
-  stack = context;
+  context->next = m_stack;
+  m_stack = context;
 }
 
 /* Pop the topmost timing variable element off the timing stack.  The
@@ -300,30 +256,30 @@ timevar_push_1 (timevar_id_t timevar)
    timing variable.  */
 
 void
-timevar_pop_1 (timevar_id_t timevar)
+timer::pop (timevar_id_t timevar)
 {
   struct timevar_time_def now;
-  struct timevar_stack_def *popped = stack;
+  struct timevar_stack_def *popped = m_stack;
 
-  gcc_assert (&timevars[timevar] == stack->timevar);
+  gcc_assert (&m_timevars[timevar] == m_stack->timevar);
 
   /* What time is it?  */
   get_time (&now);
 
   /* Attribute the elapsed time to the element we're popping.  */
-  timevar_accumulate (&popped->timevar->elapsed, &start_time, &now);
+  timevar_accumulate (&popped->timevar->elapsed, &m_start_time, &now);
 
   /* Reset the start time; from now on, time is attributed to the
      element just exposed on the stack.  */
-  start_time = now;
+  m_start_time = now;
 
   /* Take the item off the stack.  */
-  stack = stack->next;
+  m_stack = m_stack->next;
 
   /* Don't delete the stack element; instead, add it to the list of
      unused elements for later use.  */
-  popped->next = unused_stack_instances;
-  unused_stack_instances = popped;
+  popped->next = m_unused_stack_instances;
+  m_unused_stack_instances = popped;
 }
 
 /* Start timing TIMEVAR independently of the timing stack.  Elapsed
@@ -333,11 +289,19 @@ timevar_pop_1 (timevar_id_t timevar)
 void
 timevar_start (timevar_id_t timevar)
 {
-  struct timevar_def *tv = &timevars[timevar];
-
-  if (!timevar_enable)
+  if (!g_timer)
     return;
 
+  g_timer->start (timevar);
+}
+
+/* See timevar_start above.  */
+
+void
+timer::start (timevar_id_t timevar)
+{
+  struct timevar_def *tv = &m_timevars[timevar];
+
   /* Mark this timing variable as used.  */
   tv->used = 1;
 
@@ -355,12 +319,20 @@ timevar_start (timevar_id_t timevar)
 void
 timevar_stop (timevar_id_t timevar)
 {
-  struct timevar_def *tv = &timevars[timevar];
-  struct timevar_time_def now;
-
-  if (!timevar_enable)
+  if (!g_timer)
     return;
 
+  g_timer->stop (timevar);
+}
+
+/* See timevar_stop above.  */
+
+void
+timer::stop (timevar_id_t timevar)
+{
+  struct timevar_def *tv = &m_timevars[timevar];
+  struct timevar_time_def now;
+
   /* TIMEVAR must have been started via timevar_start.  */
   gcc_assert (tv->standalone);
   tv->standalone = 0; /* Enable a restart.  */
@@ -379,11 +351,19 @@ timevar_stop (timevar_id_t timevar)
 bool
 timevar_cond_start (timevar_id_t timevar)
 {
-  struct timevar_def *tv = &timevars[timevar];
-
-  if (!timevar_enable)
+  if (!g_timer)
     return false;
 
+  return g_timer->cond_start (timevar);
+}
+
+/* See timevar_cond_start above.  */
+
+bool
+timer::cond_start (timevar_id_t timevar)
+{
+  struct timevar_def *tv = &m_timevars[timevar];
+
   /* Mark this timing variable as used.  */
   tv->used = 1;
 
@@ -406,13 +386,21 @@ timevar_cond_start (timevar_id_t timevar)
 void
 timevar_cond_stop (timevar_id_t timevar, bool running)
 {
+  if (!g_timer || running)
+    return;
+
+  g_timer->cond_stop (timevar);
+}
+
+/* See timevar_cond_stop above.  */
+
+void
+timer::cond_stop (timevar_id_t timevar)
+{
   struct timevar_def *tv;
   struct timevar_time_def now;
 
-  if (!timevar_enable || running)
-    return;
-
-  tv = &timevars[timevar];
+  tv = &m_timevars[timevar];
 
   /* TIMEVAR must have been started via timevar_cond_start.  */
   gcc_assert (tv->standalone);
@@ -425,11 +413,11 @@ timevar_cond_stop (timevar_id_t timevar, bool running)
 
 /* Validate that phase times are consistent.  */
 
-static void
-validate_phases (FILE *fp)
+void
+timer::validate_phases (FILE *fp) const
 {
   unsigned int /* timevar_id_t */ id;
-  struct timevar_time_def *total = &timevars[TV_TOTAL].elapsed;
+  const timevar_time_def *total = &m_timevars[TV_TOTAL].elapsed;
   double phase_user = 0.0;
   double phase_sys = 0.0;
   double phase_wall = 0.0;
@@ -439,7 +427,7 @@ validate_phases (FILE *fp)
 
   for (id = 0; id < (unsigned int) TIMEVAR_LAST; ++id)
     {
-      struct timevar_def *tv = &timevars[(timevar_id_t) id];
+      const timevar_def *tv = &m_timevars[(timevar_id_t) id];
 
       /* Don't evaluate timing variables that were never used.  */
       if (!tv->used)
@@ -480,17 +468,14 @@ validate_phases (FILE *fp)
    for normalizing the others, and is displayed last.  */
 
 void
-timevar_print (FILE *fp)
+timer::print (FILE *fp)
 {
   /* Only print stuff if we have some sort of time information.  */
 #if defined (HAVE_USER_TIME) || defined (HAVE_SYS_TIME) || defined (HAVE_WALL_TIME)
   unsigned int /* timevar_id_t */ id;
-  struct timevar_time_def *total = &timevars[TV_TOTAL].elapsed;
+  const timevar_time_def *total = &m_timevars[TV_TOTAL].elapsed;
   struct timevar_time_def now;
 
-  if (!timevar_enable)
-    return;
-
   /* Update timing information in case we're calling this from GDB.  */
 
   if (fp == 0)
@@ -501,17 +486,17 @@ timevar_print (FILE *fp)
 
   /* If the stack isn't empty, attribute the current elapsed time to
      the old topmost element.  */
-  if (stack)
-    timevar_accumulate (&stack->timevar->elapsed, &start_time, &now);
+  if (m_stack)
+    timevar_accumulate (&m_stack->timevar->elapsed, &m_start_time, &now);
 
   /* Reset the start time; from now on, time is attributed to
      TIMEVAR.  */
-  start_time = now;
+  m_start_time = now;
 
   fputs ("\nExecution times (seconds)\n", fp);
   for (id = 0; id < (unsigned int) TIMEVAR_LAST; ++id)
     {
-      struct timevar_def *tv = &timevars[(timevar_id_t) id];
+      const timevar_def *tv = &m_timevars[(timevar_id_t) id];
       const double tiny = 5e-3;
 
       /* Don't print the total execution time here; that goes at the
diff --git a/gcc/timevar.h b/gcc/timevar.h
index c06025d..71a814a 100644
--- a/gcc/timevar.h
+++ b/gcc/timevar.h
@@ -79,35 +79,118 @@ typedef enum
 timevar_id_t;
 #undef DEFTIMEVAR
 
-/* True if timevars should be used.  In GCC, this happens with
-   the -ftime-report flag.  */
-extern bool timevar_enable;
+/* A class to hold all state relating to timing.  */
+
+class timer;
+
+/* The singleton instance of timing state.
+
+   This is non-NULL if timevars should be used.  In GCC, this happens with
+   the -ftime-report flag.  Hence this is NULL for the common,
+   needs-to-be-fast case, with an early reject happening for this being
+   NULL.  */
+extern timer *g_timer;
 
 /* Total amount of memory allocated by garbage collector.  */
 extern size_t timevar_ggc_mem_total;
 
 extern void timevar_init (void);
-extern void timevar_push_1 (timevar_id_t);
-extern void timevar_pop_1 (timevar_id_t);
 extern void timevar_start (timevar_id_t);
 extern void timevar_stop (timevar_id_t);
 extern bool timevar_cond_start (timevar_id_t);
 extern void timevar_cond_stop (timevar_id_t, bool);
-extern void timevar_print (FILE *);
+
+/* The public (within GCC) interface for timing.  */
+
+class timer
+{
+ public:
+  timer ();
+  ~timer ();
+
+  void start (timevar_id_t tv);
+  void stop (timevar_id_t tv);
+  void push (timevar_id_t tv);
+  void pop (timevar_id_t tv);
+  bool cond_start (timevar_id_t tv);
+  void cond_stop (timevar_id_t tv);
+
+  void print (FILE *fp);
+
+ private:
+  /* Private member functions.  */
+  void validate_phases (FILE *fp) const;
+
+ private:
+
+  /* Private type: a timing variable.  */
+  struct timevar_def
+  {
+    /* Elapsed time for this variable.  */
+    struct timevar_time_def elapsed;
+
+    /* If this variable is timed independently of the timing stack,
+       using timevar_start, this contains the start time.  */
+    struct timevar_time_def start_time;
+
+    /* The name of this timing variable.  */
+    const char *name;
+
+    /* Nonzero if this timing variable is running as a standalone
+       timer.  */
+    unsigned standalone : 1;
+
+    /* Nonzero if this timing variable was ever started or pushed onto
+       the timing stack.  */
+    unsigned used : 1;
+  };
+
+  /* Private type: an element on the timing stack
+     Elapsed time is attributed to the topmost timing variable on the
+     stack.  */
+  struct timevar_stack_def
+  {
+    /* The timing variable at this stack level.  */
+    struct timevar_def *timevar;
+
+    /* The next lower timing variable context in the stack.  */
+    struct timevar_stack_def *next;
+  };
+
+ private:
+
+  /* Data members (all private).  */
+
+  /* Declared timing variables.  Constructed from the contents of
+     timevar.def.  */
+  timevar_def m_timevars[TIMEVAR_LAST];
+
+  /* The top of the timing stack.  */
+  timevar_stack_def *m_stack;
+
+  /* A list of unused (i.e. allocated and subsequently popped)
+     timevar_stack_def instances.  */
+  timevar_stack_def *m_unused_stack_instances;
+
+  /* The time at which the topmost element on the timing stack was
+     pushed.  Time elapsed since then is attributed to the topmost
+     element.  */
+  timevar_time_def m_start_time;
+};
 
 /* Provided for backward compatibility.  */
 static inline void
 timevar_push (timevar_id_t tv)
 {
-  if (timevar_enable)
-    timevar_push_1 (tv);
+  if (g_timer)
+    g_timer->push (tv);
 }
 
 static inline void
 timevar_pop (timevar_id_t tv)
 {
-  if (timevar_enable)
-    timevar_pop_1 (tv);
+  if (g_timer)
+    g_timer->pop (tv);
 }
 
 // This is a simple timevar wrapper class that pushes a timevar in its
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 38de36b..b61e9bf 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -2095,8 +2095,11 @@ toplev::toplev (bool use_TV_TOTAL, bool init_signals)
 
 toplev::~toplev ()
 {
-  timevar_stop (TV_TOTAL);
-  timevar_print (stderr);
+  if (g_timer)
+    {
+      g_timer->stop (TV_TOTAL);
+      g_timer->print (stderr);
+    }
 }
 
 void
-- 
1.8.5.3