From 3d20dde708032ac70ab4cb24fa8284686a0001f3 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Sat, 30 Jan 2021 21:57:04 +0000 Subject: Various code health. --- .../proton/attribute/attributemanager.cpp | 2 +- .../documentmetastoreflushtarget.cpp | 18 +++------- .../documentmetastore/documentmetastoresaver.cpp | 2 +- .../documentmetastore/documentmetastoresaver.h | 4 +-- .../proton/flushengine/cachedflushtarget.h | 22 ++++++------ .../proton/flushengine/flush_engine_explorer.cpp | 2 +- .../proton/flushengine/flush_engine_explorer.h | 3 +- .../searchcore/proton/flushengine/flushcontext.cpp | 17 ++++------ .../searchcore/proton/flushengine/flushengine.cpp | 3 +- .../proton/flushengine/flushtargetproxy.h | 39 ++++++---------------- .../searchcore/proton/flushengine/flushtask.cpp | 7 ++-- .../searchcore/proton/flushengine/flushtask.h | 12 ++++--- .../searchcore/proton/flushengine/iflushhandler.h | 3 +- .../searchcore/proton/flushengine/iflushstrategy.h | 14 +++----- .../proton/flushengine/threadedflushtarget.cpp | 8 ++--- .../proton/flushengine/threadedflushtarget.h | 6 ++-- .../src/vespa/searchlib/attribute/attributesaver.h | 5 ++- 17 files changed, 66 insertions(+), 101 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index cc1af923fc0..0b00e274522 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -328,7 +328,7 @@ AttributeManager::flushAll(SerialNum currentSerial) for (const auto &ft : flushTargets) { vespalib::Executor::Task::UP task; task = ft->initFlush(currentSerial, std::make_shared()); - if (task.get() != NULL) { + if (task) { task->run(); } } diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp index b563cee1d0c..5e6a9f354f9 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp @@ -39,9 +39,7 @@ private: bool saveDocumentMetaStore(); // not updating snap info. public: Flusher(DocumentMetaStoreFlushTarget &dmsft, uint64_t syncToken, AttributeDirectory::Writer &writer); - ~Flusher(); - uint64_t getSyncToken() const { return _syncToken; } - bool saveSnapInfo(); + ~Flusher() override; bool flush(AttributeDirectory::Writer &writer); void updateStats(); bool cleanUp(AttributeDirectory::Writer &writer); @@ -66,29 +64,23 @@ Flusher(DocumentMetaStoreFlushTarget &dmsft, assert(_saver); } -DocumentMetaStoreFlushTarget::Flusher::~Flusher() -{ - // empty -} +DocumentMetaStoreFlushTarget::Flusher::~Flusher() = default; bool DocumentMetaStoreFlushTarget::Flusher::saveDocumentMetaStore() { vespalib::mkdir(_flushDir, false); - SerialNumFileHeaderContext fileHeaderContext(_dmsft._fileHeaderContext, - _syncToken); + SerialNumFileHeaderContext fileHeaderContext(_dmsft._fileHeaderContext, _syncToken); bool saveSuccess = false; if (_dmsft._hwInfo.disk().slow()) { search::AttributeMemorySaveTarget memorySaveTarget; saveSuccess = _saver->save(memorySaveTarget); _saver.reset(); if (saveSuccess) { - saveSuccess = memorySaveTarget.writeToFile(_dmsft._tuneFileAttributes, - fileHeaderContext); + saveSuccess = memorySaveTarget.writeToFile(_dmsft._tuneFileAttributes, fileHeaderContext); } } else { - search::AttributeFileSaveTarget saveTarget(_dmsft._tuneFileAttributes, - fileHeaderContext); + search::AttributeFileSaveTarget saveTarget(_dmsft._tuneFileAttributes, fileHeaderContext); saveSuccess = _saver->save(saveTarget); _saver.reset(); } diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp index 94bba1bf64f..9a4054d02ca 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp @@ -83,7 +83,7 @@ DocumentMetaStoreSaver(vespalib::GenerationHandler::Guard &&guard, } -DocumentMetaStoreSaver::~DocumentMetaStoreSaver() { } +DocumentMetaStoreSaver::~DocumentMetaStoreSaver() = default; bool diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h index 2f745929667..a11f60af4b6 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h @@ -32,14 +32,14 @@ private: const MetaDataStore &_metaDataStore; bool _writeDocSize; - virtual bool onSave(search::IAttributeSaveTarget &saveTarget) override; + bool onSave(search::IAttributeSaveTarget &saveTarget) override; public: DocumentMetaStoreSaver(vespalib::GenerationHandler::Guard &&guard, const search::attribute::AttributeHeader &header, const GidIterator &gidIterator, const MetaDataStore &metaDataStore); - virtual ~DocumentMetaStoreSaver(); + ~DocumentMetaStoreSaver() override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h b/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h index ecd66f9cbfb..f2b3514fcf9 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h @@ -42,16 +42,18 @@ public: const IFlushTarget::SP & getFlushTarget() { return _target; } // Implements IFlushTarget. - virtual MemoryGain getApproxMemoryGain() const override { return _memoryGain; } - virtual DiskGain getApproxDiskGain() const override { return _diskGain; } - virtual SerialNum getFlushedSerialNum() const override { return _flushedSerialNum; } - virtual Time getLastFlushTime() const override { return _lastFlushTime; } - virtual bool needUrgentFlush() const override { return _needUrgentFlush; } - - virtual Task::UP initFlush(SerialNum currentSerial, std::shared_ptr flush_token) override { return _target->initFlush(currentSerial, std::move(flush_token)); } - virtual FlushStats getLastFlushStats() const override { return _target->getLastFlushStats(); } - - virtual uint64_t getApproxBytesToWriteToDisk() const override; + MemoryGain getApproxMemoryGain() const override { return _memoryGain; } + DiskGain getApproxDiskGain() const override { return _diskGain; } + SerialNum getFlushedSerialNum() const override { return _flushedSerialNum; } + Time getLastFlushTime() const override { return _lastFlushTime; } + bool needUrgentFlush() const override { return _needUrgentFlush; } + + Task::UP initFlush(SerialNum currentSerial, std::shared_ptr flush_token) override { + return _target->initFlush(currentSerial, std::move(flush_token)); + } + FlushStats getLastFlushStats() const override { return _target->getLastFlushStats(); } + + uint64_t getApproxBytesToWriteToDisk() const override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp index 2a447957334..1f5ad8ae3a3 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp @@ -1,7 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "flush_engine_explorer.h" - +#include "flushengine.h" #include #include diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h index a674b39bc26..c2a90597ca2 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h @@ -2,11 +2,12 @@ #pragma once -#include "flushengine.h" #include namespace proton { +class FlushEngine; + /** * Class used to explore the state of a flush engine and its flush targets. */ diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp index b569e499876..ed31317cb69 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp @@ -16,20 +16,17 @@ FlushContext::FlushContext( _target(target), _task(), _lastSerial(lastSerial) -{ - // empty -} +{ } -vespalib::string FlushContext::createName(const IFlushHandler & handler, const IFlushTarget & target) -{ +vespalib::string +FlushContext::createName(const IFlushHandler & handler, const IFlushTarget & target) { return (handler.getName() + "." + target.getName()); } FlushContext::~FlushContext() { - if (_task.get() != NULL) { - LOG(warning, "Unexecuted flush task for '%s' destroyed.", - _name.c_str()); + if (_task) { + LOG(warning, "Unexecuted flush task for '%s' destroyed.", _name.c_str()); } } @@ -38,11 +35,11 @@ FlushContext::initFlush(std::shared_ptr flush_token) { LOG(debug, "Attempting to flush '%s'.", _name.c_str()); _task = _target->initFlush(std::max(_handler->getCurrentSerialNumber(), _lastSerial), std::move(flush_token)); - if (_task.get() == NULL) { + if ( ! _task ) { LOG(debug, "Target refused to init flush."); return false; } return true; } -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp index ab5b1ac5937..42b38bec821 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp @@ -220,7 +220,8 @@ FlushEngine::prune() return true; } -bool FlushEngine::isFlushing(const std::lock_guard & guard, const vespalib::string & name) const +bool +FlushEngine::isFlushing(const std::lock_guard & guard, const vespalib::string & name) const { (void) guard; for(const auto & it : _flushing) { diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h b/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h index 84bf91a2f9a..2d0be231adc 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h @@ -31,8 +31,7 @@ public: * @param target The target to decorate. * @param prefix The prefix to prepend to the target */ - FlushTargetProxy(const IFlushTarget::SP &target, - const vespalib::string & prefix); + FlushTargetProxy(const IFlushTarget::SP &target, const vespalib::string & prefix); /** * Returns the decorated flush target. This should not be used for anything * but testing, as invoking a method on the returned target beats the @@ -40,34 +39,16 @@ public: * * @return The decorated flush target. */ - const IFlushTarget::SP & - getFlushTarget() const - { - return _target; - } + const IFlushTarget::SP & getFlushTarget() const { return _target; } // Implements IFlushTarget. - virtual MemoryGain - getApproxMemoryGain() const override; - - virtual DiskGain - getApproxDiskGain() const override; - - virtual SerialNum - getFlushedSerialNum() const override; - - virtual Time - getLastFlushTime() const override; - - virtual bool - needUrgentFlush() const override; - - virtual Task::UP - initFlush(SerialNum currentSerial, std::shared_ptr flush_token) override; - - virtual searchcorespi::FlushStats - getLastFlushStats() const override; - - virtual uint64_t getApproxBytesToWriteToDisk() const override; + MemoryGain getApproxMemoryGain() const override; + DiskGain getApproxDiskGain() const override; + SerialNum getFlushedSerialNum() const override; + Time getLastFlushTime() const override; + bool needUrgentFlush() const override; + Task::UP initFlush(SerialNum currentSerial, std::shared_ptr flush_token) override; + searchcorespi::FlushStats getLastFlushStats() const override; + uint64_t getApproxBytesToWriteToDisk() const override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp index 94476d93b15..2b1bd3b423d 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp @@ -1,17 +1,18 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "flushtask.h" +#include "flushengine.h" namespace proton { FlushTask::FlushTask(uint32_t taskId, FlushEngine &engine, - const FlushContext::SP &ctx) + std::shared_ptr ctx) : _taskId(taskId), _engine(engine), - _context(ctx) + _context(std::move(ctx)) { - assert(_context.get() != NULL); + assert(_context); } FlushTask::~FlushTask() diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h index 7860acaab41..fb3474e2a90 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h @@ -1,10 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include +#include +#include namespace proton { +class FlushEngine; +class FlushContext; /** * This class decorates a task returned by initFlush() in IFlushTarget so that * the appropriate callback is invoked on the running FlushEngine. @@ -14,7 +17,7 @@ class FlushTask : public vespalib::Executor::Task private: uint32_t _taskId; FlushEngine &_engine; - FlushContext::SP _context; + std::shared_ptr _context; public: FlushTask(const FlushTask &) = delete; @@ -26,15 +29,14 @@ public: * @param engine The running flush engine. * @param ctx The context of the flush to perform. */ - FlushTask(uint32_t taskId, FlushEngine &engine, const FlushContext::SP &ctx); + FlushTask(uint32_t taskId, FlushEngine &engine, std::shared_ptr ctx); /** * Destructor. Notifies the engine that the flush is done to prevent the * engine from locking targets because of a glitch. */ - ~FlushTask(); + ~FlushTask() override; - // Implements Executor::Task. void run() override; }; diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h b/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h index ca23f27ed49..66ca730bfbc 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h @@ -2,7 +2,6 @@ #pragma once #include -#include namespace proton { @@ -35,7 +34,7 @@ public: /** * Virtual destructor required for inheritance. */ - virtual ~IFlushHandler() { } + virtual ~IFlushHandler() = default; /** * Returns the unique name of this handler. diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h b/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h index 67f6771650a..8c6a31ff270 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h @@ -14,17 +14,12 @@ namespace flushengine { class TlsStatsMap; } */ class IFlushStrategy { public: - /** - * Convenience typedefs. - */ typedef std::shared_ptr SP; IFlushStrategy(const IFlushStrategy &) = delete; IFlushStrategy & operator = (const IFlushStrategy &) = delete; - /** - * Virtual destructor required for inheritance. - */ - virtual ~IFlushStrategy() { } + + virtual ~IFlushStrategy() = default; /** * Takes an input of targets that are candidates for flush and returns @@ -34,11 +29,10 @@ public: * @return A prioritized list of targets to flush. */ virtual FlushContext::List getFlushTargets(const FlushContext::List & targetList, - const flushengine::TlsStatsMap & - tlsStatsMap) const = 0; + const flushengine::TlsStatsMap & tlsStatsMap) const = 0; protected: IFlushStrategy() = default; }; -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp index 59a5504b57b..3db341ecb70 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp @@ -49,11 +49,9 @@ ThreadedFlushTarget::initFlush(SerialNum currentSerial, std::shared_ptr promise; std::future future = promise.get_future(); - _executor.execute(makeLambdaTask([&]() - { promise.set_value(callInitFlush(_target.get(), - currentSerial, - &_getSerialNum, - flush_token)); })); + _executor.execute(makeLambdaTask([&]() { + promise.set_value(callInitFlush(_target.get(), currentSerial, &_getSerialNum, flush_token)); + })); return future.get(); } diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h index cd2ee11265f..d949ffa9c50 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h @@ -18,7 +18,7 @@ class ThreadedFlushTarget : public FlushTargetProxy { private: using IFlushTarget = searchcorespi::IFlushTarget; - vespalib::Executor &_executor; + vespalib::Executor &_executor; const IGetSerialNum &_getSerialNum; public: @@ -48,9 +48,7 @@ public: const IFlushTarget::SP &target, const vespalib::string & prefix); - // Implements IFlushTarget. - virtual Task::UP - initFlush(SerialNum currentSerial, std::shared_ptr flush_token) override; + Task::UP initFlush(SerialNum currentSerial, std::shared_ptr flush_token) override; }; } // namespace proton diff --git a/searchlib/src/vespa/searchlib/attribute/attributesaver.h b/searchlib/src/vespa/searchlib/attribute/attributesaver.h index f90f9492487..c1bf18e775c 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributesaver.h +++ b/searchlib/src/vespa/searchlib/attribute/attributesaver.h @@ -2,11 +2,10 @@ #pragma once -#include #include "attribute_header.h" +#include -namespace search -{ +namespace search { class IAttributeSaveTarget; -- cgit v1.2.3