diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2020-10-14 17:53:06 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2020-10-14 17:57:29 +0000 |
commit | 433d1fccf19f4fd390b54ac7c149c17529a37e6a (patch) | |
tree | e31c4021dfc3571ed21849af02e703d00b564a21 | |
parent | 1ad0641192b439447750c49fee0ca6255d4601fd (diff) |
GC unuse code and use std::mutex/std:condition_variable over vespalib::Monitor
16 files changed, 71 insertions, 477 deletions
diff --git a/messagebus/src/tests/messageordering/messageordering.cpp b/messagebus/src/tests/messageordering/messageordering.cpp index 481b8bbd270..fe523135364 100644 --- a/messagebus/src/tests/messageordering/messageordering.cpp +++ b/messagebus/src/tests/messageordering/messageordering.cpp @@ -29,25 +29,23 @@ getRouting() class MultiReceptor : public IMessageHandler { private: - vespalib::Monitor _mon; + std::mutex _mon; DestinationSession* _destinationSession; int _messageCounter; - MultiReceptor(const Receptor &); - MultiReceptor &operator=(const Receptor &); public: MultiReceptor() : _mon(), - _destinationSession(0), + _destinationSession(nullptr), _messageCounter(0) {} void handleMessage(Message::UP msg) override { - SimpleMessage& simpleMsg(dynamic_cast<SimpleMessage&>(*msg)); + auto & simpleMsg(dynamic_cast<SimpleMessage&>(*msg)); LOG(spam, "Attempting to acquire lock for %s", simpleMsg.getValue().c_str()); - vespalib::MonitorGuard lock(_mon); + std::lock_guard guard(_mon); vespalib::string expected(vespalib::make_string("%d", _messageCounter)); LOG(debug, "Got message %p with %s, expecting %s", @@ -79,20 +77,22 @@ public: class VerifyReplyReceptor : public IReplyHandler { - vespalib::Monitor _mon; + mutable std::mutex _mon; + mutable std::condition_variable _cond; std::string _failure; int _replyCount; public: - ~VerifyReplyReceptor(); + ~VerifyReplyReceptor() override; VerifyReplyReceptor(); void handleReply(Reply::UP reply) override; void waitUntilDone(int waitForCount) const; const std::string& getFailure() const { return _failure; } }; -VerifyReplyReceptor::~VerifyReplyReceptor() {} +VerifyReplyReceptor::~VerifyReplyReceptor() = default; VerifyReplyReceptor::VerifyReplyReceptor() : _mon(), + _cond(), _failure(), _replyCount(0) {} @@ -100,7 +100,7 @@ VerifyReplyReceptor::VerifyReplyReceptor() void VerifyReplyReceptor::handleReply(Reply::UP reply) { - vespalib::MonitorGuard lock(_mon); + std::lock_guard lock(_mon); if (reply->hasErrors()) { std::ostringstream ss; ss << "Reply failed with " @@ -113,7 +113,7 @@ VerifyReplyReceptor::handleReply(Reply::UP reply) LOG(warning, "%s", ss.str().c_str()); } else { vespalib::string expected(vespalib::make_string("%d", _replyCount)); - SimpleReply& simpleReply(static_cast<SimpleReply&>(*reply)); + auto & simpleReply(static_cast<SimpleReply&>(*reply)); if (simpleReply.getValue() != expected) { std::stringstream ss; ss << "Received out-of-sequence reply! Expected " @@ -127,14 +127,14 @@ VerifyReplyReceptor::handleReply(Reply::UP reply) } } ++_replyCount; - lock.broadcast(); + _cond.notify_all(); } void VerifyReplyReceptor::waitUntilDone(int waitForCount) const { - vespalib::MonitorGuard lock(_mon); + std::unique_lock guard(_mon); while (_replyCount < waitForCount) { - lock.wait(1000); + _cond.wait_for(guard, 1s); } } diff --git a/messagebus/src/vespa/messagebus/testlib/slobrok.cpp b/messagebus/src/vespa/messagebus/testlib/slobrok.cpp index 620a4615d08..944cd505957 100644 --- a/messagebus/src/vespa/messagebus/testlib/slobrok.cpp +++ b/messagebus/src/vespa/messagebus/testlib/slobrok.cpp @@ -13,21 +13,22 @@ namespace { class WaitTask : public FNET_Task { private: - bool _done; - vespalib::Monitor _mon; + bool _done; + std::mutex _mon; + std::condition_variable _cond; public: - WaitTask(FNET_Scheduler *s) : FNET_Task(s), _done(false), _mon() {} + explicit WaitTask(FNET_Scheduler *s) : FNET_Task(s), _done(false), _mon() {} void wait() { - vespalib::MonitorGuard guard(_mon); + std::unique_lock guard(_mon); while (!_done) { - guard.wait(); + _cond.wait(guard); } } void PerformTask() override { - vespalib::MonitorGuard guard(_mon); + std::lock_guard guard(_mon); _done = true; - guard.signal(); + _cond.notify_one(); } }; } // namespace <unnamed> @@ -52,11 +53,11 @@ void Slobrok::init() { slobrok::ConfigShim shim(_port); - _env.reset(new slobrok::SBEnv(shim)); + _env = std::make_unique<slobrok::SBEnv>(shim); _thread.setEnv(_env.get()); WaitTask wt(_env->getTransport()->GetScheduler()); wt.ScheduleNow(); - if (_pool.NewThread(&_thread, 0) == 0) { + if (_pool.NewThread(&_thread, nullptr) == nullptr) { LOG_ABORT("Could not spawn thread"); } wt.wait(); diff --git a/searchlib/src/tests/transactionlogstress/translogstress.cpp b/searchlib/src/tests/transactionlogstress/translogstress.cpp index a516fb26604..5792da7aa18 100644 --- a/searchlib/src/tests/transactionlogstress/translogstress.cpp +++ b/searchlib/src/tests/transactionlogstress/translogstress.cpp @@ -18,8 +18,6 @@ LOG_SETUP("translogstress"); using vespalib::nbostream; using search::Runnable; -using vespalib::Monitor; -using vespalib::MonitorGuard; using std::shared_ptr; using vespalib::make_string; using vespalib::ConstBufferRef; @@ -116,7 +114,7 @@ Packet::Entry EntryGenerator::getRandomEntry(SerialNum num) { _rnd.srand48(_baseSeed + num); - if (_buffers != NULL) { + if (_buffers != nullptr) { size_t i = _rnd.lrand48() % _buffers->size(); const nbostream& buffer = (*_buffers)[i]; return Packet::Entry(num, 1024, ConstBufferRef(buffer.data(), buffer.size())); @@ -209,7 +207,7 @@ private: public: FeederThread(const std::string & tlsSpec, const std::string & domain, const EntryGenerator & generator, uint32_t feedRate, size_t packetSize); - ~FeederThread(); + ~FeederThread() override; void doRun() override; SerialNumRange getRange() const { return SerialNumRange(1, _lastCommited); } }; @@ -247,7 +245,7 @@ void FeederThread::doRun() { _session = _client.open(_domain); - if (_session.get() == NULL) { + if ( ! _session) { throw std::runtime_error(vespalib::make_string("FeederThread: Could not open session to %s", _tlsSpec.c_str())); } @@ -328,10 +326,10 @@ private: SerialNum _to; SerialNum _next; State _state; - Monitor _monitor; + std::mutex _monitor; void setState(State newState) { - MonitorGuard guard(_monitor); + std::lock_guard guard(_monitor); //LOG(info, "VisitorAgent[%u]: setState(%s)", _id, newState == IDLE ? "idle" : // (newState == RUNNING ? "running" : "finished")); _state = newState; @@ -343,23 +341,23 @@ public: const EntryGenerator & generator, uint32_t id, bool validate) : Agent(tlsSpec, domain, generator, "VisitorAgent", id, validate), _visitor(), _from(0), _to(0), _next(0), _state(IDLE) {} - virtual ~VisitorAgent() {} + ~VisitorAgent() override = default; void start(SerialNum from, SerialNum to); void setIdle(); bool idle() { - MonitorGuard guard(_monitor); + std::lock_guard guard(_monitor); return _state == IDLE; } bool running() { - MonitorGuard guard(_monitor); + std::lock_guard guard(_monitor); return _state == RUNNING; } bool finished() { - MonitorGuard guard(_monitor); + std::lock_guard guard(_monitor); return _state == FINISHED; } std::string getState() { - MonitorGuard guard(_monitor); + std::lock_guard guard(_monitor); if (_state == IDLE) { return std::string("idle"); } else if (_state == FINISHED) { @@ -516,7 +514,7 @@ void ControllerThread::doRun() { _session = _client.open(_domain); - if (_session.get() == NULL) { + if ( ! _session) { throw std::runtime_error(vespalib::make_string("ControllerThread: Could not open session to %s", _tlsSpec.c_str())); } diff --git a/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp b/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp index 40db92cbe78..11568ad2e36 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp @@ -9,7 +9,6 @@ #include <vespa/log/log.h> LOG_SETUP(".translog.client_session"); -using vespalib::LockGuard; using namespace std::chrono_literals; namespace search::transactionlog::client { diff --git a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp index 9efc68bc8ec..07da8087544 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp @@ -22,8 +22,6 @@ using vespalib::make_string_short::fmt; using vespalib::makeTask; using vespalib::makeClosure; using vespalib::makeLambdaTask; -using vespalib::Monitor; -using vespalib::MonitorGuard; using std::runtime_error; using std::make_shared; diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp b/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp index 84919a59a97..1883943a53f 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp @@ -45,8 +45,6 @@ struct RpcTask : public vespalib::Executor::Task { } -using vespalib::LockGuard; - TransLogClient::TransLogClient(const vespalib::string & rpcTarget) : _executor(std::make_unique<vespalib::ThreadStackExecutor>(1, 128 * 1024, translogclient_rpc_callback)), _rpcTarget(rpcTarget), diff --git a/slobrok/src/vespa/slobrok/sbmirror.cpp b/slobrok/src/vespa/slobrok/sbmirror.cpp index 855ed4dd82d..460d61cb2a8 100644 --- a/slobrok/src/vespa/slobrok/sbmirror.cpp +++ b/slobrok/src/vespa/slobrok/sbmirror.cpp @@ -7,10 +7,7 @@ #include <vespa/log/log.h> LOG_SETUP(".slobrok.mirror"); -using vespalib::LockGuard; - -namespace slobrok { -namespace api { +namespace slobrok::api { MirrorAPI::MirrorAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config) : FNET_Task(orb.GetScheduler()), @@ -326,5 +323,4 @@ MirrorAPI::RequestDone(FRT_RPCRequest *req) ScheduleNow(); } -} // namespace api -} // namespace slobrok +} diff --git a/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp b/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp index 5dacaa5d204..622c9b9985f 100644 --- a/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp +++ b/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp @@ -28,7 +28,8 @@ TEST("test that all tasks are executed") { } void verifyResizeTaskLimit(bool up) { - Monitor lock; + std::mutex lock; + std::condition_variable cond; std::atomic<uint64_t> started(0); std::atomic<uint64_t> allowed(0); SingleExecutor executor(10); @@ -38,11 +39,11 @@ void verifyResizeTaskLimit(bool up) { EXPECT_NOT_EQUAL(16u, roundedTaskLimit); for (uint64_t i(0); i < 10; i++) { - executor.execute(makeLambdaTask([&lock, &started, &allowed] { + executor.execute(makeLambdaTask([&lock, &cond, &started, &allowed] { started++; - MonitorGuard guard(lock); + std::unique_lock guard(lock); while (allowed < started) { - guard.wait(1ms); + cond.wait_for(guard, 1ms); } })); } @@ -58,11 +59,11 @@ void verifyResizeTaskLimit(bool up) { while (started < 10); EXPECT_EQUAL(10u, started); EXPECT_EQUAL(16u, executor.getTaskLimit()); - executor.execute(makeLambdaTask([&lock, &started, &allowed] { + executor.execute(makeLambdaTask([&lock, &cond, &started, &allowed] { started++; - MonitorGuard guard(lock); + std::unique_lock guard(lock); while (allowed < started) { - guard.wait(1ms); + cond.wait_for(guard, 1ms); } })); while (started < 11); diff --git a/storage/src/tests/storageserver/dummystoragelink.cpp b/storage/src/tests/storageserver/dummystoragelink.cpp deleted file mode 100644 index ab7c413f2ea..00000000000 --- a/storage/src/tests/storageserver/dummystoragelink.cpp +++ /dev/null @@ -1,181 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/storageframework/defaultimplementation/clock/realclock.h> -#include <tests/common/dummystoragelink.h> -#include <sys/time.h> - -namespace storage { - -DummyStorageLink* DummyStorageLink::_last(0); - -DummyStorageLink::DummyStorageLink() - : StorageLink("Dummy storage link"), - _commands(), - _replies(), - _injected(), - _autoReply(false), - _useDispatch(false), - _ignore(false), - _waitMonitor() -{ - _last = this; -} - -DummyStorageLink::~DummyStorageLink() -{ - // Often a chain with dummy link on top is deleted in unit tests. - // If they haven't been closed already, close them for a cleaner - // shutdown - if (getState() == OPENED) { - close(); - flush(); - } - closeNextLink(); - reset(); -} - -bool DummyStorageLink::onDown(const api::StorageMessage::SP& cmd) -{ - if (_ignore) { - return false; - } - if (_injected.size() > 0) { - vespalib::LockGuard guard(_lock); - sendUp(*_injected.begin()); - _injected.pop_front(); - } else if (_autoReply) { - if (!cmd->getType().isReply()) { - std::shared_ptr<api::StorageReply> reply( - std::dynamic_pointer_cast<api::StorageCommand>(cmd) - ->makeReply().release()); - reply->setResult(api::ReturnCode( - api::ReturnCode::OK, "Automatically generated reply")); - sendUp(reply); - } - } - if (isBottom()) { - vespalib::MonitorGuard lock(_waitMonitor); - { - vespalib::LockGuard guard(_lock); - _commands.push_back(cmd); - } - lock.broadcast(); - return true; - } - return StorageLink::onDown(cmd); -} - -bool DummyStorageLink::onUp(const api::StorageMessage::SP& reply) { - if (isTop()) { - vespalib::MonitorGuard lock(_waitMonitor); - { - vespalib::LockGuard guard(_lock); - _replies.push_back(reply); - } - lock.broadcast(); - return true; - } - return StorageLink::onUp(reply); - -} - -void DummyStorageLink::injectReply(api::StorageReply* reply) -{ - assert(reply); - vespalib::LockGuard guard(_lock); - _injected.push_back(std::shared_ptr<api::StorageReply>(reply)); -} - -void DummyStorageLink::reset() { - vespalib::MonitorGuard lock(_waitMonitor); - vespalib::LockGuard guard(_lock); - _commands.clear(); - _replies.clear(); - _injected.clear(); -} - -void DummyStorageLink::waitForMessages(unsigned int msgCount, int timeout) -{ - framework::defaultimplementation::RealClock clock; - framework::MilliSecTime endTime( - clock.getTimeInMillis() + framework::MilliSecTime(timeout * 1000)); - vespalib::MonitorGuard lock(_waitMonitor); - while (_commands.size() + _replies.size() < msgCount) { - if (timeout != 0 && clock.getTimeInMillis() > endTime) { - std::ostringstream ost; - ost << "Timed out waiting for " << msgCount << " messages to " - << "arrive in dummy storage link. Only " - << (_commands.size() + _replies.size()) << " messages seen " - << "after timout of " << timeout << " seconds was reached."; - throw vespalib::IllegalStateException(ost.str(), VESPA_STRLOC); - } - if (timeout >= 0) { - lock.wait((endTime - clock.getTimeInMillis()).getTime()); - } else { - lock.wait(); - } - } -} - -void DummyStorageLink::waitForMessage(const api::MessageType& type, int timeout) -{ - framework::defaultimplementation::RealClock clock; - framework::MilliSecTime endTime( - clock.getTimeInMillis() + framework::MilliSecTime(timeout * 1000)); - vespalib::MonitorGuard lock(_waitMonitor); - while (true) { - for (uint32_t i=0; i<_commands.size(); ++i) { - if (_commands[i]->getType() == type) return; - } - for (uint32_t i=0; i<_replies.size(); ++i) { - if (_replies[i]->getType() == type) return; - } - if (timeout != 0 && clock.getTimeInMillis() > endTime) { - std::ostringstream ost; - ost << "Timed out waiting for " << type << " message to " - << "arrive in dummy storage link. Only " - << (_commands.size() + _replies.size()) << " messages seen " - << "after timout of " << timeout << " seconds was reached."; - if (_commands.size() == 1) { - ost << " Found command of type " << _commands[0]->getType(); - } - if (_replies.size() == 1) { - ost << " Found command of type " << _replies[0]->getType(); - } - throw vespalib::IllegalStateException(ost.str(), VESPA_STRLOC); - } - if (timeout >= 0) { - lock.wait((endTime - clock.getTimeInMillis()).getTime()); - } else { - lock.wait(); - } - } -} - -api::StorageMessage::SP -DummyStorageLink::getAndRemoveMessage(const api::MessageType& type) -{ - vespalib::MonitorGuard lock(_waitMonitor); - for (std::vector<api::StorageMessage::SP>::iterator it = _commands.begin(); - it != _commands.end(); ++it) - { - if ((*it)->getType() == type) { - api::StorageMessage::SP result(*it); - _commands.erase(it); - return result; - } - } - for (std::vector<api::StorageMessage::SP>::iterator it = _replies.begin(); - it != _replies.end(); ++it) - { - if ((*it)->getType() == type) { - api::StorageMessage::SP result(*it); - _replies.erase(it); - return result; - } - } - std::ostringstream ost; - ost << "No message of type " << type << " found."; - throw vespalib::IllegalStateException(ost.str(), VESPA_STRLOC); -} - -} // storage diff --git a/storage/src/tests/storageserver/dummystoragelink.h b/storage/src/tests/storageserver/dummystoragelink.h deleted file mode 100644 index 3902e1db19c..00000000000 --- a/storage/src/tests/storageserver/dummystoragelink.h +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <vespa/vespalib/util/sync.h> -#include <list> -#include <sstream> -#include <vespa/storageapi/messageapi/storagecommand.h> -#include <string> -#include <vector> -#include <vespa/storage/common/storagelink.h> -#include <vespa/storage/common/bucketmessages.h> -#include <vespa/storageapi/message/internal.h> - -class FastOS_ThreadPool; - -namespace storage { - -class DummyStorageLink : public StorageLink { - - mutable vespalib::Lock _lock; // to protect below containers: - std::vector<api::StorageMessage::SP> _commands; - std::vector<api::StorageMessage::SP> _replies; - std::list<api::StorageMessage::SP> _injected; - - bool _autoReply; - bool _useDispatch; - bool _ignore; - static DummyStorageLink* _last; - vespalib::Monitor _waitMonitor; - -public: - DummyStorageLink(); - ~DummyStorageLink(); - - bool onDown(const api::StorageMessage::SP&); - bool onUp(const api::StorageMessage::SP&); - - void addOnTopOfChain(StorageLink& link) { - link.addTestLinkOnTop(this); - } - - void print(std::ostream& ost, bool verbose, const std::string& indent) const - { - (void) verbose; - ost << indent << "DummyStorageLink(" - << "autoreply = " << (_autoReply ? "on" : "off") - << ", dispatch = " << (_useDispatch ? "on" : "off") - << ", " << _commands.size() << " commands" - << ", " << _replies.size() << " replies"; - if (_injected.size() > 0) - ost << ", " << _injected.size() << " injected"; - ost << ")"; - } - - void injectReply(api::StorageReply* reply); - void reset(); - void setAutoreply(bool autoReply) { _autoReply = autoReply; } - void setIgnore(bool ignore) { _ignore = ignore; } - // Timeout is given in seconds - void waitForMessages(unsigned int msgCount = 1, int timeout = -1); - // Wait for a single message of a given type - void waitForMessage(const api::MessageType&, int timeout = -1); - - api::StorageMessage::SP getCommand(size_t i) const { - vespalib::LockGuard guard(_lock); - api::StorageMessage::SP ret = _commands[i]; - return ret; - } - api::StorageMessage::SP getReply(size_t i) const { - vespalib::LockGuard guard(_lock); - api::StorageMessage::SP ret = _replies[i]; - return ret; - } - size_t getNumCommands() const { - vespalib::LockGuard guard(_lock); - return _commands.size(); - } - size_t getNumReplies() const { - vespalib::LockGuard guard(_lock); - return _replies.size(); - } - - const std::vector<api::StorageMessage::SP>& getCommands() const - { return _commands; } - const std::vector<api::StorageMessage::SP>& getReplies() const - { return _replies; } - - std::vector<api::StorageMessage::SP> getCommandsOnce() { - vespalib::MonitorGuard lock(_waitMonitor); - std::vector<api::StorageMessage::SP> retval; - { - vespalib::LockGuard guard(_lock); - retval.swap(_commands); - } - return retval; - } - - std::vector<api::StorageMessage::SP> getRepliesOnce() { - vespalib::MonitorGuard lock(_waitMonitor); - std::vector<api::StorageMessage::SP> retval; - { - vespalib::LockGuard guard(_lock); - retval.swap(_replies); - } - return retval; - } - - api::StorageMessage::SP getAndRemoveMessage(const api::MessageType&); - - static DummyStorageLink* getLast() { return _last; } -}; - -} - diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp index 3c2d040010c..237acbb2d2c 100644 --- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp +++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp @@ -2,7 +2,6 @@ #include "statuswebserver.h" #include <vespa/storageframework/storageframework.h> -#include <vespa/storageapi/message/persistence.h> #include <vespa/vespalib/util/host_name.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/component/vtag.h> @@ -19,7 +18,6 @@ StatusWebServer::StatusWebServer( framework::StatusReporterMap& reporterMap, const config::ConfigUri & configUri) : _reporterMap(reporterMap), - _workerMonitor(), _port(0), _httpServer(), _configFetcher(configUri.getContext()), @@ -34,11 +32,11 @@ StatusWebServer::~StatusWebServer() // Avoid getting config during shutdown _configFetcher.close(); - if (_httpServer.get() != 0) { + if (_httpServer) { LOG(debug, "Shutting down status web server on port %u", _httpServer->getListenPort()); } // Delete http server to ensure that no more incoming requests reach us. - _httpServer.reset(0); + _httpServer.reset(); } void StatusWebServer::configure(std::unique_ptr<vespa::config::content::core::StorStatusConfig> config) @@ -55,7 +53,7 @@ void StatusWebServer::configure(std::unique_ptr<vespa::config::content::core::St // Negative port number means don't run the web server if (newPort >= 0) { try { - server.reset(new WebServer(*this, newPort)); + server = std::make_unique<WebServer>(*this, newPort); } catch (const vespalib::PortListenException & e) { LOG(error, "Failed listening to network port(%d) with protocol(%s): '%s', giving up and restarting.", e.get_port(), e.get_protocol().c_str(), e.what()); @@ -148,7 +146,7 @@ void StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, vespalib::Portal::GetRequest request) { vespalib::string link(urlpath.getPath()); - if (link.size() > 0 && link[0] == '/') link = link.substr(1); + if (!link.empty() && link[0] == '/') link = link.substr(1); size_t slashPos = link.find('/'); if (slashPos != std::string::npos) link = link.substr(0, slashPos); diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h index e94af34346a..5955339d43b 100644 --- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h +++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h @@ -62,7 +62,6 @@ class StatusWebServer : private config::IFetcherCallback<vespa::config::content: }; framework::StatusReporterMap& _reporterMap; - vespalib::Monitor _workerMonitor; uint16_t _port; std::unique_ptr<WebServer> _httpServer; config::ConfigFetcher _configFetcher; diff --git a/storage/src/vespa/storage/visiting/visitorlibraries.cpp b/storage/src/vespa/storage/visiting/visitorlibraries.cpp deleted file mode 100644 index 39dc49ddf10..00000000000 --- a/storage/src/vespa/storage/visiting/visitorlibraries.cpp +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "visitorlibraries.h" -#include <vespa/defaults.h> - -#include <vespa/log/log.h> - -LOG_SETUP(".visiting.libraryloader"); - -namespace storage { - -VisitorLibraries::LibMap VisitorLibraries::_libs; -vespalib::Lock VisitorLibraries::_libLock; - -/** - * Utility function to get a dynamic library. - * Assumes _libLock has been grabbed before calling. - */ -VisitorLibraries::LibraryRef -VisitorLibraries::getLibrary(StorageServerInterface& storageServer, const std::string& libName, const std::string& libraryPath) -{ - vespalib::LockGuard guard(_libLock); - - LibMap::iterator it = _libs.find(libName); - if (it != _libs.end()) { - return LibraryRef(it->second.factory, it->second.environment.get()); - } - - std::shared_ptr<FastOS_DynamicLibrary> lib(new FastOS_DynamicLibrary); - std::string file = libraryPath + "lib" + libName + ".so"; - if (!lib->Open(file.c_str())) { - std::string error = lib->GetLastErrorString(); - std::string absfile = vespa::Defaults::underVespaHome("libexec/vespa/storage/"); - absfile.append("lib" + libName + ".so"); - if (!lib->Open(absfile.c_str())) { - LOG(error, "Could not load library %s: %s", - file.c_str(), error.c_str()); - return LibraryRef(); - } - } - std::shared_ptr<VisitorEnvironment> env( - getVisitorEnvironment(storageServer, *lib, libName)); - - LibMapEntry entry; - entry.library = lib; - entry.environment = env; - entry.factory = lib.get() ? (VisitorFactoryFuncT) lib->GetSymbol("makeVisitor") : 0; - _libs[libName] = entry; - - return LibraryRef(entry.factory, env.get()); -} - -std::shared_ptr<VisitorEnvironment> -VisitorLibraries::getVisitorEnvironment(StorageServerInterface& storageServer, FastOS_DynamicLibrary& lib, - const std::string& libName) -{ - typedef VisitorEnvironment::UP - (*VisitorEnvFuncT)(StorageServerInterface& server); - VisitorEnvFuncT factoryFunc - = (VisitorEnvFuncT) lib.GetSymbol("makeVisitorEnvironment"); - if (factoryFunc == 0) { - std::string err = lib.GetLastErrorString(); - LOG(error, "Unable to load symbol 'makeVisitorEnvironment' from " - "'%s': %s", libName.c_str(), err.c_str()); - return std::shared_ptr<VisitorEnvironment>(); - } - return std::shared_ptr<VisitorEnvironment>( - factoryFunc(storageServer).release()); -} - -} diff --git a/storage/src/vespa/storage/visiting/visitorlibraries.h b/storage/src/vespa/storage/visiting/visitorlibraries.h deleted file mode 100644 index 74778b0483d..00000000000 --- a/storage/src/vespa/storage/visiting/visitorlibraries.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -/** - This class handles ownership and creation of dynamic visitor libraries. -*/ - -#include "visitor.h" - -namespace storage { - -class VisitorLibraries { -public: - typedef Visitor* (*VisitorFactoryFuncT)(StorageServerInterface& server, - VisitorEnvironment& env, - const vdslib::Parameters& params); - - struct LibMapEntry { - std::shared_ptr<FastOS_DynamicLibrary> library; - std::shared_ptr<VisitorEnvironment> environment; - VisitorFactoryFuncT factory; - }; - - typedef std::map<std::string, LibMapEntry> LibMap; - typedef std::pair<VisitorFactoryFuncT, VisitorEnvironment*> LibraryRef; - - static LibraryRef getLibrary(StorageServerInterface& storageServer, const std::string& libName, const std::string& libraryPath); - -private: - static LibMap _libs; - static vespalib::Lock _libLock; - - static std::shared_ptr<VisitorEnvironment> getVisitorEnvironment(StorageServerInterface& storageServer, - FastOS_DynamicLibrary& lib, - const std::string& libName); -}; - -} diff --git a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp index e0d78c5d1b7..4f93f891cf8 100644 --- a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp +++ b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp @@ -50,6 +50,14 @@ Runnable::UP chain(Runnable::UP first, Runnable::UP second) { //----------------------------------------------------------------------------- +Signal::Signal() noexcept + : valid(true), + generation(0), + monitor(std::make_unique<std::mutex>()), + cond(std::make_unique<std::condition_variable>()) +{} +Signal::~Signal() = default; + SimpleThreadBundle::Pool::Pool(size_t bundleSize) : _lock(), _bundleSize(bundleSize), diff --git a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h index 135fc2d7562..40844b277f1 100644 --- a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h +++ b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h @@ -6,7 +6,6 @@ #include "thread.h" #include "runnable.h" #include "thread_bundle.h" -#include "noncopyable.hpp" namespace vespalib { @@ -46,32 +45,35 @@ struct Part { struct Signal { bool valid; size_t generation; - Monitor monitor; - Signal() noexcept : valid(true), generation(0), monitor() {} + std::unique_ptr<std::mutex> monitor; + std::unique_ptr<std::condition_variable> cond; + Signal() noexcept; + Signal(Signal &&) noexcept = default; + ~Signal(); size_t wait(size_t &localGen) { - MonitorGuard guard(monitor); + std::unique_lock guard(*monitor); while (localGen == generation) { - guard.wait(); + cond->wait(guard); } size_t diff = (generation - localGen); localGen = generation; return (valid ? diff : 0); } void send() { - MonitorGuard guard(monitor); + std::lock_guard guard(*monitor); ++generation; - guard.signal(); + cond->notify_one(); } void broadcast() { - MonitorGuard guard(monitor); + std::lock_guard guard(*monitor); ++generation; - guard.broadcast(); + cond->notify_all(); } void cancel() { - MonitorGuard guard(monitor); + std::lock_guard guard(*monitor); ++generation; valid = false; - guard.broadcast(); + cond->notify_all(); } }; @@ -105,7 +107,7 @@ public: }; private: - struct Worker : Runnable, noncopyable { + struct Worker : Runnable { using UP = std::unique_ptr<Worker>; Thread thread; Signal &signal; |