git commit: Timer now uses Clock instead of Stopwatch to enable easier testing.

[vi...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master e738310ea -> bd4dad4dc


Timer now uses Clock instead of Stopwatch to enable easier testing.

Review: https://reviews.apache.org/r/20918


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bd4dad4d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bd4dad4d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bd4dad4d

Branch: refs/heads/master
Commit: bd4dad4dc20d28791bca5fc970b609f6307a1bad
Parents: e738310
Author: Dominic Hamon <dh...@twopensource.com>
Authored: Tue May 27 14:51:42 2014 -0700
Committer: Vinod Kone <vi...@twitter.com>
Committed: Tue May 27 14:51:43 2014 -0700

----------------------------------------------------------------------
 .../include/process/metrics/timer.hpp           | 27 +++++++++-----------
 3rdparty/libprocess/src/tests/metrics_tests.cpp | 26 +++++++++++++------
 2 files changed, 30 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/bd4dad4d/3rdparty/libprocess/include/process/metrics/timer.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/metrics/timer.hpp b/3rdparty/libprocess/include/process/metrics/timer.hpp
index 9236954..01cb290 100644
--- a/3rdparty/libprocess/include/process/metrics/timer.hpp
+++ b/3rdparty/libprocess/include/process/metrics/timer.hpp
@@ -3,6 +3,7 @@
 
 #include <string>
 
+#include <process/clock.hpp>
 #include <process/future.hpp>
 #include <process/internal.hpp>
 
@@ -11,7 +12,6 @@
 #include <stout/duration.hpp>
 #include <stout/hashmap.hpp>
 #include <stout/option.hpp>
-#include <stout/stopwatch.hpp>
 #include <stout/try.hpp>
 
 namespace process {
@@ -48,26 +48,26 @@ public:
     return value;
   }
 
-  // Start the stopwatch.
+  // Start the Timer.
   void start()
   {
     process::internal::acquire(&data->lock);
     {
-      data->stopwatch.start();
+      data->start = Clock::now();
     }
     process::internal::release(&data->lock);
   }
 
-  // Stop the stopwatch.
+  // Stop the Timer.
   void stop()
   {
+    const Time stop = Clock::now();
+
     double value;
 
     process::internal::acquire(&data->lock);
     {
-      data->stopwatch.stop();
-
-      data->lastValue = T(data->stopwatch.elapsed()).value();
+      data->lastValue = T(stop - data->start).value();
 
       value = data->lastValue.get();
     }
@@ -80,13 +80,10 @@ public:
   template<typename U>
   Future<U> time(const Future<U>& future)
   {
-    Stopwatch stopwatch;
-    stopwatch.start();
-
     // We need to take a copy of 'this' here to ensure that the
     // Timer is not destroyed in the interim.
     future
-      .onAny(lambda::bind(_time, stopwatch, *this));
+      .onAny(lambda::bind(_time, Clock::now(), *this));
 
     return future;
   }
@@ -96,19 +93,19 @@ private:
     Data() : lock(0) {}
 
     int lock;
-    Stopwatch stopwatch;
+    Time start;
     Option<double> lastValue;
   };
 
-  static void _time(Stopwatch stopwatch, Timer that)
+  static void _time(Time start, Timer that)
   {
-    stopwatch.stop();
+    const Time stop = Clock::now();
 
     double value;
 
     process::internal::acquire(&that.data->lock);
     {
-      that.data->lastValue = T(stopwatch.elapsed()).value();
+      that.data->lastValue = T(stop - start).value();
       value = that.data->lastValue.get();
     }
     process::internal::release(&that.data->lock);

http://git-wip-us.apache.org/repos/asf/mesos/blob/bd4dad4d/3rdparty/libprocess/src/tests/metrics_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/metrics_tests.cpp b/3rdparty/libprocess/src/tests/metrics_tests.cpp
index 8a13121..2707d44 100644
--- a/3rdparty/libprocess/src/tests/metrics_tests.cpp
+++ b/3rdparty/libprocess/src/tests/metrics_tests.cpp
@@ -342,13 +342,14 @@ TEST(Metrics, Timer)
   timer.stop();
 
   // Time a no-op.
+  Clock::pause();
   timer.start();
-  os::sleep(Microseconds(1));
+  Clock::advance(Microseconds(1));
   timer.stop();
 
   Future<double> value = timer.value();
   AWAIT_READY(value);
-  EXPECT_GE(value.get(), Microseconds(1).ms());
+  EXPECT_FLOAT_EQ(value.get(), Microseconds(1).ns());
 
   // It is not an error to stop a timer that has already been stopped.
   timer.stop();
@@ -357,8 +358,13 @@ TEST(Metrics, Timer)
 }
 
 
-// TODO(bmahler): Use 'Clock' in Timer so that we can test that Timer
-// is correctly timing futures.
+static Future<int> advanceAndReturn()
+{
+  Clock::advance(Seconds(1));
+  return 42;
+}
+
+
 TEST(Metrics, AsyncTimer)
 {
   metrics::Timer<Microseconds> t("test/timer");
@@ -366,15 +372,19 @@ TEST(Metrics, AsyncTimer)
 
   AWAIT_READY(metrics::add(t));
 
-  Future<int> result = 42;
-
-  result = t.time(result);
+  // Time a Future that returns immediately. Even though the method advances the
+  // clock and we advance the clock here, the Future should be timed as if it
+  // takes 0 time. Ie, we're not timing the method call but the Future.
+  Clock::pause();
+  Future<int> result = t.time(advanceAndReturn());
+  Clock::advance(Seconds(1));
   AWAIT_READY(result);
 
   EXPECT_EQ(42, result.get());
 
+  // The future should have taken zero time.
   AWAIT_READY(t.value());
-  EXPECT_GT(t.value().get(), 0.0);
+  EXPECT_FLOAT_EQ(t.value().get(), 0.0);
 
   AWAIT_READY(metrics::remove(t));
 }