diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-03-02 10:56:55 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2022-03-02 10:56:55 +0000 |
commit | e325460fadf6f2a5f98c528ba4c5ee2758efbdac (patch) | |
tree | 727f0420d03767ee8fc92a7f767e51355c141dc7 | |
parent | 3b94a726a38ecf74a83084a70d378f31076130de (diff) |
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; } |