Use wait_until deadline to reduce number of times we need to sample the clock.

11 files changed, 74 insertions, 68 deletions

diff --git a/config/src/tests/configagent/configagent.cpp b/config/src/tests/configagent/configagent.cpp
index 9684d401a37..c2d68c5d9a0 100644
--- a/config/src/tests/configagent/configagent.cpp
+++ b/config/src/tests/configagent/configagent.cpp
@@ -89,9 +89,8 @@ public:
         return std::move(_update);
     }
 
-    bool wait(vespalib::duration timeout) override
+    bool wait_until(vespalib::steady_time) override
     {
-        (void) timeout;
         return true;
     }
 
diff --git a/config/src/tests/configholder/configholder.cpp b/config/src/tests/configholder/configholder.cpp
index b2f6cd83693..5ff0efc92dc 100644
--- a/config/src/tests/configholder/configholder.cpp
+++ b/config/src/tests/configholder/configholder.cpp
@@ -34,12 +34,12 @@ TEST("Require that waiting is done")
 
     ConfigHolder holder;
     vespalib::Timer timer;
-    holder.wait(1000ms);
+    holder.wait_for(1000ms);
     EXPECT_GREATER_EQUAL(timer.elapsed(), ONE_SEC);
     EXPECT_LESS(timer.elapsed(), ONE_MINUTE);
 
     holder.handle(std::make_unique<ConfigUpdate>(value, true, 0));
-    ASSERT_TRUE(holder.wait(100ms));
+    ASSERT_TRUE(holder.wait_for(100ms));
 }
 
 TEST("Require that polling for elements work")
@@ -58,10 +58,10 @@ TEST("Require that negative time does not mean forever.") {
     ConfigHolder holder;
     vespalib::Timer timer;
     ASSERT_FALSE(holder.poll());
-    ASSERT_FALSE(holder.wait(10ms));
-    ASSERT_FALSE(holder.wait(0ms));
-    ASSERT_FALSE(holder.wait(-1ms));
-    ASSERT_FALSE(holder.wait(-7ms));
+    ASSERT_FALSE(holder.wait_for(10ms));
+    ASSERT_FALSE(holder.wait_for(0ms));
+    ASSERT_FALSE(holder.wait_for(-1ms));
+    ASSERT_FALSE(holder.wait_for(-7ms));
     EXPECT_LESS(timer.elapsed(), ONE_MINUTE);
 }
 
@@ -70,7 +70,7 @@ TEST_MT_F("Require that wait is interrupted", 2, ConfigHolder)
     if (thread_id == 0) {
         vespalib::Timer timer;
         TEST_BARRIER();
-        f.wait(1000ms);
+        f.wait_for(1000ms);
         EXPECT_LESS(timer.elapsed(), ONE_MINUTE);
         EXPECT_GREATER(timer.elapsed(), 400ms);
         TEST_BARRIER();
diff --git a/config/src/tests/configretriever/configretriever.cpp b/config/src/tests/configretriever/configretriever.cpp
index 1515632cf4b..cc36614d4c5 100644
--- a/config/src/tests/configretriever/configretriever.cpp
+++ b/config/src/tests/configretriever/configretriever.cpp
@@ -123,7 +123,7 @@ struct SubscriptionFixture
           sub(std::make_shared<ConfigSubscription>(0, key, holder, std::make_unique<MySource>()))
     {
         holder->handle(std::make_unique<ConfigUpdate>(value, 3, 3));
-        ASSERT_TRUE(sub->nextUpdate(0, 0ms));
+        ASSERT_TRUE(sub->nextUpdate(0, steady_clock::now()));
         sub->flip();
     }
 };
