Store timer duration in native duration type & Add proper test for timer

Storing duration in doubles is sub-optimal as each cast may incur in truncation.
Accumulations may also truncate the value further.
Using the native value preserves the duration unit user request its value in seconds.

Use duration_cast instead of division:
Interestingly, dividing by 1.0s yields a different (slightly longer) assembly code than when making an explicit cast to seconds.
The difference seems to me is the cast manages to generate SEE vector registers/instructions.

dune/common/test/CMakeLists.txt

@@ -348,6 +348,9 @@ dune_add_test(SOURCES typeutilitytest.cc
 dune_add_test(SOURCES typelisttest.cc
               LABELS quick)
 
+dune_add_test(SOURCES timertest.cc
+              LABELS quick)
+
 dune_add_test(SOURCES utilitytest.cc
               LABELS quick)
 

dune/common/test/timertest.cc

+// -*- tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+// vi: set et ts=4 sw=2 sts=2:
+// SPDX-FileCopyrightInfo: Copyright © DUNE Project contributors, see file LICENSE.md in module root
+// SPDX-License-Identifier: LicenseRef-GPL-2.0-only-with-DUNE-exception
+
+#include <chrono>
+#include <thread>
+
+#include <dune/common/timer.hh>
+
+#include <dune/common/test/testsuite.hh>
+
+int
+main()
+{
+  Dune::TestSuite suite;
+
+  using clock = std::chrono::high_resolution_clock;
+  using namespace std::chrono_literals;
+  Dune::Timer top_timer(false);
+  auto top_chrono_start = clock::now();
+  std::this_thread::sleep_for(10ms);
+  top_timer.start();
+
+  Dune::Timer loop_timer;
+  for (std::size_t i = 0; i != 5; ++i) {
+    { // let chrono timer run for longer time
+      auto chrono_start = clock::now();
+      loop_timer.start();
+      std::this_thread::sleep_for(10ms);
+      loop_timer.stop();
+      double dune_elapsed = loop_timer.lastElapsed();
+      std::this_thread::sleep_for(10ms);
+      auto chrono_elapsed = clock::now() - chrono_start;
+
+      std::this_thread::sleep_for(10ms);
+      suite.check(dune_elapsed == loop_timer.lastElapsed())
+        << "Elapsed time is not the same as when timer is stop";
+      suite.check(dune_elapsed < (chrono_elapsed / 1.0s))
+        << "Dune timer " << dune_elapsed << "s takes more time than chrono timer "
+        << (chrono_elapsed / 1.0s) << "s";
+    }
+
+    { // let chrono timer run for longer time
+      loop_timer.start();
+      auto chrono_start = clock::now();
+      std::this_thread::sleep_for(10ms);
+      auto chrono_elapsed = clock::now() - chrono_start;
+      std::this_thread::sleep_for(10ms);
+      loop_timer.stop();
+      double dune_elapsed = loop_timer.lastElapsed();
+
+      std::this_thread::sleep_for(10ms);
+      suite.check(dune_elapsed == loop_timer.lastElapsed())
+        << "Elapsed time is not the same as when timer is stop";
+      suite.check(dune_elapsed > (chrono_elapsed / 1.0s))
+        << "Dune timer " << dune_elapsed << "s takes less time than chrono timer "
+        << (chrono_elapsed / 1.0s) << "s";
+    }
+  }
+
+  double top_dune_elapsed = top_timer.stop();
+  std::this_thread::sleep_for(10ms);
+  auto top_chrono_elapsed = clock::now() - top_chrono_start;
+
+  suite.check(top_dune_elapsed < (top_chrono_elapsed / 1.0s))
+    << "Top Dune timer " << top_dune_elapsed << "s takes less time than Top chrono timer "
+    << (top_chrono_elapsed / 1.0s) << "s";
+
+  suite.check(top_dune_elapsed > loop_timer.elapsed())
+    << "Top Dune timer " << top_dune_elapsed << "s takes less time than Loop Dune timer "
+    << loop_timer.elapsed() << "s";
+}

dune/common/timer.hh

@@ -29,6 +29,8 @@ namespace Dune {
    */
   class Timer
   {
+    using Clock = std::chrono::high_resolution_clock;
+    using Units = std::chrono::duration<double>; // seconds stored as double
   public:
 
     /** \brief A new timer, create and reset
@@ -44,9 +46,9 @@ namespace Dune {
     //! Reset timer while keeping the running/stopped state
     void reset() noexcept
     {
-      sumElapsed_ = 0.0;
-      storedLastElapsed_ = 0.0;
-      rawReset();
+      sumElapsed_ = std::chrono::seconds{0};
+      storedLastElapsed_ = std::chrono::seconds{0};
+      cstart = Clock::now();
     }
 
 
@@ -55,7 +57,7 @@ namespace Dune {
     {
       if (not (isRunning_))
       {
-        rawReset();
+        cstart = Clock::now();
         isRunning_ = true;
       }
     }
@@ -64,33 +66,22 @@ namespace Dune {
     //! Get elapsed user-time from last reset until now/last stop in seconds.
     double elapsed () const noexcept
     {
-      // if timer is running add the time elapsed since last start to sum
-      if (isRunning_)
-        return sumElapsed_ + lastElapsed();
-
-      return sumElapsed_;
+      return durationCast(rawElapsed());
     }
 
-
     //! Get elapsed user-time from last start until now/last stop in seconds.
     double lastElapsed () const noexcept
     {
-      // if timer is running return the current value
-      if (isRunning_)
-        return rawElapsed();
-
-      // if timer is not running return stored value from last run
-      return storedLastElapsed_;
+      return durationCast(rawLastElapsed());
     }
 
-
     //! Stop the timer and return elapsed().
     double stop() noexcept
     {
       if (isRunning_)
       {
         // update storedLastElapsed_ and  sumElapsed_ and stop timer
-        storedLastElapsed_ = lastElapsed();
+        storedLastElapsed_ = rawLastElapsed();
         sumElapsed_ += storedLastElapsed_;
         isRunning_ = false;
       }
@@ -101,23 +92,34 @@ namespace Dune {
   private:
 
     bool isRunning_;
-    double sumElapsed_;
-    double storedLastElapsed_;
-
+    Clock::duration sumElapsed_;
+    Clock::duration storedLastElapsed_;
 
-    void rawReset() noexcept
+    Clock::duration rawElapsed () const noexcept
     {
-      cstart = std::chrono::high_resolution_clock::now();
+      // if timer is running add the time elapsed since last start to sum
+      if (isRunning_)
+        return sumElapsed_ + rawLastElapsed();
+
+      return sumElapsed_;
     }
 
-    double rawElapsed () const noexcept
+    //! Get elapsed user-time from last start until now/last stop in seconds.
+    Clock::duration rawLastElapsed () const noexcept
     {
-      std::chrono::high_resolution_clock::time_point now = std::chrono::high_resolution_clock::now();
-      std::chrono::duration<double> time_span = std::chrono::duration_cast<std::chrono::duration<double> >(now - cstart);
-      return time_span.count();
+      // if timer is running return the current value
+      if (isRunning_)
+        return Clock::now() - cstart;
+
+      // if timer is not running return stored value from last run
+      return storedLastElapsed_;
+    }
+
+    double durationCast(Clock::duration duration) const noexcept {
+      return std::chrono::duration_cast<Units>(duration).count();
     }
 
-    std::chrono::high_resolution_clock::time_point cstart;
+    Clock::time_point cstart;
   }; // end class Timer
 
   /** @} end documentation */