Merge pull request #2020 from randomascii/steady_clock_now_good

Make ScopedMetric portable, accurate, and efficient

2 files changed, 36 insertions, 49 deletions

diff --git a/src/metrics.cc b/src/metrics.cc
index dbaf221..9a4dd12 100644
--- a/src/metrics.cc
+++ b/src/metrics.cc
@@ -18,13 +18,8 @@
 #include <stdio.h>
 #include <string.h>
 
-#ifndef _WIN32
-#include <sys/time.h>
-#else
-#include <windows.h>
-#endif
-
 #include <algorithm>
+#include <chrono>
 
 #include "util.h"
 
@@ -34,49 +29,35 @@ Metrics* g_metrics = NULL;
 
 namespace {
 
-#ifndef _WIN32
 /// Compute a platform-specific high-res timer value that fits into an int64.
 int64_t HighResTimer() {
-  timeval tv;
-  if (gettimeofday(&tv, NULL) < 0)
-    Fatal("gettimeofday: %s", strerror(errno));
-  return (int64_t)tv.tv_sec * 1000*1000 + tv.tv_usec;
+  auto now = chrono::steady_clock::now();
+  return chrono::duration_cast<chrono::steady_clock::duration>(
+             now.time_since_epoch())
+      .count();
 }
 
-/// Convert a delta of HighResTimer() values to microseconds.
-int64_t TimerToMicros(int64_t dt) {
-  // No conversion necessary.
-  return dt;
-}
-#else
-int64_t LargeIntegerToInt64(const LARGE_INTEGER& i) {
-  return ((int64_t)i.HighPart) << 32 | i.LowPart;
-}
-
-int64_t HighResTimer() {
-  LARGE_INTEGER counter;
-  if (!QueryPerformanceCounter(&counter))
-    Fatal("QueryPerformanceCounter: %s", GetLastErrorString().c_str());
-  return LargeIntegerToInt64(counter);
+constexpr int64_t GetFrequency() {
+  // If numerator isn't 1 then we lose precision and that will need to be
+  // assessed.
+  static_assert(std::chrono::steady_clock::period::num == 1,
+                "Numerator must be 1");
+  return std::chrono::steady_clock::period::den /
+         std::chrono::steady_clock::period::num;
 }
 
 int64_t TimerToMicros(int64_t dt) {
-  static int64_t ticks_per_sec = 0;
-  if (!ticks_per_sec) {
-    LARGE_INTEGER freq;
-    if (!QueryPerformanceFrequency(&freq))
-      Fatal("QueryPerformanceFrequency: %s", GetLastErrorString().c_str());
-    ticks_per_sec = LargeIntegerToInt64(freq);
-  }
+  // dt is in ticks.  We want microseconds.
+  return (dt * 1000000) / GetFrequency();
+}
 
+int64_t TimerToMicros(double dt) {
   // dt is in ticks.  We want microseconds.
-  return (dt * 1000000) / ticks_per_sec;
+  return (dt * 1000000) / GetFrequency();
 }
-#endif
 
 }  // anonymous namespace
 
-
 ScopedMetric::ScopedMetric(Metric* metric) {
   metric_ = metric;
   if (!metric_)
@@ -87,7 +68,9 @@ ScopedMetric::~ScopedMetric() {
   if (!metric_)
     return;
   metric_->count++;
-  int64_t dt = TimerToMicros(HighResTimer() - start_);
+  // Leave in the timer's natural frequency to avoid paying the conversion cost
+  // on every measurement.
+  int64_t dt = HighResTimer() - start_;
   metric_->sum += dt;
 }
 
@@ -112,18 +95,23 @@ void Metrics::Report() {
   for (vector<Metric*>::iterator i = metrics_.begin();
        i != metrics_.end(); ++i) {
     Metric* metric = *i;
-    double total = metric->sum / (double)1000;
-    double avg = metric->sum / (double)metric->count;
+    uint64_t micros = TimerToMicros(metric->sum);
+    double total = micros / (double)1000;
+    double avg = micros / (double)metric->count;
     printf("%-*s\t%-6d\t%-8.1f\t%.1f\n", width, metric->name.c_str(),
            metric->count, avg, total);
   }
 }
 
-uint64_t Stopwatch::Now() const {
-  return TimerToMicros(HighResTimer());
+double Stopwatch::Elapsed() const {
+  // Convert to micros after converting to double to minimize error.
+  return 1e-6 * TimerToMicros(static_cast<double>(NowRaw() - started_));
+}
+
+uint64_t Stopwatch::NowRaw() const {
+  return HighResTimer();
 }
 
 int64_t GetTimeMillis() {
   return TimerToMicros(HighResTimer()) / 1000;
 }
-
diff --git a/src/metrics.h b/src/metrics.h
index 11239b5..c9ba236 100644
--- a/src/metrics.h
+++ b/src/metrics.h
@@ -28,11 +28,10 @@ struct Metric {
   std::string name;
   /// Number of times we've hit the code path.
   int count;
-  /// Total time (in micros) we've spent on the code path.
+  /// Total time (in platform-dependent units) we've spent on the code path.
   int64_t sum;
 };
 
-
 /// A scoped object for recording a metric across the body of a function.
 /// Used by the METRIC_RECORD macro.
 struct ScopedMetric {
@@ -68,15 +67,15 @@ struct Stopwatch {
   Stopwatch() : started_(0) {}
 
   /// Seconds since Restart() call.
-  double Elapsed() const {
-    return 1e-6 * static_cast<double>(Now() - started_);
-  }
+  double Elapsed() const;
 
-  void Restart() { started_ = Now(); }
+  void Restart() { started_ = NowRaw(); }
 
  private:
   uint64_t started_;
-  uint64_t Now() const;
+  // Return the current time using the native frequency of the high resolution
+  // timer.
+  uint64_t NowRaw() const;
 };
 
 /// The primary interface to metrics.  Use METRIC_RECORD("foobar") at the top