diff --git a/config/src/tests/subscription/subscription.cpp b/config/src/tests/subscription/subscription.cpp
index 13eafbf876e..fa184b90100 100644
--- a/config/src/tests/subscription/subscription.cpp
+++ b/config/src/tests/subscription/subscription.cpp
@@ -10,42 +10,47 @@ using namespace config;
 
 namespace {
 
-    struct SourceFixture
-    {
-        int numClose;
-        int numGetConfig;
-        int numReload;
-        SourceFixture()
-            : numClose(0),
-              numGetConfig(0),
-              numReload(0)
-        { }
-    };
-
-    struct MySource : public Source
-    {
-        MySource(SourceFixture * src)
-            : source(src)
-        {}
+struct SourceFixture
+{
+    int numClose;
+    int numGetConfig;
+    int numReload;
+    SourceFixture()
+        : numClose(0),
+          numGetConfig(0),
+          numReload(0)
+    { }
+};
+
+struct MySource : public Source
+{
+    MySource(SourceFixture * src)
+        : source(src)
+    {}
 
-        void getConfig() override { source->numGetConfig++; }
-        void close() override { source->numClose++; }
-        void reload(int64_t gen) override { (void) gen; source->numReload++; }
+    void getConfig() override { source->numGetConfig++; }
+    void close() override { source->numClose++; }
+    void reload(int64_t gen) override { (void) gen; source->numReload++; }
 
-        SourceFixture * source;
-    };
+    SourceFixture * source;
+};
 
-    struct SubscriptionFixture
+struct SubscriptionFixture
+{
+    std::shared_ptr<IConfigHolder> holder;
+    ConfigSubscription sub;
+    SourceFixture src;
+    SubscriptionFixture(const ConfigKey & key)
+        : holder(new ConfigHolder()),
+          sub(0, key, holder, std::make_unique<MySource>(&src))
     {
-        std::shared_ptr<IConfigHolder> holder;
-        ConfigSubscription sub;
-        SourceFixture src;
-        SubscriptionFixture(const ConfigKey & key)
-            : holder(new ConfigHolder()),
-              sub(0, key, holder, std::make_unique<MySource>(&src))
-        {
-        }
-    };
+    }
+};
+
+vespalib::steady_time
+deadline(vespalib::duration timeout) {
+    return vespalib::steady_clock::now() + timeout;
+}
 }
 
 TEST_FF("requireThatKeyIsReturned", ConfigKey("foo", "bar", "bim", "boo"), SubscriptionFixture(f1))
