integrate new MockTick from havardpe

6 files changed, 91 insertions, 112 deletions

diff --git a/staging_vespalib/src/tests/metrics/mock_tick.cpp b/staging_vespalib/src/tests/metrics/mock_tick.cpp
index 2178b5364e9..68e8d7a7daa 100644
--- a/staging_vespalib/src/tests/metrics/mock_tick.cpp
+++ b/staging_vespalib/src/tests/metrics/mock_tick.cpp
@@ -3,76 +3,4 @@
 
 namespace vespalib::metrics {
 
-TimeStamp
-MockTick::next(TimeStamp prev)
-{
-    std::unique_lock<std::mutex> locker(_lock);
-    _prevValue = prev;
-    _blocked = true;
-    _blockedCond.notify_all();
-    while (_runFlag && !_provided) {
-        _providedCond.wait(locker);
-    }
-    _blocked = false;
-    if (_provided) {
-        _provided = false;
-        return _nextValue;
-    } else {
-        // killed
-        return TimeStamp(0);
-    }
-}
-
-void
-MockTick::kill()
-{
-    std::unique_lock<std::mutex> locker(_lock);
-    _runFlag = false;
-    _blockedCond.notify_all();
-    _providedCond.notify_all();
-}
-
-bool
-MockTick::alive()
-{
-    std::unique_lock<std::mutex> locker(_lock);
-    return _runFlag;
-}
-
-void
-MockTick::provide(TimeStamp value)
-{
-    std::unique_lock<std::mutex> locker(_lock);
-    _nextValue = value;
-    _blocked = false;
-    _provided = true;
-    _providedCond.notify_all();
-}
-
-TimeStamp
-MockTick::waitUntilBlocked()
-{
-    std::unique_lock<std::mutex> locker(_lock);
-    while (_runFlag && !_blocked) {
-        _blockedCond.wait(locker);
-    }
-    if (_blocked) {
-        return _prevValue;
-    } else {
-        // killed
-        return TimeStamp(0);
-    }
-}
-
-MockTick::MockTick()
-    : _lock(),
-      _runFlag(true),
-      _provided(false),
-      _blocked(false),
-      _providedCond(),
-      _blockedCond(),
-      _nextValue(0.0),
-      _prevValue(0.0)
-{}
-
 } // namespace vespalib::metrics
diff --git a/staging_vespalib/src/tests/metrics/mock_tick.h b/staging_vespalib/src/tests/metrics/mock_tick.h
index e0336260880..3f244ea6c9f 100644
--- a/staging_vespalib/src/tests/metrics/mock_tick.h
+++ b/staging_vespalib/src/tests/metrics/mock_tick.h
@@ -5,36 +5,87 @@
 #include <condition_variable>
 #include <mutex>
 #include <vespa/vespalib/metrics/clock.h>
