Storing timers in classes

[markdewing]

Timers are not essential for functionality of the classes, but they are important for the overall understanding of the performance of the code. One design goal is to minimize the syntactic clutter of timers in individual classes.

Here we consider two ways to store timers in a class.
The simplest is to store each individual timer with an individual name.

class A {
  NewTimer &firstTimer;
  NewTimer &secondTimer;

 A() : firstTimer(*TimerManager.createTimer("first timer")),
        secondTimer(*TimerManager.createTimer("second timer")) {}
}

Another alternative is to store timers in a vector

class A {
  TimerList_t myTimers;  // TimerList_t is std::vector<NewTimer *>
}

The previous code used this technique, but accessed the list with a numerical index (myTimers[0]), which is not very clear.

The cleaned up version of this approach adds a enum to name the index to the timer list (myTimers[First_Timer]), along with an extra data structure connecting the enums to the timer names

TimerNameList_t<ExampleTimers> ExampleTimerNames =
{{First_Timer, "first time"},
 {Second_Timer, "second timer"}}

And finally a call to setup_timers is used to initialize the myTimers list.

The diff in #1816 contains examples of both (the Fermion classes have examples of the first, the QMCDriver classes have examples of the second)

Advantages of the first method

• simple and straightforward

Disadvantages of the first method

• initialization of the timers is part of the constructor, possibly making it harder to see other values being initialized
• Timers are dependent on naming to keep them identified as timers
• Need as many names in the class as there are timers

Advantages of the second method

• Only need a single name added to the class for all timers
• The data structures needed for timer initialization can be more separated from the rest of the class

Disadvantages of the second method

• More code to write and read and maintain

If we follow better C++ design principles, the enums and naming structures should get moved to be part of the class (so they are not in the global namespace). In my opinion, this decreases some of the advantages of the second method for keeping timer 'clutter' out of the class. So for simplicity, maybe we should just use the first method?

[ye-luo]

The first method also reduce the direct use of pointers. Method 2 still use pointers. Using reference also conveys a clear message that the owner ship is not handled by the class.

[PDoakORNL]

If we are going to keep so many timers, I like method 2 with the enum and name map in the class scope. Method 2 can probably be done with a std::vector<std::reference_wrapper<NewTimer>> in the class and retain the advantage of references.

However if we could reduce the number of permanent timers I think simplicity of method 1 would win out.

[ye-luo]

@markdewing could you help writing some manual by updating https://github.com/QMCPACK/qmcpack/wiki/How-to-add-timers and remove the wiki once it gets in the manual?

[ye-luo]

Now in favor of vector of reference wrappers instead of pointers.