@@ -56,17 +61,17 @@ TEST_FF("requireThatKeyIsReturned", ConfigKey("foo", "bar", "bim", "boo"), Subsc
 TEST_F("requireThatUpdateReturns", SubscriptionFixture(ConfigKey::create<MyConfig>("myid")))
 {
     f1.holder->handle(std::make_unique<ConfigUpdate>(ConfigValue(), 1, 1));
-    ASSERT_TRUE(f1.sub.nextUpdate(0, 0ms));
+    ASSERT_TRUE(f1.sub.nextUpdate(0, deadline(0ms)));
     ASSERT_TRUE(f1.sub.hasChanged());
     ASSERT_EQUAL(1, f1.sub.getGeneration());
 }
 
 TEST_F("requireThatNextUpdateBlocks", SubscriptionFixture(ConfigKey::create<MyConfig>("myid")))
 {
-    ASSERT_FALSE(f1.sub.nextUpdate(0, 0ms));
+    ASSERT_FALSE(f1.sub.nextUpdate(0, deadline(0ms)));
     f1.holder->handle(std::make_unique<ConfigUpdate>(ConfigValue(), 1, 1));
     vespalib::Timer timer;
-    ASSERT_FALSE(f1.sub.nextUpdate(1, 500ms));
+    ASSERT_FALSE(f1.sub.nextUpdate(1, deadline(500ms)));
     ASSERT_TRUE(timer.elapsed() > 400ms);
 }
 
@@ -75,7 +80,7 @@ TEST_MT_F("requireThatNextUpdateReturnsWhenNotified", 2, SubscriptionFixture(Con
     if (thread_id == 0) {
         vespalib::Timer timer;
         f1.holder->handle(std::make_unique<ConfigUpdate>(ConfigValue(), 1, 1));
-        ASSERT_TRUE(f1.sub.nextUpdate(2, 5000ms));
+        ASSERT_TRUE(f1.sub.nextUpdate(2, deadline(5000ms)));
         ASSERT_TRUE(timer.elapsed() > 200ms);
     } else {
         std::this_thread::sleep_for(500ms);
@@ -89,7 +94,7 @@ TEST_MT_F("requireThatNextUpdateReturnsInterrupted", 2, SubscriptionFixture(Conf
     if (thread_id == 0) {
         vespalib::Timer timer;
         f1.holder->handle(std::make_unique<ConfigUpdate>(ConfigValue(), 1, 1));
-        ASSERT_TRUE(f1.sub.nextUpdate(1, 5000ms));
+        ASSERT_TRUE(f1.sub.nextUpdate(1, deadline(5000ms)));
         ASSERT_TRUE(timer.elapsed() > 300ms);
     } else {
         std::this_thread::sleep_for(500ms);
@@ -100,15 +105,15 @@ TEST_MT_F("requireThatNextUpdateReturnsInterrupted", 2, SubscriptionFixture(Conf
 TEST_F("Require that isChanged takes generation into account", SubscriptionFixture(ConfigKey::create<MyConfig>("myid")))
 {
     f1.holder->handle(std::make_unique<ConfigUpdate>(ConfigValue(StringVector(), "a"), true, 1));
-    ASSERT_TRUE(f1.sub.nextUpdate(0, 0ms));
+    ASSERT_TRUE(f1.sub.nextUpdate(0, deadline(0ms)));
     f1.sub.flip();
     ASSERT_EQUAL(1, f1.sub.getLastGenerationChanged());
     f1.holder->handle(std::make_unique<ConfigUpdate>(ConfigValue(StringVector(), "b"), true, 2));
-    ASSERT_TRUE(f1.sub.nextUpdate(1, 0ms));
+    ASSERT_TRUE(f1.sub.nextUpdate(1, deadline(0ms)));
     f1.sub.flip();
     ASSERT_EQUAL(2, f1.sub.getLastGenerationChanged());
     f1.holder->handle(std::make_unique<ConfigUpdate>(ConfigValue(), false, 3));
-    ASSERT_TRUE(f1.sub.nextUpdate(2, 0ms));
+    ASSERT_TRUE(f1.sub.nextUpdate(2, deadline(0ms)));
     f1.sub.flip();
     ASSERT_EQUAL(2, f1.sub.getLastGenerationChanged());
 }
diff --git a/config/src/vespa/config/common/configholder.cpp b/config/src/vespa/config/common/configholder.cpp
index 95fa3fecba8..d61e06ba0e6 100644
--- a/config/src/vespa/config/common/configholder.cpp
+++ b/config/src/vespa/config/common/configholder.cpp
@@ -32,10 +32,10 @@ ConfigHolder::handle(std::unique_ptr<ConfigUpdate> update)
 }
 
 bool
-ConfigHolder::wait(vespalib::duration timeout)
+ConfigHolder::wait_until(vespalib::steady_time deadline)
 {
     std::unique_lock guard(_lock);
-    return static_cast<bool>(_current) || (_cond.wait_for(guard, timeout) == std::cv_status::no_timeout);
+    return static_cast<bool>(_current) || (_cond.wait_until(guard, deadline) == std::cv_status::no_timeout);
 }
 
 bool
diff --git a/config/src/vespa/config/common/configholder.h b/config/src/vespa/config/common/configholder.h
index 1cbbb383f98..c84e0718931 100644
--- a/config/src/vespa/config/common/configholder.h
+++ b/config/src/vespa/config/common/configholder.h
@@ -18,7 +18,7 @@ public:
 
     std::unique_ptr<ConfigUpdate> provide() override;
     void handle(std::unique_ptr<ConfigUpdate> update) override;
-    bool wait(vespalib::duration timeoutI) override;
+    bool wait_until(vespalib::steady_time deadline) override;
     bool poll() override;
     void interrupt() override;
 public:
diff --git a/config/src/vespa/config/common/configmanager.cpp b/config/src/vespa/config/common/configmanager.cpp
index 1417aed79ae..d95fb12b06a 100644
--- a/config/src/vespa/config/common/configmanager.cpp
+++ b/config/src/vespa/config/common/configmanager.cpp
@@ -33,7 +33,7 @@ ConfigManager::subscribe(const ConfigKey & key, vespalib::duration timeout)
     source->reload(_generation);
 
     source->getConfig();
-    ConfigSubscription::SP subscription(new ConfigSubscription(id, key, holder, std::move(source)));
+    auto subscription = std::make_shared<ConfigSubscription>(id, key, holder, std::move(source));
 
     vespalib::steady_time endTime = vespalib::steady_clock::now() + timeout;
     while (vespalib::steady_clock::now() < endTime) {
diff --git a/config/src/vespa/config/common/waitable.h b/config/src/vespa/config/common/waitable.h
index e15d1a0ace7..7c51dd6bbf6 100644
--- a/config/src/vespa/config/common/waitable.h
+++ b/config/src/vespa/config/common/waitable.h
@@ -11,7 +11,10 @@ namespace config {
  */
 struct Waitable
 {
-    virtual bool wait(vespalib::duration timeout) = 0;
+    bool wait_for(vespalib::duration timeout) {
+        return wait_until(vespalib::steady_clock::now() + timeout);
+    }
+    virtual bool wait_until(vespalib::steady_time deadline) = 0;
     virtual ~Waitable() = default;
 };
 
diff --git a/config/src/vespa/config/subscription/configsubscription.cpp b/config/src/vespa/config/subscription/configsubscription.cpp
index 859e5b6f056..e0528f0246e 100644
--- a/config/src/vespa/config/subscription/configsubscription.cpp
+++ b/config/src/vespa/config/subscription/configsubscription.cpp
@@ -29,7 +29,7 @@ ConfigSubscription::~ConfigSubscription()
 
 
 bool
-ConfigSubscription::nextUpdate(int64_t generation, vespalib::duration timeout)
+ConfigSubscription::nextUpdate(int64_t generation, vespalib::steady_time deadline)
 {
     if (_closed || !_holder->poll()) {
         return false;
@@ -42,7 +42,7 @@ ConfigSubscription::nextUpdate(int64_t generation, vespalib::duration timeout)
     if (isGenerationNewer(_next->getGeneration(), generation)) {
         return true;
     }
-    return (!_closed && _holder->wait(timeout));
+    return (!_closed && _holder->wait_until(deadline));
 }
 
 bool
diff --git a/config/src/vespa/config/subscription/configsubscription.h b/config/src/vespa/config/subscription/configsubscription.h
index 063af97dcec..5938a085ec2 100644
--- a/config/src/vespa/config/subscription/configsubscription.h
+++ b/config/src/vespa/config/subscription/configsubscription.h
@@ -49,7 +49,7 @@ public:
     /// Used by ConfigSubscriptionSet
     SubscriptionId getSubscriptionId() const { return _id; }
     const ConfigKey & getKey() const;
-    bool nextUpdate(int64_t generation, vespalib::duration timeout);
+    bool nextUpdate(int64_t generation, vespalib::steady_time deadline);
     int64_t getGeneration() const;
     bool hasChanged() const;
     bool hasGenerationChanged() const;
diff --git a/config/src/vespa/config/subscription/configsubscriptionset.cpp b/config/src/vespa/config/subscription/configsubscriptionset.cpp
index cbf2eb2333a..e106f21e4b2 100644
--- a/config/src/vespa/config/subscription/configsubscriptionset.cpp
+++ b/config/src/vespa/config/subscription/configsubscriptionset.cpp
@@ -13,6 +13,7 @@ LOG_SETUP(".config.subscription.configsubscriptionset");
 
 using vespalib::duration;
 using vespalib::steady_clock;
+using vespalib::steady_time;
 
 namespace config {
 
@@ -39,13 +40,13 @@ ConfigSubscriptionSet::acquireSnapshot(duration timeout, bool ignoreChange)
         _state = FROZEN;
     }
 
-    steady_clock::time_point startTime = steady_clock::now();
-    duration timeLeft = timeout;
+    steady_time now = steady_clock::now();
+    const steady_time deadline = now + timeout;
     int64_t lastGeneration = _currentGeneration;
     bool inSync = false;
 
-    LOG(spam, "Going into nextConfig loop, time left is %f", vespalib::to_s(timeLeft));
-    while (!isClosed() && (timeLeft >= duration::zero()) && !inSync) {
+    LOG(spam, "Going into nextConfig loop, time left is %f", vespalib::to_s(deadline - now));
+    while (!isClosed() && (now <= deadline)) {
         size_t numChanged = 0;
         size_t numGenerationChanged = 0;
         bool generationsInSync = true;
@@ -54,7 +55,7 @@ ConfigSubscriptionSet::acquireSnapshot(duration timeout, bool ignoreChange)
         // Run nextUpdate on all subscribers to get them in sync.
         for (const auto & subscription : _subscriptionList) {
 
-            if (!subscription->nextUpdate(_currentGeneration, timeLeft) && !subscription->hasGenerationChanged()) {
+            if (!subscription->nextUpdate(_currentGeneration, deadline) && !subscription->hasGenerationChanged()) {
                 subscription->reset();
                 continue;
             }
@@ -76,14 +77,12 @@ ConfigSubscriptionSet::acquireSnapshot(duration timeout, bool ignoreChange)
             if (subscription->getGeneration() != generation) {
                 generationsInSync = false;
             }
-            // Adjust timeout
-            timeLeft = timeout - (steady_clock::now() - startTime);
         }
         inSync = generationsInSync && (_subscriptionList.size() == numGenerationChanged) && (ignoreChange || numChanged > 0);
         lastGeneration = generation;
-        timeLeft = timeout - (steady_clock::now() - startTime);
-        if (!inSync && (timeLeft > duration::zero())) {
-            std::this_thread::sleep_for(std::min(_maxNapTime, timeLeft));
+        now = steady_clock::now();
+        if (!inSync && (now < deadline)) {
+            std::this_thread::sleep_until(std::min(now + _maxNapTime, deadline));
         } else {
             break;
         }