diff options
59 files changed, 353 insertions, 272 deletions
diff --git a/config/src/vespa/config/frt/frtsource.cpp b/config/src/vespa/config/frt/frtsource.cpp index 94acabc5926..449ad5df1ba 100644 --- a/config/src/vespa/config/frt/frtsource.cpp +++ b/config/src/vespa/config/frt/frtsource.cpp @@ -3,6 +3,7 @@ #include "frtconfigresponse.h" #include "frtsource.h" #include <vespa/vespalib/util/closuretask.h> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".config.frt.frtsource"); @@ -73,7 +74,7 @@ FRTSource::RequestDone(FRT_RPCRequest * request) LOG(debug, "request aborted, stopping"); return; } - assert(_currentRequest.get() != NULL); + assert(_currentRequest); // If this was error from FRT side and nothing to do with config, notify // connection about the error. if (request->IsError()) { diff --git a/config/src/vespa/config/frt/protocol.cpp b/config/src/vespa/config/frt/protocol.cpp index 4ad55726863..cfd248e5d86 100644 --- a/config/src/vespa/config/frt/protocol.cpp +++ b/config/src/vespa/config/frt/protocol.cpp @@ -4,6 +4,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/data/slime/slime.h> #include <sstream> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".config.frt.protocol"); diff --git a/config/src/vespa/config/retriever/configretriever.cpp b/config/src/vespa/config/retriever/configretriever.cpp index 08e7f15f0c5..f144cf9aa99 100644 --- a/config/src/vespa/config/retriever/configretriever.cpp +++ b/config/src/vespa/config/retriever/configretriever.cpp @@ -2,6 +2,7 @@ #include "configretriever.h" #include <vespa/config/common/exceptions.h> +#include <cassert> using std::chrono::milliseconds; diff --git a/config/src/vespa/config/retriever/simpleconfigurer.cpp b/config/src/vespa/config/retriever/simpleconfigurer.cpp index 1f8868f9d6a..0091fa7bfd5 100644 --- a/config/src/vespa/config/retriever/simpleconfigurer.cpp +++ b/config/src/vespa/config/retriever/simpleconfigurer.cpp @@ -1,8 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include "simpleconfigurer.h" +#include <cassert> + #include <vespa/log/log.h> LOG_SETUP(".config.retriever.simpleconfigurer"); -#include "simpleconfigurer.h" namespace config { @@ -12,7 +14,7 @@ SimpleConfigurer::SimpleConfigurer(SimpleConfigRetriever::UP retriever, SimpleCo _thread(*this), _started(false) { - assert(_retriever.get() != NULL); + assert(_retriever); } void diff --git a/config/src/vespa/config/subscription/sourcespec.cpp b/config/src/vespa/config/subscription/sourcespec.cpp index 30926d75465..326b3191fd0 100644 --- a/config/src/vespa/config/subscription/sourcespec.cpp +++ b/config/src/vespa/config/subscription/sourcespec.cpp @@ -10,6 +10,7 @@ #include <vespa/config/set/configinstancesourcefactory.h> #include <vespa/vespalib/text/stringtokenizer.h> #include <vespa/config/print/asciiconfigwriter.h> +#include <cassert> namespace config { diff --git a/document/src/vespa/document/repo/documenttyperepo.cpp b/document/src/vespa/document/repo/documenttyperepo.cpp index 677da16190d..578d2999038 100644 --- a/document/src/vespa/document/repo/documenttyperepo.cpp +++ b/document/src/vespa/document/repo/documenttyperepo.cpp @@ -15,6 +15,7 @@ #include <vespa/vespalib/util/exceptions.h> #include <vespa/document/config/config-documenttypes.h> #include <fstream> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".documenttyperepo"); diff --git a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp index 7d7153e8461..dcfc0fa5f6e 100644 --- a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp @@ -9,6 +9,7 @@ #include <vespa/documentapi/documentapi.h> #include <vespa/vespalib/util/exceptions.h> #include <sstream> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".documentprotocol"); diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp index 166c3be5e66..e49d412fee1 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp @@ -11,6 +11,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <vespa/config-stor-distribution.h> #include <vespa/config/subscription/configuri.h> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".storagepolicy"); diff --git a/messagebus/src/vespa/messagebus/protocolrepository.cpp b/messagebus/src/vespa/messagebus/protocolrepository.cpp index a9891069c44..10ea4ecb25d 100644 --- a/messagebus/src/vespa/messagebus/protocolrepository.cpp +++ b/messagebus/src/vespa/messagebus/protocolrepository.cpp @@ -1,5 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "protocolrepository.h" +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".protocolrepository"); diff --git a/messagebus/src/vespa/messagebus/routing/routingnode.cpp b/messagebus/src/vespa/messagebus/routing/routingnode.cpp index a47abe185cc..f24afbc07ca 100644 --- a/messagebus/src/vespa/messagebus/routing/routingnode.cpp +++ b/messagebus/src/vespa/messagebus/routing/routingnode.cpp @@ -10,6 +10,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <vespa/messagebus/network/inetwork.h> #include <stack> +#include <cassert> using vespalib::make_string; diff --git a/messagebus/src/vespa/messagebus/sequencer.cpp b/messagebus/src/vespa/messagebus/sequencer.cpp index 2d7a36a28ef..e19dfa0ee61 100644 --- a/messagebus/src/vespa/messagebus/sequencer.cpp +++ b/messagebus/src/vespa/messagebus/sequencer.cpp @@ -2,6 +2,7 @@ #include "sequencer.h" #include "tracelevel.h" #include <vespa/vespalib/util/stringfmt.h> +#include <cassert> using vespalib::make_string; diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp index 84e92c2c316..7db9686d5c8 100644 --- a/metrics/src/vespa/metrics/metricmanager.cpp +++ b/metrics/src/vespa/metrics/metricmanager.cpp @@ -13,6 +13,7 @@ #include <vespa/vespalib/stllike/hashtable.hpp> #include <sstream> #include <algorithm> +#include <cassert> #include <vespa/log/bufferedlogger.h> LOG_SETUP(".metrics.manager"); diff --git a/metrics/src/vespa/metrics/metricsnapshot.cpp b/metrics/src/vespa/metrics/metricsnapshot.cpp index 86a33f0993f..f7460c8b6ad 100644 --- a/metrics/src/vespa/metrics/metricsnapshot.cpp +++ b/metrics/src/vespa/metrics/metricsnapshot.cpp @@ -1,8 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "metricsnapshot.h" #include "metricmanager.h" -#include <vespa/log/log.h> +#include <cassert> +#include <vespa/log/log.h> LOG_SETUP(".metrics.snapshot"); namespace metrics { diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp index 297a3319939..3285e03db67 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp @@ -12,6 +12,7 @@ #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/stllike/hash_map.hpp> #include <algorithm> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".dummypersistence"); diff --git a/searchcore/src/vespa/searchcore/proton/matching/partial_result.h b/searchcore/src/vespa/searchcore/proton/matching/partial_result.h index 5bcc8dafea2..ac3c6ab5e0f 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/partial_result.h +++ b/searchcore/src/vespa/searchcore/proton/matching/partial_result.h @@ -5,6 +5,7 @@ #include <vespa/vespalib/util/dual_merge_director.h> #include <vespa/searchlib/common/rankedhit.h> #include <vector> +#include <cassert> namespace proton::matching { @@ -28,7 +29,7 @@ private: public: PartialResult(size_t maxSize_in, bool hasSortData_in); - ~PartialResult(); + ~PartialResult() override; size_t size() const { return _hits.size(); } size_t maxSize() const { return _maxSize; } size_t totalHits() const { return _totalHits; } diff --git a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp index b8572541f36..7a590b66a8a 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "activediskindexes.h" +#include <cassert> using std::set; using vespalib::string; diff --git a/searchlib/src/vespa/searchlib/bitcompression/posocc_field_params.cpp b/searchlib/src/vespa/searchlib/bitcompression/posocc_field_params.cpp index b789bf16947..751c1db50b2 100644 --- a/searchlib/src/vespa/searchlib/bitcompression/posocc_field_params.cpp +++ b/searchlib/src/vespa/searchlib/bitcompression/posocc_field_params.cpp @@ -5,6 +5,7 @@ #include <vespa/searchlib/index/postinglistparams.h> #include <vespa/vespalib/data/fileheader.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".posocc_field_params"); diff --git a/searchlib/src/vespa/searchlib/index/schemautil.cpp b/searchlib/src/vespa/searchlib/index/schemautil.cpp index 1fce4a1fe99..57a90892d4f 100644 --- a/searchlib/src/vespa/searchlib/index/schemautil.cpp +++ b/searchlib/src/vespa/searchlib/index/schemautil.cpp @@ -3,6 +3,7 @@ #include "schemautil.h" #include <set> #include <fstream> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".index.schemautil"); diff --git a/searchlib/src/vespa/searchlib/index/uri_field.cpp b/searchlib/src/vespa/searchlib/index/uri_field.cpp index 30b78c24410..af0881dd6b4 100644 --- a/searchlib/src/vespa/searchlib/index/uri_field.cpp +++ b/searchlib/src/vespa/searchlib/index/uri_field.cpp @@ -1,6 +1,7 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "uri_field.h" +#include <cassert> namespace search::index { diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_tree_analyzer.cpp b/searchlib/src/vespa/searchlib/predicate/predicate_tree_analyzer.cpp index e8257d28c63..dba6ebfb117 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_tree_analyzer.cpp +++ b/searchlib/src/vespa/searchlib/predicate/predicate_tree_analyzer.cpp @@ -4,6 +4,7 @@ #include <vespa/document/predicate/predicate.h> #include <algorithm> #include <cmath> +#include <cassert> using document::Predicate; using std::map; diff --git a/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp b/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp index b7e02894e6b..97ab61d3045 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp @@ -5,6 +5,7 @@ #include <vespa/vespalib/data/fileheader.h> #include <vespa/searchlib/common/fileheadercontext.h> #include <vespa/fastlib/io/bufferedfile.h> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".transactionlog.domainpart"); diff --git a/searchlib/src/vespa/searchlib/transactionlog/session.cpp b/searchlib/src/vespa/searchlib/transactionlog/session.cpp index c91b719be37..3aedeb11121 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/session.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/session.cpp @@ -3,8 +3,9 @@ #include "domain.h" #include "domainpart.h" #include <vespa/fastlib/io/bufferedfile.h> -#include <vespa/log/log.h> +#include <cassert> +#include <vespa/log/log.h> LOG_SETUP(".transactionlog.session"); using vespalib::LockGuard; diff --git a/staging_vespalib/src/vespa/vespalib/util/document_runnable.cpp b/staging_vespalib/src/vespa/vespalib/util/document_runnable.cpp index c0a32197c07..0f7e6155276 100644 --- a/staging_vespalib/src/vespa/vespalib/util/document_runnable.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/document_runnable.cpp @@ -2,6 +2,7 @@ #include "document_runnable.h" #include <vespa/vespalib/util/exceptions.h> +#include <cassert> namespace document { diff --git a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp index 65857183879..795a7ef1ec3 100644 --- a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp @@ -1,7 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "singleexecutor.h" -#include <vespa/vespalib/util/time.h> +#include <cassert> namespace vespalib { diff --git a/storage/src/tests/common/dummystoragelink.cpp b/storage/src/tests/common/dummystoragelink.cpp index a747fbcf998..05ef0df432a 100644 --- a/storage/src/tests/common/dummystoragelink.cpp +++ b/storage/src/tests/common/dummystoragelink.cpp @@ -1,12 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "dummystoragelink.h" #include <vespa/storageframework/defaultimplementation/clock/realclock.h> -#include <sys/time.h> #include <vespa/vespalib/util/exceptions.h> +#include <cassert> namespace storage { -DummyStorageLink* DummyStorageLink::_last(0); +DummyStorageLink* DummyStorageLink::_last(nullptr); DummyStorageLink::DummyStorageLink() : StorageLink("Dummy storage link"), diff --git a/storage/src/tests/common/teststorageapp.cpp b/storage/src/tests/common/teststorageapp.cpp index 1847de0e84f..c03e17138e0 100644 --- a/storage/src/tests/common/teststorageapp.cpp +++ b/storage/src/tests/common/teststorageapp.cpp @@ -57,9 +57,9 @@ TestStorageApp::TestStorageApp(StorageComponentRegisterImpl::UP compReg, _compReg.setPriorityConfig( *config::ConfigGetter<StorageComponent::PriorityConfig> ::getConfig(uri.getConfigId(), uri.getContext())); - loadTypes.reset(new documentapi::LoadTypeSet( + loadTypes = std::make_shared<documentapi::LoadTypeSet>( *config::ConfigGetter<vespa::config::content::LoadTypeConfig> - ::getConfig(uri.getConfigId(), uri.getContext()))); + ::getConfig(uri.getConfigId(), uri.getContext())); } else { if (index == 0xffff) index = 0; loadTypes.reset(new documentapi::LoadTypeSet); @@ -72,20 +72,18 @@ TestStorageApp::TestStorageApp(StorageComponentRegisterImpl::UP compReg, _compReg.setDocumentTypeRepo(_docMan.getTypeRepoSP()); _compReg.setLoadTypes(loadTypes); _compReg.setBucketIdFactory(document::BucketIdFactory()); - lib::Distribution::SP distr(new lib::Distribution( - lib::Distribution::getDefaultDistributionConfig( - redundancy, nodeCount))); + auto distr = std::make_shared<lib::Distribution>( + lib::Distribution::getDefaultDistributionConfig(redundancy, nodeCount)); _compReg.setDistribution(distr); } -TestStorageApp::~TestStorageApp() {} +TestStorageApp::~TestStorageApp() = default; void TestStorageApp::setDistribution(Redundancy redundancy, NodeCount nodeCount) { - lib::Distribution::SP distr(new lib::Distribution( - lib::Distribution::getDefaultDistributionConfig( - redundancy, nodeCount))); + auto distr = std::make_shared<lib::Distribution>( + lib::Distribution::getDefaultDistributionConfig(redundancy, nodeCount)); _compReg.setDistribution(distr); } @@ -98,8 +96,7 @@ TestStorageApp::setTypeRepo(std::shared_ptr<const document::DocumentTypeRepo> re void TestStorageApp::setClusterState(const lib::ClusterState& c) { - _nodeStateUpdater.setClusterState( - lib::ClusterState::CSP(new lib::ClusterState(c))); + _nodeStateUpdater.setClusterState(std::make_shared<lib::ClusterState>(c)); } void @@ -109,8 +106,7 @@ TestStorageApp::waitUntilInitialized( // Always use real clock for wait timeouts. Component clock may be faked // in tests framework::defaultimplementation::RealClock clock; - framework::MilliSecTime endTime( - clock.getTimeInMillis() + timeout.getMillis()); + framework::MilliSecTime endTime(clock.getTimeInMillis() + timeout.getMillis()); while (!isInitialized()) { std::this_thread::sleep_for(1ms); framework::MilliSecTime currentTime(clock.getTimeInMillis()); @@ -142,8 +138,7 @@ namespace { TestServiceLayerApp::TestServiceLayerApp(vespalib::stringref configId) : TestStorageApp(std::make_unique<ServiceLayerComponentRegisterImpl>(true), // TODO remove B-tree flag once default lib::NodeType::STORAGE, getIndexFromConfig(configId), configId), - _compReg(dynamic_cast<ServiceLayerComponentRegisterImpl&>( - TestStorageApp::getComponentRegister())), + _compReg(dynamic_cast<ServiceLayerComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), _persistenceProvider(), _partitions(1) { @@ -157,8 +152,7 @@ TestServiceLayerApp::TestServiceLayerApp(DiskCount dc, NodeIndex index, vespalib::stringref configId) : TestStorageApp(std::make_unique<ServiceLayerComponentRegisterImpl>(true), // TODO remove B-tree flag once default lib::NodeType::STORAGE, index, configId), - _compReg(dynamic_cast<ServiceLayerComponentRegisterImpl&>( - TestStorageApp::getComponentRegister())), + _compReg(dynamic_cast<ServiceLayerComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), _persistenceProvider(), _partitions(dc) { @@ -192,9 +186,7 @@ spi::PersistenceProvider& TestServiceLayerApp::getPersistenceProvider() { if (_persistenceProvider.get() == 0) { - throw vespalib::IllegalStateException( - "Persistence provider requested but not initialized.", - VESPA_STRLOC); + throw vespalib::IllegalStateException("Persistence provider requested but not initialized.", VESPA_STRLOC); } return *_persistenceProvider; } @@ -203,9 +195,7 @@ spi::PartitionStateList& TestServiceLayerApp::getPartitions() { if (_persistenceProvider.get() == 0) { - throw vespalib::IllegalStateException( - "Partition list requested but not initialized.", - VESPA_STRLOC); + throw vespalib::IllegalStateException("Partition list requested but not initialized.", VESPA_STRLOC); } return _partitions; } @@ -224,8 +214,7 @@ namespace { template<typename T> const T getConfig(vespalib::stringref configId) { config::ConfigUri uri(configId); - return *config::ConfigGetter<T>::getConfig( - uri.getConfigId(), uri.getContext()); + return *config::ConfigGetter<T>::getConfig(uri.getConfigId(), uri.getContext()); } } @@ -243,8 +232,7 @@ TestDistributorApp::TestDistributorApp(vespalib::stringref configId) : TestStorageApp( std::make_unique<DistributorComponentRegisterImpl>(), lib::NodeType::DISTRIBUTOR, getIndexFromConfig(configId), configId), - _compReg(dynamic_cast<DistributorComponentRegisterImpl&>( - TestStorageApp::getComponentRegister())), + _compReg(dynamic_cast<DistributorComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), _lastUniqueTimestampRequested(0), _uniqueTimestampCounter(0) { @@ -252,13 +240,11 @@ TestDistributorApp::TestDistributorApp(vespalib::stringref configId) configure(configId); } -TestDistributorApp::TestDistributorApp(NodeIndex index, - vespalib::stringref configId) +TestDistributorApp::TestDistributorApp(NodeIndex index, vespalib::stringref configId) : TestStorageApp( std::make_unique<DistributorComponentRegisterImpl>(), lib::NodeType::DISTRIBUTOR, index, configId), - _compReg(dynamic_cast<DistributorComponentRegisterImpl&>( - TestStorageApp::getComponentRegister())), + _compReg(dynamic_cast<DistributorComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), _lastUniqueTimestampRequested(0), _uniqueTimestampCounter(0) { @@ -266,12 +252,12 @@ TestDistributorApp::TestDistributorApp(NodeIndex index, configure(configId); } -TestDistributorApp::~TestDistributorApp() {} +TestDistributorApp::~TestDistributorApp() = default; api::Timestamp TestDistributorApp::getUniqueTimestamp() { - vespalib::Lock lock(_accessLock); + vespalib::LockGuard guard(_accessLock); uint64_t timeNow(getClock().getTimeInSeconds().getTime()); if (timeNow == _lastUniqueTimestampRequested) { ++_uniqueTimestampCounter; diff --git a/storage/src/vespa/storage/common/distributorcomponent.h b/storage/src/vespa/storage/common/distributorcomponent.h index e55e6f45cb1..27680b8c6ce 100644 --- a/storage/src/vespa/storage/common/distributorcomponent.h +++ b/storage/src/vespa/storage/common/distributorcomponent.h @@ -60,8 +60,7 @@ struct DistributorManagedComponent struct DistributorComponentRegister : public virtual StorageComponentRegister { - virtual void registerDistributorComponent( - DistributorManagedComponent&) = 0; + virtual void registerDistributorComponent(DistributorManagedComponent&) = 0; }; class DistributorComponent : public StorageComponent, @@ -86,10 +85,10 @@ public: typedef std::unique_ptr<DistributorComponent> UP; DistributorComponent(DistributorComponentRegister& compReg, vespalib::stringref name); - ~DistributorComponent(); + ~DistributorComponent() override; api::Timestamp getUniqueTimestamp() const { - assert(_timeCalculator); return _timeCalculator->getUniqueTimestamp(); + return _timeCalculator->getUniqueTimestamp(); } const DistributorConfig& getDistributorConfig() const { return _distributorConfig; diff --git a/storage/src/vespa/storage/common/servicelayercomponent.h b/storage/src/vespa/storage/common/servicelayercomponent.h index 22a33aac14c..2308480411e 100644 --- a/storage/src/vespa/storage/common/servicelayercomponent.h +++ b/storage/src/vespa/storage/common/servicelayercomponent.h @@ -79,11 +79,9 @@ public: const ContentBucketSpaceRepo &getBucketSpaceRepo() const; StorBucketDatabase& getBucketDatabase(document::BucketSpace bucketSpace) const; MinimumUsedBitsTracker& getMinUsedBitsTracker() { - assert(_minUsedBitsTracker != 0); return *_minUsedBitsTracker; } const MinimumUsedBitsTracker& getMinUsedBitsTracker() const { - assert(_minUsedBitsTracker != 0); return *_minUsedBitsTracker; } uint16_t getIdealPartition(const document::Bucket&) const; diff --git a/storage/src/vespa/storage/common/storagelinkqueued.hpp b/storage/src/vespa/storage/common/storagelinkqueued.hpp index 22c5e9ba5f2..ea6430f9e5c 100644 --- a/storage/src/vespa/storage/common/storagelinkqueued.hpp +++ b/storage/src/vespa/storage/common/storagelinkqueued.hpp @@ -8,6 +8,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <sstream> #include <chrono> +#include <cassert> namespace storage { diff --git a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp index 654fcbd1380..daafa3541c5 100644 --- a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp +++ b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp @@ -2,9 +2,9 @@ #include "storagecomponentregisterimpl.h" #include <vespa/vespalib/util/exceptions.h> +#include <cassert> #include <vespa/log/log.h> - LOG_SETUP(".storage.component.register"); namespace storage { diff --git a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h index 2f8bfaab87b..956397d7cf4 100644 --- a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h +++ b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h @@ -41,24 +41,20 @@ public: typedef std::unique_ptr<StorageComponentRegisterImpl> UP; StorageComponentRegisterImpl(); - ~StorageComponentRegisterImpl(); + ~StorageComponentRegisterImpl() override; const vespalib::string& getClusterName() const { return _clusterName; } - const lib::NodeType& getNodeType() const - { assert(_nodeType != 0); return *_nodeType; } + const lib::NodeType& getNodeType() const { return *_nodeType; } uint16_t getIndex() const { return _index; } std::shared_ptr<const document::DocumentTypeRepo> getTypeRepo() { return _docTypeRepo; } documentapi::LoadTypeSet::SP getLoadTypes() { return _loadTypes; } const document::BucketIdFactory& getBucketIdFactory() { return _bucketIdFactory; } lib::Distribution::SP getDistribution() { return _distribution; } - NodeStateUpdater& getNodeStateUpdater() - { assert(_nodeStateUpdater != 0); return *_nodeStateUpdater; } + NodeStateUpdater& getNodeStateUpdater() { return *_nodeStateUpdater; } void registerStorageComponent(StorageComponent&) override; - void setNodeInfo(vespalib::stringref clusterName, - const lib::NodeType& nodeType, - uint16_t index); + void setNodeInfo(vespalib::stringref clusterName, const lib::NodeType& nodeType, uint16_t index); virtual void setNodeStateUpdater(NodeStateUpdater& updater); virtual void setDocumentTypeRepo(std::shared_ptr<const document::DocumentTypeRepo>); virtual void setLoadTypes(documentapi::LoadTypeSet::SP); diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp index 4df2b5e591b..6878c0d0e82 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp @@ -76,7 +76,7 @@ void FileStorHandlerImpl::addMergeStatus(const document::Bucket& bucket, MergeStatus::SP status) { vespalib::LockGuard mlock(_mergeStatesLock); - if (_mergeStates.find(bucket) != _mergeStates.end()) {; + if (_mergeStates.find(bucket) != _mergeStates.end()) { LOG(warning, "A merge status already existed for %s. Overwriting it.", bucket.toString().c_str()); } _mergeStates[bucket] = status; @@ -802,7 +802,7 @@ FileStorHandlerImpl::Stripe::failOperations(const document::Bucket &bucket, cons { vespalib::MonitorGuard guard(_lock); - BucketIdx& idx(bmi::get<2>(_queue)); + BucketIdx& idx(bmi::get<2>(*_queue)); std::pair<BucketIdx::iterator, BucketIdx::iterator> range(idx.equal_range(bucket)); for (auto iter = range.first; iter != range.second;) { @@ -869,9 +869,13 @@ FileStorHandlerImpl::MessageEntry::~MessageEntry() = default; FileStorHandlerImpl::Disk::Disk(const FileStorHandlerImpl & owner, MessageSender & messageSender, uint32_t numStripes) : metrics(0), _nextStripeId(0), - _stripes(numStripes, Stripe(owner, messageSender)), + _stripes(), state(FileStorHandler::AVAILABLE) { + _stripes.reserve(numStripes); + for (size_t i(0); i < numStripes; i++) { + _stripes.emplace_back(owner, messageSender); + } assert(numStripes > 0); } @@ -884,6 +888,7 @@ FileStorHandlerImpl::Disk::Disk(Disk && rhs) noexcept FileStorHandlerImpl::Disk::~Disk() = default; FileStorHandlerImpl::Stripe::~Stripe() = default; +FileStorHandlerImpl::Stripe::Stripe(Stripe &&) noexcept = default; void FileStorHandlerImpl::Disk::flush() @@ -921,6 +926,10 @@ FileStorHandlerImpl::Disk::schedule(const std::shared_ptr<api::StorageMessage>& FileStorHandlerImpl::Stripe::Stripe(const FileStorHandlerImpl & owner, MessageSender & messageSender) : _owner(owner), _messageSender(messageSender), + _metrics(nullptr), + _lock(), + _queue(std::make_unique<PriorityQueue>()), + _lockedBuckets(), _active_merges(0) {} @@ -933,7 +942,7 @@ FileStorHandlerImpl::Stripe::getNextMessage(uint32_t timeout, Disk & disk) // second attempt. This is key to allowing the run loop to register // ticks at regular intervals while not busy-waiting. for (int attempt = 0; (attempt < 2) && ! disk.isClosed() && !_owner.isPaused(); ++attempt) { - PriorityIdx& idx(bmi::get<1>(_queue)); + PriorityIdx& idx(bmi::get<1>(*_queue)); PriorityIdx::iterator iter(idx.begin()), end(idx.end()); while (iter != end && operationIsInhibited(guard, iter->_bucket, *iter->_command)) { @@ -1032,11 +1041,11 @@ void FileStorHandlerImpl::Stripe::abort(std::vector<std::shared_ptr<api::Storage const AbortBucketOperationsCommand& cmd) { vespalib::MonitorGuard lockGuard(_lock); - for (auto it(_queue.begin()); it != _queue.end();) { + for (auto it(_queue->begin()); it != _queue->end();) { api::StorageMessage& msg(*it->_command); if (messageMayBeAborted(msg) && cmd.shouldAbort(it->_bucket)) { aborted.emplace_back(static_cast<api::StorageCommand&>(msg).makeReply()); - it = _queue.erase(it); + it = _queue->erase(it); } else { ++it; } @@ -1046,7 +1055,7 @@ void FileStorHandlerImpl::Stripe::abort(std::vector<std::shared_ptr<api::Storage bool FileStorHandlerImpl::Stripe::schedule(MessageEntry messageEntry) { vespalib::MonitorGuard lockGuard(_lock); - _queue.emplace_back(std::move(messageEntry)); + _queue->emplace_back(std::move(messageEntry)); lockGuard.broadcast(); return true; } @@ -1055,8 +1064,8 @@ void FileStorHandlerImpl::Stripe::flush() { vespalib::MonitorGuard lockGuard(_lock); - while (!(_queue.empty() && _lockedBuckets.empty())) { - LOG(debug, "Still %ld in queue and %ld locked buckets", _queue.size(), _lockedBuckets.size()); + while (!(_queue->empty() && _lockedBuckets.empty())) { + LOG(debug, "Still %ld in queue and %ld locked buckets", _queue->size(), _lockedBuckets.size()); lockGuard.wait(100); } } @@ -1229,7 +1238,7 @@ FileStorHandlerImpl::Stripe::dumpQueueHtml(std::ostream & os) const { vespalib::MonitorGuard guard(_lock); - const PriorityIdx& idx = bmi::get<1>(_queue); + const PriorityIdx& idx = bmi::get<1>(*_queue); for (const auto & entry : idx) { os << "<li>" << entry._command->toString() << " (priority: " << (int)entry._command->getPriority() << ")</li>\n"; @@ -1238,8 +1247,9 @@ FileStorHandlerImpl::Stripe::dumpQueueHtml(std::ostream & os) const namespace { -void dump_lock_entry(const document::BucketId& bucketId, const FileStorHandlerImpl::Stripe::LockEntry& entry, - api::LockingRequirements lock_mode, FileStorHandlerImpl::Clock::time_point now_ts, std::ostream& os) { +void +dump_lock_entry(const document::BucketId& bucketId, const FileStorHandlerImpl::Stripe::LockEntry& entry, + api::LockingRequirements lock_mode, FileStorHandlerImpl::Clock::time_point now_ts, std::ostream& os) { os << api::MessageType::get(entry.msgType).getName() << ":" << entry.msgId << " (" << bucketId << ", " << api::to_string(lock_mode) << " lock) Running for " << std::chrono::duration_cast<std::chrono::seconds>(now_ts - entry.timestamp).count() << " secs<br/>\n"; @@ -1269,7 +1279,7 @@ FileStorHandlerImpl::Stripe::dumpQueue(std::ostream & os) const { vespalib::MonitorGuard guard(_lock); - const PriorityIdx& idx = bmi::get<1>(_queue); + const PriorityIdx& idx = bmi::get<1>(*_queue); for (const auto & entry : idx) { os << entry._bucket.getBucketId() << ": " << entry._command->toString() << " (priority: " << (int)entry._command->getPriority() << ")\n"; diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h index a609473afd5..34cb7ba9266 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h @@ -84,9 +84,9 @@ public: api::MessageType::Id msgType; api::StorageMessage::Id msgId; - LockEntry() : timestamp(), priority(0), msgType(), msgId(0) { } + LockEntry() noexcept : timestamp(), priority(0), msgType(), msgId(0) { } - LockEntry(uint8_t priority_, api::MessageType::Id msgType_, api::StorageMessage::Id msgId_) + LockEntry(uint8_t priority_, api::MessageType::Id msgType_, api::StorageMessage::Id msgId_) noexcept : timestamp(Clock::now()), priority(priority_), msgType(msgType_), msgId(msgId_) { } }; @@ -98,6 +98,9 @@ public: }; Stripe(const FileStorHandlerImpl & owner, MessageSender & messageSender); + Stripe(Stripe &&) noexcept; + Stripe(const Stripe &) = delete; + Stripe & operator =(const Stripe &) = delete; ~Stripe(); void flush(); bool schedule(MessageEntry messageEntry); @@ -111,7 +114,7 @@ public: } size_t getQueueSize() const { vespalib::MonitorGuard guard(_lock); - return _queue.size(); + return _queue->size(); } void release(const document::Bucket & bucket, api::LockingRequirements reqOfReleasedLock, api::StorageMessage::Id lockMsgId); @@ -133,8 +136,8 @@ public: void dumpActiveHtml(std::ostream & os) const; void dumpQueueHtml(std::ostream & os) const; vespalib::Monitor & exposeLock() { return _lock; } - PriorityQueue & exposeQueue() { return _queue; } - BucketIdx & exposeBucketIdx() { return bmi::get<2>(_queue); } + PriorityQueue & exposeQueue() { return *_queue; } + BucketIdx & exposeBucketIdx() { return bmi::get<2>(*_queue); } void setMetrics(FileStorStripeMetrics * metrics) { _metrics = metrics; } private: bool hasActive(vespalib::MonitorGuard & monitor, const AbortBucketOperationsCommand& cmd) const; @@ -143,13 +146,13 @@ public: FileStorHandler::LockedMessage getMessage(vespalib::MonitorGuard & guard, PriorityIdx & idx, PriorityIdx::iterator iter); using LockedBuckets = vespalib::hash_map<document::Bucket, MultiLockEntry, document::Bucket::hash>; - const FileStorHandlerImpl &_owner; - MessageSender &_messageSender; - FileStorStripeMetrics *_metrics; - vespalib::Monitor _lock; - PriorityQueue _queue; - LockedBuckets _lockedBuckets; - uint32_t _active_merges; + const FileStorHandlerImpl &_owner; + MessageSender &_messageSender; + FileStorStripeMetrics *_metrics; + vespalib::Monitor _lock; + std::unique_ptr<PriorityQueue> _queue; + LockedBuckets _lockedBuckets; + uint32_t _active_merges; }; struct Disk { FileStorDiskMetrics * metrics; @@ -207,9 +210,9 @@ public: } std::vector<Stripe> & getStripes() { return _stripes; } private: - uint32_t _nextStripeId; - std::vector<Stripe> _stripes; - std::atomic<DiskState> state; + uint32_t _nextStripeId; + std::vector<Stripe> _stripes; + std::atomic<DiskState> state; }; class BucketLock : public FileStorHandler::BucketLockInterface { @@ -224,9 +227,9 @@ public: api::LockingRequirements lockingRequirements() const noexcept override { return _lockReq; } private: - Stripe & _stripe; - document::Bucket _bucket; - api::StorageMessage::Id _uniqueMsgId; + Stripe & _stripe; + document::Bucket _bucket; + api::StorageMessage::Id _uniqueMsgId; api::LockingRequirements _lockReq; }; diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.h b/storage/src/vespa/storage/storageserver/communicationmanager.h index 7ac9d575ee6..ce7906385d0 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.h +++ b/storage/src/vespa/storage/storageserver/communicationmanager.h @@ -137,7 +137,7 @@ public: void dispatch_sync(std::shared_ptr<api::StorageMessage> msg) override; void dispatch_async(std::shared_ptr<api::StorageMessage> msg) override; - mbus::RPCMessageBus& getMessageBus() { assert(_mbus.get()); return *_mbus; } + mbus::RPCMessageBus& getMessageBus() { return *_mbus; } const PriorityConverter& getPriorityConverter() const { return _docApiConverter.getPriorityConverter(); } /** diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp index 83e31b78954..dff2117eac9 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp +++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp @@ -9,6 +9,7 @@ #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/util/stringfmt.h> #include <algorithm> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".mergethrottler"); diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index dcd8cc4ba39..982bcd78f6f 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -13,6 +13,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <unordered_map> #include <sstream> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".visitor.instance.visitor"); diff --git a/storage/src/vespa/storage/visiting/visitormanager.cpp b/storage/src/vespa/storage/visiting/visitormanager.cpp index 1a1f1498578..c1cd52cf5e5 100644 --- a/storage/src/vespa/storage/visiting/visitormanager.cpp +++ b/storage/src/vespa/storage/visiting/visitormanager.cpp @@ -10,6 +10,7 @@ #include <vespa/config/common/exceptions.h> #include <vespa/documentapi/loadtypes/loadtypeset.h> #include <vespa/vespalib/util/stringfmt.h> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".visitor.manager"); diff --git a/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp b/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp index 2595514e6f4..ad2d0df5824 100644 --- a/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp +++ b/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp @@ -4,6 +4,7 @@ #include <vespa/storageframework/storageframework.h> #include <vespa/metrics/metricmanager.h> #include <vespa/vespalib/util/exceptions.h> +#include <cassert> namespace storage::framework::defaultimplementation { diff --git a/storageframework/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.cpp b/storageframework/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.cpp index 2e90e1ae3ee..5c8e70f2773 100644 --- a/storageframework/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.cpp +++ b/storageframework/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "testcomponentregister.h" +#include <cassert> namespace storage::framework::defaultimplementation { diff --git a/storageframework/src/vespa/storageframework/generic/component/component.cpp b/storageframework/src/vespa/storageframework/generic/component/component.cpp index b0569bdee41..3818a2865d9 100644 --- a/storageframework/src/vespa/storageframework/generic/component/component.cpp +++ b/storageframework/src/vespa/storageframework/generic/component/component.cpp @@ -4,6 +4,7 @@ #include "componentregister.h" #include <vespa/storageframework/generic/metric/metricregistrator.h> #include <vespa/storageframework/generic/thread/threadpool.h> +#include <cassert> namespace storage::framework { diff --git a/storageframework/src/vespa/storageframework/generic/component/component.h b/storageframework/src/vespa/storageframework/generic/component/component.h index 165d9c47d6e..c91d7feb532 100644 --- a/storageframework/src/vespa/storageframework/generic/component/component.h +++ b/storageframework/src/vespa/storageframework/generic/component/component.h @@ -110,7 +110,6 @@ class Component : private ManagedComponent void setClock(Clock& c) override { _clock = &c; } void setThreadPool(ThreadPool& tp) override { _threadPool = &tp; } void setUpgradeFlag(UpgradeFlags flag) override { - assert(_upgradeFlag.is_lock_free()); _upgradeFlag.store(flag, std::memory_order_relaxed); } void open() override; diff --git a/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp b/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp index dc4f8bbc1f1..c56a6fbb750 100644 --- a/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp +++ b/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp @@ -1,8 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "tickingthread.h" #include "threadpool.h" -#include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <cassert> namespace storage::framework { diff --git a/vdslib/src/vespa/vdslib/bucketdistribution.cpp b/vdslib/src/vespa/vdslib/bucketdistribution.cpp index a8c39ad6577..15947d82fed 100644 --- a/vdslib/src/vespa/vdslib/bucketdistribution.cpp +++ b/vdslib/src/vespa/vdslib/bucketdistribution.cpp @@ -17,10 +17,7 @@ BucketDistribution::BucketDistribution(uint32_t numColumns, uint32_t numBucketBi setNumColumns(numColumns); } -BucketDistribution::BucketDistribution(const BucketDistribution &) = default; -BucketDistribution & BucketDistribution::operator = (const BucketDistribution &) = default; - -BucketDistribution::~BucketDistribution() {} +BucketDistribution::~BucketDistribution() = default; void BucketDistribution::getBucketCount(uint32_t numColumns, uint32_t numBucketBits, std::vector<uint32_t> &ret) @@ -47,9 +44,8 @@ BucketDistribution::getBucketMigrateCount(uint32_t numColumns, uint32_t numBucke void BucketDistribution::reset() { - for (std::vector<uint32_t>::iterator it = _bucketToColumn.begin(); - it != _bucketToColumn.end(); ++it) { - *it = 0; + for (uint32_t & value : _bucketToColumn) { + value = 0; } _numColumns = 1; } diff --git a/vdslib/src/vespa/vdslib/bucketdistribution.h b/vdslib/src/vespa/vdslib/bucketdistribution.h index c9e584af7ef..5d06be53b49 100644 --- a/vdslib/src/vespa/vdslib/bucketdistribution.h +++ b/vdslib/src/vespa/vdslib/bucketdistribution.h @@ -21,10 +21,6 @@ public: * @param numBucketBits The number of bits to use for bucket id. */ BucketDistribution(uint32_t numColumns, uint32_t numBucketBits); - BucketDistribution(const BucketDistribution &); - BucketDistribution & operator = (const BucketDistribution &); - BucketDistribution(BucketDistribution &&) = default; - BucketDistribution & operator = (BucketDistribution &&) = default; ~BucketDistribution(); /** diff --git a/vdslib/src/vespa/vdslib/distribution/distribution.cpp b/vdslib/src/vespa/vdslib/distribution/distribution.cpp index 0a154f3dec1..474ed63e8c3 100644 --- a/vdslib/src/vespa/vdslib/distribution/distribution.cpp +++ b/vdslib/src/vespa/vdslib/distribution/distribution.cpp @@ -14,6 +14,7 @@ #include <list> #include <algorithm> #include <cmath> +#include <cassert> #include <vespa/log/bufferedlogger.h> LOG_SETUP(".vdslib.distribution"); diff --git a/vespalib/src/tests/sync/sync_test.cpp b/vespalib/src/tests/sync/sync_test.cpp index a17f9089877..c8888bc8e54 100644 --- a/vespalib/src/tests/sync/sync_test.cpp +++ b/vespalib/src/tests/sync/sync_test.cpp @@ -119,17 +119,7 @@ Test::Main() CHECK_UNLOCKED(monitor); } } - // copy/assign is nop, but legal - { - Lock a; - Lock b(a); - b = a; - } - { - Monitor a; - Monitor b(a); - b = a; - } + // you can lock const objects { const Lock lock; diff --git a/vespalib/src/vespa/vespalib/data/slime/json_format.cpp b/vespalib/src/vespa/vespalib/data/slime/json_format.cpp index d2f953b38c8..080e3b8da48 100644 --- a/vespalib/src/vespa/vespalib/data/slime/json_format.cpp +++ b/vespalib/src/vespa/vespalib/data/slime/json_format.cpp @@ -8,6 +8,7 @@ #include <vespa/vespalib/locale/c.h> #include <cmath> #include <sstream> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".vespalib.data.slime.json_format"); diff --git a/vespalib/src/vespa/vespalib/testkit/test_hook.h b/vespalib/src/vespa/vespalib/testkit/test_hook.h index 28419c0b31b..706a1c9f741 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_hook.h +++ b/vespalib/src/vespa/vespalib/testkit/test_hook.h @@ -6,6 +6,7 @@ #include <vespa/vespalib/util/barrier.h> #include <string> #include <vector> +#include <cassert> #include "test_master.h" namespace vespalib { diff --git a/vespalib/src/vespa/vespalib/testkit/test_master.cpp b/vespalib/src/vespa/vespalib/testkit/test_master.cpp index 789d40d478d..4b673d0ee0d 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_master.cpp +++ b/vespalib/src/vespa/vespalib/testkit/test_master.cpp @@ -3,6 +3,7 @@ #include "test_master.h" #include <vespa/vespalib/util/barrier.h> #include <cstring> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".vespalib.testkit.test_master"); @@ -33,7 +34,7 @@ __thread TestMaster::ThreadState *TestMaster::_threadState = 0; //----------------------------------------------------------------------------- -TestMaster::TraceItem::~TraceItem() { } +TestMaster::TraceItem::~TraceItem() = default; TestMaster::ThreadState & TestMaster::threadState(const vespalib::LockGuard &) diff --git a/vespalib/src/vespa/vespalib/util/CMakeLists.txt b/vespalib/src/vespa/vespalib/util/CMakeLists.txt index 0973653861f..fd645f9008c 100644 --- a/vespalib/src/vespa/vespalib/util/CMakeLists.txt +++ b/vespalib/src/vespa/vespalib/util/CMakeLists.txt @@ -50,6 +50,7 @@ vespa_add_library(vespalib_vespalib_util OBJECT stash.cpp string_hash.cpp stringfmt.cpp + sync.cpp thread.cpp thread_bundle.cpp threadstackexecutor.cpp diff --git a/vespalib/src/vespa/vespalib/util/alloc.cpp b/vespalib/src/vespa/vespalib/util/alloc.cpp index 1303e7ffdee..5ae499f5482 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.cpp +++ b/vespalib/src/vespa/vespalib/util/alloc.cpp @@ -8,6 +8,7 @@ #include <map> #include <atomic> #include <unordered_map> +#include <cassert> #include <vespa/fastos/file.h> #include <unistd.h> diff --git a/vespalib/src/vespa/vespalib/util/child_process.cpp b/vespalib/src/vespa/vespalib/util/child_process.cpp index ce0c2eb1779..b804d4ca87c 100644 --- a/vespalib/src/vespa/vespalib/util/child_process.cpp +++ b/vespalib/src/vespa/vespalib/util/child_process.cpp @@ -56,9 +56,6 @@ public: } // namespace child_process -using child_process::Timer; - -//----------------------------------------------------------------------------- void ChildProcess::Reader::OnReceiveData(const void *data, size_t length) @@ -88,7 +85,7 @@ ChildProcess::Reader::hasData() bool -ChildProcess::Reader::waitForData(Timer &timer, MonitorGuard &lock) +ChildProcess::Reader::waitForData(child_process::Timer &timer, MonitorGuard &lock) { // NB: caller has lock on _cond CounterGuard count(_waitCnt); @@ -131,7 +128,7 @@ ChildProcess::Reader::read(char *buf, uint32_t len, int msTimeout) if (eof()) { return 0; } - Timer timer(msTimeout); + child_process::Timer timer(msTimeout); MonitorGuard lock(_cond); waitForData(timer, lock); uint32_t bytes = 0; @@ -162,7 +159,7 @@ ChildProcess::Reader::readLine(std::string &line, int msTimeout) if (eof()) { return false; } - Timer timer(msTimeout); + child_process::Timer timer(msTimeout); MonitorGuard lock(_cond); while (waitForData(timer, lock)) { while (hasData()) { @@ -299,7 +296,7 @@ ChildProcess::run(const std::string &input, const char *cmd, std::string &output, int msTimeout) { ChildProcess proc(cmd); - Timer timer(msTimeout); + child_process::Timer timer(msTimeout); char buf[4096]; proc.write(input.data(), input.length()); proc.close(); // close stdin diff --git a/vespalib/src/vespa/vespalib/util/rwlock.cpp b/vespalib/src/vespa/vespalib/util/rwlock.cpp index d1e155851cf..fc3799bc1d0 100644 --- a/vespalib/src/vespa/vespalib/util/rwlock.cpp +++ b/vespalib/src/vespa/vespalib/util/rwlock.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/util/rwlock.h> +#include "rwlock.h" +#include <cassert> namespace vespalib { @@ -39,4 +40,20 @@ void RWLock::unlockWrite() { } } +RWLock * +RWLockReader::stealLock() { + RWLock * ret(_lock); + assert(ret != nullptr); + _lock = nullptr; + return ret; +} + +RWLock * +RWLockWriter::stealLock() { + RWLock * ret(_lock); + assert(ret != nullptr); + _lock = nullptr; + return ret; +} + } // namespace vespalib diff --git a/vespalib/src/vespa/vespalib/util/rwlock.h b/vespalib/src/vespa/vespalib/util/rwlock.h index 9b1f9b93289..8818291ae6c 100644 --- a/vespalib/src/vespa/vespalib/util/rwlock.h +++ b/vespalib/src/vespa/vespalib/util/rwlock.h @@ -119,13 +119,8 @@ class RWLockReader { private: RWLock * _lock; - RWLock * stealLock() { - RWLock * ret(_lock); - assert(ret != NULL); - _lock = NULL; - return ret; - } - void cleanup() { if (_lock != NULL) { _lock->unlockRead(); } } + RWLock * stealLock(); + void cleanup() { if (_lock != nullptr) { _lock->unlockRead(); } } public: /** @@ -141,7 +136,7 @@ public: * @brief Construct initially unlocked guard. * @param tag (unused) marker argument **/ - RWLockReader(const RWLock::InitiallyUnlockedGuard &tag) : _lock(NULL) { (void)tag; } + RWLockReader(const RWLock::InitiallyUnlockedGuard &tag) : _lock(nullptr) { (void)tag; } /** * @brief Steal the lock from the given RWLockReader @@ -191,13 +186,8 @@ class RWLockWriter { private: RWLock * _lock; - RWLock * stealLock() { - RWLock * ret(_lock); - assert(ret != NULL); - _lock = NULL; - return ret; - } - void cleanup() { if (_lock != NULL) { _lock->unlockWrite(); } } + RWLock * stealLock(); + void cleanup() { if (_lock != nullptr) { _lock->unlockWrite(); } } public: /** @@ -213,7 +203,7 @@ public: * @brief Construct initially unlocked guard. * @param tag (unused) marker argument **/ - RWLockWriter(const RWLock::InitiallyUnlockedGuard &tag) : _lock(NULL) { (void)tag; } + RWLockWriter(const RWLock::InitiallyUnlockedGuard &tag) : _lock(nullptr) { (void)tag; } /** * @brief Steal the lock from the given RWLockWriter diff --git a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp index 01979c25164..886003a2ab6 100644 --- a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp +++ b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp @@ -2,6 +2,7 @@ #include "simple_thread_bundle.h" #include "exceptions.h" +#include <cassert> using namespace vespalib::fixed_thread_bundle; @@ -75,7 +76,7 @@ SimpleThreadBundle::Pool::obtain() return ret; } } - return SimpleThreadBundle::UP(new SimpleThreadBundle(_bundleSize)); + return std::make_unique<SimpleThreadBundle>(_bundleSize); } void diff --git a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h index 4fa73c1112f..dbf9f7025b6 100644 --- a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h +++ b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h @@ -48,7 +48,7 @@ struct Signal { bool valid; size_t generation; Monitor monitor; - Signal() : valid(true), generation(0), monitor() {} + Signal() noexcept : valid(true), generation(0), monitor() {} size_t wait(size_t &localGen) { MonitorGuard guard(monitor); while (localGen == generation) { diff --git a/vespalib/src/vespa/vespalib/util/sync.cpp b/vespalib/src/vespa/vespalib/util/sync.cpp new file mode 100644 index 00000000000..3819b2b3369 --- /dev/null +++ b/vespalib/src/vespa/vespalib/util/sync.cpp @@ -0,0 +1,153 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "sync.h" +#include <cassert> + +namespace vespalib { + +Lock::Lock() noexcept + : _mutex(std::make_unique<std::mutex>()) +{} +Lock::Lock(Lock &&rhs) noexcept = default; +Lock::~Lock() = default; + + +Monitor::Monitor() noexcept + : Lock(), + _cond(std::make_unique<std::condition_variable>()) +{} +Monitor::Monitor(Monitor &&rhs) noexcept = default; +Monitor::~Monitor() = default; + + +TryLock::TryLock(const Lock &lock) + : _guard(*lock._mutex, std::try_to_lock), + _cond(nullptr) +{} + +TryLock::TryLock(const Monitor &mon) + : _guard(*mon._mutex, std::try_to_lock), + _cond(_guard ? mon._cond.get() : nullptr) +{} + +TryLock::TryLock(TryLock &&rhs) noexcept + : _guard(std::move(rhs._guard)), + _cond(rhs._cond) +{ + rhs._cond = nullptr; +} + +TryLock::~TryLock() = default; + +bool +TryLock::hasLock() const { + return static_cast<bool>(_guard); +} + +void +TryLock::unlock() { + if (_guard) { + _guard.unlock(); + _cond = nullptr; + } +} + +LockGuard::LockGuard() : _guard() {} + +LockGuard::LockGuard(LockGuard &&rhs) noexcept : _guard(std::move(rhs._guard)) { } +LockGuard::LockGuard(const Lock &lock) : _guard(*lock._mutex) { } +LockGuard::LockGuard(TryLock &&tlock) : _guard(std::move(tlock._guard)) +{ + tlock._cond = nullptr; +} + +LockGuard & +LockGuard::operator=(LockGuard &&rhs) noexcept{ + if (this != &rhs) { + _guard = std::move(rhs._guard); + } + return *this; +} + +void +LockGuard::unlock() { + if (_guard) { + _guard.unlock(); + } +} +LockGuard::~LockGuard() = default; + +bool +LockGuard::locks(const Lock& lock) const { + return (_guard && _guard.mutex() == lock._mutex.get()); +} + +MonitorGuard::MonitorGuard() : _guard(), _cond(nullptr) {} +MonitorGuard::MonitorGuard(MonitorGuard &&rhs) noexcept + : _guard(std::move(rhs._guard)), + _cond(rhs._cond) +{ + rhs._cond = nullptr; +} +MonitorGuard::MonitorGuard(const Monitor &monitor) + : _guard(*monitor._mutex), + _cond(monitor._cond.get()) +{ } +MonitorGuard::MonitorGuard(TryLock &&tlock) + : _guard(), + _cond(nullptr) +{ + if (tlock._guard && tlock._cond != nullptr) { + _guard = std::move(tlock._guard); + _cond = tlock._cond; + tlock._cond = nullptr; + } +} + +MonitorGuard & +MonitorGuard::operator=(MonitorGuard &&rhs) noexcept { + if (this != &rhs) { + _guard = std::move(rhs._guard); + _cond = rhs._cond; + rhs._cond = nullptr; + } + return *this; +} + +void +MonitorGuard::unlock() { + assert(_guard); + _guard.unlock(); + _cond = nullptr; +} +void +MonitorGuard::wait() { + _cond->wait(_guard); +} +bool +MonitorGuard::wait(int msTimeout) { + return wait(std::chrono::milliseconds(msTimeout)); +} +bool +MonitorGuard::wait(std::chrono::nanoseconds timeout) { + return _cond->wait_for(_guard, timeout) == std::cv_status::no_timeout; +} +void +MonitorGuard::signal() { + _cond->notify_one(); +} +void +MonitorGuard::broadcast() { + _cond->notify_all(); +} +void +MonitorGuard::unsafeSignalUnlock() { + _guard.unlock(); + _cond->notify_one(); + _cond = nullptr; +} + +MonitorGuard::~MonitorGuard() = default; + +} // namespace vespalib + diff --git a/vespalib/src/vespa/vespalib/util/sync.h b/vespalib/src/vespa/vespalib/util/sync.h index 043bb477c89..b1227ce658e 100644 --- a/vespalib/src/vespa/vespalib/util/sync.h +++ b/vespalib/src/vespa/vespalib/util/sync.h @@ -2,10 +2,9 @@ #pragma once -#include <cassert> +#include "time.h" #include <mutex> #include <condition_variable> -#include <chrono> namespace vespalib { @@ -25,19 +24,16 @@ protected: friend class LockGuard; friend class TryLock; - mutable std::mutex _mutex; + std::unique_ptr<std::mutex> _mutex; public: /** * @brief Create a new Lock. * * Creates a Lock that has mutex instrumentation disabled. **/ - Lock() noexcept : _mutex() {} - //TODO Remove The below methods are very bad and have a dodgy history. - Lock(const Lock &) : Lock() { } - Lock(Lock &&) noexcept : Lock() { } - Lock &operator=(const Lock &) { return *this; } - Lock &operator=(Lock &&) { return *this; } + Lock() noexcept; + Lock(Lock && rhs) noexcept; + ~Lock(); }; @@ -60,19 +56,16 @@ private: friend class MonitorGuard; friend class TryLock; - mutable std::condition_variable _cond; + std::unique_ptr<std::condition_variable> _cond; public: /** * @brief Create a new Monitor. * * Creates a Monitor that has mutex instrumentation disabled. **/ - Monitor() noexcept : Lock(), _cond() {} - //TODO Remove The below methods are very bad and have a dodgy history. - Monitor(const Monitor &) : Monitor() { } - Monitor(Monitor &&) noexcept : Monitor() { } - Monitor &operator=(const Monitor &) { return *this; } - Monitor &operator=(Monitor &&) { return *this; } + Monitor() noexcept; + Monitor(Monitor && rhs) noexcept; + ~Monitor(); }; @@ -114,59 +107,39 @@ private: std::unique_lock<std::mutex> _guard; std::condition_variable *_cond; - TryLock(const TryLock &) = delete; - TryLock &operator=(const TryLock &) = delete; - public: /** * @brief Try to obtain the lock represented by the given Lock object * * @param lock the lock to obtain **/ - TryLock(const Lock &lock) - : _guard(lock._mutex, std::try_to_lock), - _cond(nullptr) - { - } + TryLock(const Lock &lock); /** * @brief Try to lock the given Monitor * * @param mon the monitor to lock **/ - TryLock(const Monitor &mon) - : _guard(mon._mutex, std::try_to_lock), - _cond(_guard ? &mon._cond : nullptr) - { - } + TryLock(const Monitor &mon); - TryLock(TryLock &&rhs) - : _guard(std::move(rhs._guard)), - _cond(rhs._cond) - { - rhs._cond = nullptr; - } + TryLock(TryLock &&rhs) noexcept; /** * @brief Release the lock held by this object, if any **/ - ~TryLock() = default; - - TryLock &operator=(TryLock &&rhs) { - if (this != &rhs) { - _guard = std::move(rhs._guard); - _cond = rhs._cond; - rhs._cond = nullptr; - } - return *this; - } + ~TryLock(); + + TryLock(const TryLock &) = delete; + TryLock &operator=(const TryLock &) = delete; + + TryLock &operator=(TryLock &&rhs) noexcept; /** * @brief Check whether this object holds a lock * * @return true if this object holds a lock **/ - bool hasLock() { return static_cast<bool>(_guard); } + bool hasLock() const; /** * @brief Release the lock held by this object. * @@ -175,12 +148,7 @@ public: * need to release the lock before the object is destructed, as * the destructor will release the lock. **/ - void unlock() { - if (_guard) { - _guard.unlock(); - _cond = nullptr; - } - } + void unlock(); }; @@ -200,19 +168,20 @@ class LockGuard { private: std::unique_lock<std::mutex> _guard; - LockGuard &operator=(const LockGuard &) = delete; public: /** * @brief A noop guard without any mutex. **/ - LockGuard() : _guard() {} + LockGuard(); LockGuard(const LockGuard &rhs) = delete; + LockGuard &operator=(const LockGuard &) = delete; + /** * @brief Steal the lock from the given LockGuard * * @param rhs steal the lock from this one **/ - LockGuard(LockGuard &&rhs) : _guard(std::move(rhs._guard)) { } + LockGuard(LockGuard &&rhs) noexcept; /** * @brief Obtain the lock represented by the given Lock object. * @@ -220,7 +189,7 @@ public: * * @param lock take it **/ - LockGuard(const Lock &lock) : _guard(lock._mutex) { } + LockGuard(const Lock &lock); /** * @brief Create a LockGuard from a TryLock. @@ -231,17 +200,9 @@ public: * * @param tlock take the lock from this one **/ - LockGuard(TryLock &&tlock) : _guard(std::move(tlock._guard)) - { - tlock._cond = nullptr; - } + LockGuard(TryLock &&tlock); - LockGuard &operator=(LockGuard &&rhs) { - if (this != &rhs) { - _guard = std::move(rhs._guard); - } - return *this; - } + LockGuard &operator=(LockGuard &&rhs) noexcept; /** * @brief Release the lock held by this object. @@ -251,24 +212,18 @@ public: * need to release the lock before the object is destructed, as * the destructor will release the lock. **/ - void unlock() { - if (_guard) { - _guard.unlock(); - } - } + void unlock(); /** * @brief Release the lock held by this object if unlock has not * been called. **/ - ~LockGuard() = default; + ~LockGuard(); /** * Allow code to match guard with lock. This allows functions to take a * guard ref as input, ensuring that the caller have grabbed a lock. */ - bool locks(const Lock& lock) const { - return (_guard && _guard.mutex() == &lock._mutex); - } + bool locks(const Lock& lock) const; }; @@ -296,19 +251,14 @@ public: /** * @brief A noop guard without any condition. **/ - MonitorGuard() : _guard(), _cond(nullptr) {} + MonitorGuard(); MonitorGuard(const MonitorGuard &rhs) = delete; /** * @brief Steal the lock from the given MonitorGuard * * @param rhs steal the lock from this one **/ - MonitorGuard(MonitorGuard &&rhs) - : _guard(std::move(rhs._guard)), - _cond(rhs._cond) - { - rhs._cond = nullptr; - } + MonitorGuard(MonitorGuard &&rhs) noexcept; /** * @brief Obtain the lock on the given Monitor object. * @@ -316,11 +266,7 @@ public: * * @param monitor take the lock on it **/ - MonitorGuard(const Monitor &monitor) - : _guard(monitor._mutex), - _cond(&monitor._cond) - { - } + MonitorGuard(const Monitor &monitor); /** * @brief Create a MonitorGuard from a TryLock. * @@ -330,25 +276,9 @@ public: * * @param tlock take the lock from this one **/ - MonitorGuard(TryLock &&tlock) - : _guard(), - _cond(nullptr) - { - if (tlock._guard && tlock._cond != nullptr) { - _guard = std::move(tlock._guard); - _cond = tlock._cond; - tlock._cond = nullptr; - } - } + MonitorGuard(TryLock &&tlock); - MonitorGuard &operator=(MonitorGuard &&rhs) { - if (this != &rhs) { - _guard = std::move(rhs._guard); - _cond = rhs._cond; - rhs._cond = nullptr; - } - return *this; - } + MonitorGuard &operator=(MonitorGuard &&rhs) noexcept; /** @@ -359,17 +289,11 @@ public: * need to release the lock before this object is destructed, as * the destructor will release the lock. **/ - void unlock() { - assert(_guard); - _guard.unlock(); - _cond = nullptr; - } + void unlock(); /** * @brief Wait for a signal on the underlying Monitor. **/ - void wait() { - _cond->wait(_guard); - } + void wait(); /** * @brief Wait for a signal on the underlying Monitor with the * given timeout. @@ -377,25 +301,17 @@ public: * @param msTimeout timeout in milliseconds * @return true if a signal was received, false if the wait timed out. **/ - bool wait(int msTimeout) { - return wait(std::chrono::milliseconds(msTimeout)); - } - bool wait(std::chrono::nanoseconds timeout) { - return _cond->wait_for(_guard, timeout) == std::cv_status::no_timeout; - } + bool wait(int msTimeout); + bool wait(duration timeout); /** * @brief Send a signal to a single waiter on the underlying * Monitor. **/ - void signal() { - _cond->notify_one(); - } + void signal(); /** * @brief Send a signal to all waiters on the underlying Monitor. **/ - void broadcast() { - _cond->notify_all(); - } + void broadcast(); /** * @brief Send a signal to a single waiter on the underlying * Monitor, but unlock the monitor right before doing so. @@ -404,24 +320,20 @@ public: * synchronization to ensure that the underlying Monitor object * will live long enough to be signaled. **/ - void unsafeSignalUnlock() { - _guard.unlock(); - _cond->notify_one(); - _cond = nullptr; - } + void unsafeSignalUnlock(); /** * @brief Release the lock held by this object if unlock has not * been called. **/ - ~MonitorGuard() = default; + ~MonitorGuard(); /** * Allow code to match guard with lock. This allows functions to take a * guard ref as input, ensuring that the caller have grabbed a lock. */ bool monitors(const Monitor& m) const { - return (_cond != nullptr && _cond == &m._cond); + return (_cond != nullptr && _cond == m._cond.get()); } }; diff --git a/vespalib/src/vespa/vespalib/util/thread.cpp b/vespalib/src/vespa/vespalib/util/thread.cpp index 4eb436458a2..c595a76b8fe 100644 --- a/vespalib/src/vespa/vespalib/util/thread.cpp +++ b/vespalib/src/vespa/vespalib/util/thread.cpp @@ -2,6 +2,7 @@ #include "thread.h" #include <thread> +#include <cassert> namespace vespalib { |