Go back to TimerOutput::Scope?

[bangerth]

I have come across #6664 ("Replace TimerOutput::Scope to avoid deadlocks") which replaces the uses of TimerOutput::Scope by explicit calls to timer.enter_subsection() and timer.leave_subsection(). But I am mystified by what that actually solved. At least the current state of TimerOutput::Scope looks as follows:

inline TimerOutput::Scope::Scope(dealii::TimerOutput &timer_,
                                 const std::string   &section_name_)
  : timer(timer_)
  , section_name(section_name_)
  , in(true)
{
  timer.enter_subsection(section_name);
}



inline void
TimerOutput::Scope::stop()
{
  if (!in)
    return;
  in = false;

  timer.leave_subsection(section_name);
}


TimerOutput::Scope::~Scope()
{
  try
    {
      stop();
    }
  catch (...)
    {}
}

This is not functionally different from manually calling enter/leave_subsection() with the exception of the try-catch clause. On the other hand, the old scheme did not require us to track where we exit a function -- the destructor did the leaving.

Did TimerOutput::Scope change? Or is the presence of the try-catch the problem? How about a replacement class that just doesn't have the try-catch that does what we want in the destructor?

[gassmoeller]

I think your suggestion will reintroduce the MPI deadlocks that #6664 fixed, at least for deal.II versions 9.6 and 9.7 (any version before dealii/dealii#12248).

The problem here is that it is dangerous to do MPI communication in a destructor, because the destructor may be called during stack unwinding on only some of the MPI processes, creating a deadlock. Before my deal.II patch linked above timer.leave_subsection() initiates MPI communication to synchronize the results of the timer.

If we instead use enter/leave_subsection as implemented in #6664 we will never get to the leave_subsection part on ranks that throw exceptions. While the other MPI ranks (that didnt throw the exception) could run into MPI communication, the rank that threw the exception will propagate up the stack, and we have code in place that catches and aborts the program without initiating MPI communication on these ranks (https://github.com/geodynamics/aspect/blob/main/source/main.cc#L978).

In principle now that dealii/dealii#12248 is merged we can reintroduce the use of TimerScope for deal.II 9.8 and later. I would prefer we use the deal.II class then instead of creating our own in ASPECT. Getting all this to work cleanly was pretty tricky, and I would prefer avoiding to maintain our own copy of the class.