+#include <vespa/vespalib/testkit/test_kit.h>
 
 namespace vespalib::metrics {
 
-// share the MockTick between the tested and the tester.
+// used to test clients of the Tick interface
+// values shared between threads are bounded queues with max size 1
 class MockTick : public Tick {
 private:
-    std::mutex _lock;
-    bool _runFlag;
-    bool _provided;
-    bool _blocked;
-    std::condition_variable _providedCond;
-    std::condition_variable _blockedCond;
-    TimeStamp _nextValue;
-    TimeStamp _prevValue;
-public:
-    MockTick();
-    TimeStamp next(TimeStamp prev) override;
-    void kill() override;
-    bool alive() override;
+    using Guard = std::unique_lock<std::mutex>;
+    struct Value {
+        TimeStamp value{0.0};
+        bool    valid{false};
+    };
+
+    TimeStamp               _first_value;
+    std::mutex              _lock;
+    std::condition_variable _cond;
+    bool                    _alive;
+    Value                   _prev;
+    Value                   _next;
+
+    void push(Value &dst, TimeStamp value) {
+        Guard guard(_lock);
+        while (_alive && dst.valid) {
+            _cond.wait(guard);
+        }
+        dst.value = value;
+        dst.valid = true;
+        _cond.notify_one();
+    }
+
+    TimeStamp pop(Value &src) {
+        Guard guard(_lock);
+        while (_alive && !src.valid) {
+            _cond.wait(guard);
+        }
+        src.valid = false;
+        _cond.notify_one();
+        return src.value;
+    }
 
-    void provide(TimeStamp value);
-    TimeStamp waitUntilBlocked();
+    TimeStamp peek(const Value &src) {
+        Guard guard(_lock);
+        while (_alive && !src.valid) {
+            _cond.wait(guard);
+        }
+        return src.value;
+    }
+
+public:
+    MockTick(TimeStamp first_value)
+        : _first_value(first_value), _lock(), _cond(), _alive(true), _prev(), _next() {}
+    TimeStamp first() override { return _first_value; }
+    TimeStamp next(TimeStamp prev) override {
+        push(_prev, prev);
+        return pop(_next);
+    }
+    TimeStamp give(TimeStamp next_value) {
+        TimeStamp prev_value = pop(_prev);
+        push(_next, next_value);
+        EXPECT_EQUAL(peek(_prev).count(), next_value.count());
+        return prev_value;
+    }
+    bool alive() const override { return _alive; }
+    void kill() override {
+        Guard guard(_lock);
+        _alive = false;
+        _cond.notify_all();
+    }
 };
 
-struct TickProxy : Tick {
-    std::shared_ptr<Tick> tick;
-    TickProxy(std::shared_ptr<Tick> tick_in) : tick(std::move(tick_in)) {}
-    TimeStamp next(TimeStamp prev) override { return tick->next(prev); }
-    void kill() override { tick->kill(); }
-    bool alive() override { return tick->alive(); }
+// share the MockTick between the tested and the tester.
+class TickProxy : public Tick {
+private:
+    std::shared_ptr<Tick> _tick;
+public:
+    TickProxy(std::shared_ptr<Tick> tick) : _tick(std::move(tick)) {}
+    TimeStamp first() override { return _tick->first(); }
+    TimeStamp next(TimeStamp prev) override { return _tick->next(prev); }
+    bool alive() const override { return _tick->alive(); }
+    void kill() override { _tick->kill(); }
 };
 
 } // namespace vespalib::metrics
diff --git a/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp b/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp
index 463a285e7b3..ce518474a7f 100644
--- a/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp
+++ b/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp
@@ -119,10 +119,8 @@ TEST("use simple_metrics_collector")
     using namespace vespalib::metrics;
     SimpleManagerConfig cf;
     cf.sliding_window_seconds = 5;
-    std::shared_ptr<MockTick> ticker = std::make_shared<MockTick>();
-    ticker->provide(TimeStamp(1.0));
+    std::shared_ptr<MockTick> ticker = std::make_shared<MockTick>(TimeStamp(1.0));
     auto manager = SimpleMetricsManager::createForTest(cf, std::make_unique<TickProxy>(ticker));
-    EXPECT_EQUAL(1.0, ticker->waitUntilBlocked().count());
 
     Counter myCounter = manager->counter("foo");
     myCounter.add();
@@ -134,9 +132,7 @@ TEST("use simple_metrics_collector")
     myGauge.sample(43.0);
     myGauge.sample(42.0);
 
-    EXPECT_EQUAL(1.0, ticker->waitUntilBlocked().count());
-    ticker->provide(TimeStamp(2.0));
-    EXPECT_EQUAL(2.0, ticker->waitUntilBlocked().count());
+    EXPECT_EQUAL(1.0, ticker->give(TimeStamp(2.0)).count());
 
     Snapshot snap1 = manager->snapshot();
     EXPECT_EQUAL(1.0, snap1.startTime());
@@ -178,9 +174,7 @@ TEST("use simple_metrics_collector")
     myGauge.sample(14.0, two);
     myGauge.sample(11.0, three);
 
-    EXPECT_EQUAL(2.0, ticker->waitUntilBlocked().count());
-    ticker->provide(TimeStamp(4.5));
-    EXPECT_EQUAL(4.5, ticker->waitUntilBlocked().count());
+    EXPECT_EQUAL(2.0, ticker->give(TimeStamp(4.5)).count());
 
     Snapshot snap2 = manager->snapshot();
     EXPECT_EQUAL(1.0, snap2.startTime());
@@ -193,8 +187,7 @@ TEST("use simple_metrics_collector")
 
     // flush sliding window
     for (int i = 5; i <= 10; ++i) {
-        ticker->provide(TimeStamp(i));
-        ticker->waitUntilBlocked();
+        ticker->give(TimeStamp(i));
     }
     Snapshot snap3 = manager->snapshot();
     EXPECT_EQUAL(5.0, snap3.startTime());
diff --git a/staging_vespalib/src/vespa/vespalib/metrics/clock.h b/staging_vespalib/src/vespa/vespalib/metrics/clock.h
index a5c29cf8579..2af2ad89427 100644
--- a/staging_vespalib/src/vespa/vespalib/metrics/clock.h
+++ b/staging_vespalib/src/vespa/vespalib/metrics/clock.h
@@ -21,9 +21,9 @@ using TimeStamp = std::chrono::duration<double, std::ratio<1,1>>;
 struct Tick {
     using UP = std::unique_ptr<Tick>;
     virtual TimeStamp next(TimeStamp prev) = 0;
-    TimeStamp first() { return next(TimeStamp(0.0)); }
+    virtual TimeStamp first() = 0;
     virtual void kill() = 0;
-    virtual bool alive() = 0;
+    virtual bool alive() const = 0;
     virtual ~Tick() {}
 };
 
diff --git a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp
index 0bcfc1ea272..31a04e67ac4 100644
--- a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp
+++ b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp
@@ -21,6 +21,12 @@ SimpleTick::SimpleTick()
 {}
 
 TimeStamp
+SimpleTick::first()
+{
+    return now();
+}
+
+TimeStamp
 SimpleTick::next(TimeStamp prev)
 {
     std::unique_lock<std::mutex> locker(_lock);
@@ -28,8 +34,9 @@ SimpleTick::next(TimeStamp prev)
         TimeStamp curr = now();
         if (curr - prev >= oneSec) {
             return curr;
-        }
-        if (curr <= prev) {
+        } else if (curr < prev) {
+             // clock was adjusted backwards
+             prev = curr;
             _cond.wait_for(locker, oneSec);
         } else {
             _cond.wait_for(locker, oneSec - (curr - prev));
@@ -47,9 +54,8 @@ SimpleTick::kill()
 }
 
 bool
-SimpleTick::alive()
+SimpleTick::alive() const
 {
-    std::unique_lock<std::mutex> locker(_lock);
     return _runFlag;
 }
 
diff --git a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h
index 959dd6b4d98..74b9d88d255 100644
--- a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h
+++ b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h
@@ -17,9 +17,10 @@ private:
     std::condition_variable _cond;
 public:
     SimpleTick();
+    TimeStamp first() override;
     TimeStamp next(TimeStamp prev) override;
     void kill() override;
-    bool alive() override;
+    bool alive() const override;
 };
 
 } // namespace vespalib::metrics