From d959db08f601bb916f7f00ec69a9aa2f3943255e Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 8 Oct 2020 08:29:25 +0000 Subject: Use vespalib::Lock -> std::mutex --- .../tests/docstore/file_chunk/file_chunk_test.cpp | 28 +++++++++---------- .../src/vespa/searchlib/docstore/filechunk.cpp | 2 +- searchlib/src/vespa/searchlib/docstore/filechunk.h | 9 +++---- searchlib/src/vespa/searchlib/docstore/lid_info.h | 9 ++++--- .../src/vespa/searchlib/docstore/logdatastore.cpp | 7 +++-- .../src/vespa/searchlib/docstore/logdatastore.h | 31 +++++++++++----------- .../searchlib/docstore/writeablefilechunk.cpp | 29 ++++++++++---------- .../vespa/searchlib/docstore/writeablefilechunk.h | 6 ++--- 8 files changed, 59 insertions(+), 62 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/tests/docstore/file_chunk/file_chunk_test.cpp b/searchlib/src/tests/docstore/file_chunk/file_chunk_test.cpp index 70c23dc4191..1e99221551f 100644 --- a/searchlib/src/tests/docstore/file_chunk/file_chunk_test.cpp +++ b/searchlib/src/tests/docstore/file_chunk/file_chunk_test.cpp @@ -21,7 +21,7 @@ using common::FileHeaderContext; using vespalib::ThreadStackExecutor; struct MyFileHeaderContext : public FileHeaderContext { - virtual void addTags(vespalib::GenericHeader &header, const vespalib::string &name) const override { + void addTags(vespalib::GenericHeader &header, const vespalib::string &name) const override { (void) header; (void) name; } @@ -29,7 +29,7 @@ struct MyFileHeaderContext : public FileHeaderContext { struct SetLidObserver : public ISetLid { std::vector lids; - virtual void setLid(const vespalib::LockGuard &guard, uint32_t lid, const LidInfo &lidInfo) override { + void setLid(const LockGuard &guard, uint32_t lid, const LidInfo &lidInfo) override { (void) guard; (void) lidInfo; lids.push_back(lid); @@ -38,12 +38,12 @@ struct SetLidObserver : public ISetLid { struct BucketizerObserver : public IBucketizer { mutable std::vector lids; - virtual document::BucketId getBucketOf(const vespalib::GenerationHandler::Guard &guard, uint32_t lid) const override { + document::BucketId getBucketOf(const vespalib::GenerationHandler::Guard &guard, uint32_t lid) const override { (void) guard; lids.push_back(lid); return document::BucketId(); } - virtual vespalib::GenerationHandler::Guard getGuard() const override { + vespalib::GenerationHandler::Guard getGuard() const override { return vespalib::GenerationHandler::Guard(); } }; @@ -62,7 +62,7 @@ struct FixtureBase { uint64_t serialNum; TuneFileSummary tuneFile; MyFileHeaderContext fileHeaderCtx; - vespalib::Lock updateLock; + std::mutex updateLock; SetLidObserver lidObserver; BucketizerObserver bucketizer; @@ -70,8 +70,7 @@ struct FixtureBase { return serialNum++; }; - FixtureBase(const vespalib::string &baseName, - bool dirCleanup = true) + explicit FixtureBase(const vespalib::string &baseName, bool dirCleanup = true) : dir(baseName), executor(1, 0x10000), serialNum(1), @@ -83,20 +82,21 @@ struct FixtureBase { { dir.cleanup(dirCleanup); } - ~FixtureBase() {} - void assertLidMap(const std::vector &expLids) { + ~FixtureBase(); + void assertLidMap(const std::vector &expLids) const { EXPECT_EQUAL(expLids, lidObserver.lids); } - void assertBucketizer(const std::vector &expLids) { + void assertBucketizer(const std::vector &expLids) const { EXPECT_EQUAL(expLids, bucketizer.lids); } }; +FixtureBase::~FixtureBase() = default; + struct ReadFixture : public FixtureBase { FileChunk chunk; - ReadFixture(const vespalib::string &baseName, - bool dirCleanup = true) + explicit ReadFixture(const vespalib::string &baseName, bool dirCleanup = true) : FixtureBase(baseName, dirCleanup), chunk(FileChunk::FileId(0), FileChunk::NameId(1234), @@ -108,7 +108,7 @@ struct ReadFixture : public FixtureBase { dir.cleanup(dirCleanup); } void updateLidMap(uint32_t docIdLimit) { - vespalib::LockGuard guard(updateLock); + std::unique_lock guard(updateLock); chunk.updateLidMap(guard, lidObserver, serialNum, docIdLimit); } }; @@ -145,7 +145,7 @@ struct WriteFixture : public FixtureBase { return *this; } void updateLidMap(uint32_t docIdLimit) { - vespalib::LockGuard guard(updateLock); + std::unique_lock guard(updateLock); chunk.updateLidMap(guard, lidObserver, serialNum, docIdLimit); serialNum = chunk.getSerialNum(); } diff --git a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp index b4ef45187ee..d66e178717c 100644 --- a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp @@ -320,7 +320,7 @@ appendChunks(FixedParams * args, Chunk::UP chunk) for (const Chunk::Entry & e : ll) { LidInfo lidInfo(args->fileId, chunk->getId(), e.netSize()); if (args->db.getLid(args->lidReadGuard, e.getLid()) == lidInfo) { - vespalib::LockGuard guard(args->db.getLidGuard(e.getLid())); + auto guard(args->db.getLidGuard(e.getLid())); if (args->db.getLid(args->lidReadGuard, e.getLid()) == lidInfo) { // I am still in use so I need to taken care of. vespalib::ConstBufferRef data(chunk->getLid(e.getLid())); diff --git a/searchlib/src/vespa/searchlib/docstore/filechunk.h b/searchlib/src/vespa/searchlib/docstore/filechunk.h index 6839423d6db..3febb51ca69 100644 --- a/searchlib/src/vespa/searchlib/docstore/filechunk.h +++ b/searchlib/src/vespa/searchlib/docstore/filechunk.h @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -30,9 +29,9 @@ class IWriteData { public: typedef std::unique_ptr UP; - using LockGuard = vespalib::LockGuard; + using LockGuard = std::unique_lock; - virtual ~IWriteData() { } + virtual ~IWriteData() = default; virtual void write(LockGuard guard, uint32_t chunkId, uint32_t lid, const void *buffer, size_t sz) = 0; virtual void close() = 0; @@ -41,7 +40,7 @@ public: class IFileChunkVisitorProgress { public: - virtual ~IFileChunkVisitorProgress() { } + virtual ~IFileChunkVisitorProgress() = default; virtual void updateProgress() = 0; }; @@ -73,7 +72,7 @@ private: class FileChunk { public: - using LockGuard = vespalib::LockGuard; + using LockGuard = std::unique_lock; class NameId { public: explicit NameId(size_t id) noexcept : _id(id) { } diff --git a/searchlib/src/vespa/searchlib/docstore/lid_info.h b/searchlib/src/vespa/searchlib/docstore/lid_info.h index 8444a9e3575..56629e4988a 100644 --- a/searchlib/src/vespa/searchlib/docstore/lid_info.h +++ b/searchlib/src/vespa/searchlib/docstore/lid_info.h @@ -68,8 +68,8 @@ typedef std::vector LidInfoWithLidV; class ISetLid { public: - using LockGuard = vespalib::LockGuard; - virtual ~ISetLid() { } + using LockGuard = std::unique_lock; + virtual ~ISetLid() = default; virtual void setLid(const LockGuard & guard, uint32_t lid, const LidInfo & lm) = 0; }; @@ -77,10 +77,11 @@ class IGetLid { public: using Guard = vespalib::GenerationHandler::Guard; - virtual ~IGetLid() { } + using LockGuard = std::unique_lock; + virtual ~IGetLid() = default; virtual LidInfo getLid(const Guard & guard, uint32_t lid) const = 0; - virtual vespalib::LockGuard getLidGuard(uint32_t lid) const = 0; + virtual LockGuard getLidGuard(uint32_t lid) const = 0; virtual Guard getLidReadGuard() const = 0; }; diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp index 4adb3507eeb..37510afc572 100644 --- a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp +++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp @@ -464,8 +464,7 @@ bool LogDataStore::shouldCompactToActiveFile(size_t compactedSize) const { void LogDataStore::setNewFileChunk(const LockGuard & guard, FileChunk::UP file) { - (void) guard; - assert(guard.locks(_updateLock)); + assert(hasUpdateLock(guard)); size_t fileId = file->getFileId().getId(); assert( ! _fileChunks[fileId]); _fileChunks[fileId] = std::move(file); @@ -854,7 +853,7 @@ LogDataStore::findIncompleteCompactedFiles(const NameIdSet & partList) { LogDataStore::NameIdSet LogDataStore::getAllActiveFiles() const { NameIdSet files; - vespalib::LockGuard guard(_updateLock); + LockGuard guard(_updateLock); for (const auto & fc : _fileChunks) { if (fc) { files.insert(fc->getNameId()); @@ -1219,7 +1218,7 @@ LogDataStore::canShrinkLidSpace() const } bool -LogDataStore::canShrinkLidSpace(const vespalib::LockGuard &) const +LogDataStore::canShrinkLidSpace(const LockGuard &) const { return getDocIdLimit() < _lidInfo.size() && _compactLidSpaceGeneration < _genHandler.getFirstUsedGeneration(); diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.h b/searchlib/src/vespa/searchlib/docstore/logdatastore.h index c709c607f37..12073c49cde 100644 --- a/searchlib/src/vespa/searchlib/docstore/logdatastore.h +++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.h @@ -33,7 +33,7 @@ private: using FileId = FileChunk::FileId; public: using NameIdSet = std::set; - using LockGuard = vespalib::LockGuard; + using LockGuard = std::unique_lock; using CompressionConfig = vespalib::compression::CompressionConfig; class Config { public: @@ -146,9 +146,9 @@ public: } // Implements IGetLid API - LockGuard getLidGuard(uint32_t lid) const override { + IGetLid::LockGuard getLidGuard(uint32_t lid) const override { (void) lid; - return LockGuard(_updateLock); + return IGetLid::LockGuard(_updateLock); } // Implements IGetLid API @@ -160,11 +160,14 @@ public: return LidInfo(); } } - FileId getActiveFileId(const vespalib::LockGuard & guard) const { - assert(guard.locks(_updateLock)); + FileId getActiveFileId(const LockGuard & guard) const { + assert(hasUpdateLock(guard)); (void) guard; return _active; } + bool hasUpdateLock(const LockGuard & guard) const { + return (guard.mutex() == &_updateLock) && guard.owns_lock(); + } DataStoreStorageStats getStorageStats() const override; vespalib::MemoryUsage getMemoryUsage() const override; @@ -185,7 +188,7 @@ private: class FileChunkHolder; // Implements ISetLid API - void setLid(const LockGuard & guard, uint32_t lid, const LidInfo & lm) override; + void setLid(const ISetLid::LockGuard & guard, uint32_t lid, const LidInfo & lm) override; void compactWorst(double bloatLimit, double spreadLimit, bool prioritizeDiskBloat); void compactFile(FileId chunkId); @@ -211,25 +214,21 @@ private: vespalib::string ls(const NameIdSet & partList); WriteableFileChunk & getActive(const LockGuard & guard) { - assert(guard.locks(_updateLock)); - (void) guard; + assert(hasUpdateLock(guard)); return static_cast(*_fileChunks[_active.getId()]); } const WriteableFileChunk & getActive(const LockGuard & guard) const { - assert(guard.locks(_updateLock)); - (void) guard; + assert(hasUpdateLock(guard)); return static_cast(*_fileChunks[_active.getId()]); } const FileChunk * getPrevActive(const LockGuard & guard) const { - assert(guard.locks(_updateLock)); - (void) guard; + assert(hasUpdateLock(guard)); return ( !_prevActive.isActive() ) ? _fileChunks[_prevActive.getId()].get() : nullptr; } void setActive(const LockGuard & guard, FileId fileId) { - assert(guard.locks(_updateLock)); - (void) guard; + assert(hasUpdateLock(guard)); _prevActive = _active; _active = fileId; } @@ -270,7 +269,7 @@ private: bool shouldCompactToActiveFile(size_t compactedSize) const; std::pair findNextToCompact(double bloatLimit, double spreadLimit, bool prioritizeDiskBloat); void incGeneration(); - bool canShrinkLidSpace(const vespalib::LockGuard &guard) const; + bool canShrinkLidSpace(const LockGuard &guard) const; typedef std::vector FileIdxVector; Config _config; @@ -282,7 +281,7 @@ private: std::vector _holdFileChunks; FileId _active; FileId _prevActive; - vespalib::Lock _updateLock; + mutable std::mutex _updateLock; bool _readOnly; vespalib::ThreadExecutor &_executor; SerialNum _initFlushSyncToken; diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp index beece528795..cdf6220dfbc 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp @@ -220,7 +220,7 @@ WriteableFileChunk::read(LidInfoWithLidV::const_iterator begin, size_t count, IB vespalib::hash_map chunksOnFile; std::vector buffers; { - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); for (size_t i(0); i < count; i++) { const LidInfoWithLid & li = *(begin + i); uint32_t chunk = li.getChunkId(); @@ -260,7 +260,7 @@ WriteableFileChunk::read(uint32_t lid, SubChunkId chunkId, vespalib::DataBuffer { ChunkInfo chunkInfo; if (!frozen()) { - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); if ((chunkId >= _chunkInfo.size()) || !_chunkInfo[chunkId].valid()) { auto found = _chunkMap.find(chunkId); if (found != _chunkMap.end()) { @@ -282,7 +282,7 @@ WriteableFileChunk::internalFlush(uint32_t chunkId, uint64_t serialNum) { Chunk * active(nullptr); { - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); active = _chunkMap[chunkId].get(); } @@ -298,7 +298,7 @@ WriteableFileChunk::internalFlush(uint32_t chunkId, uint64_t serialNum) tmp->getBuf().moveFreeToData(padAfter); } { - LockGuard innerGuard(_lock); + vespalib::LockGuard innerGuard(_lock); setDiskFootprint(FileChunk::getDiskFootprint() + tmp->getBuf().getDataLen()); } enque(std::move(tmp)); @@ -396,11 +396,11 @@ WriteableFileChunk::fetchNextChain(ProcessedChunkMap & orderedChunks, const uint } ChunkMeta -WriteableFileChunk::computeChunkMeta(const LockGuard & guard, +WriteableFileChunk::computeChunkMeta(const vespalib::LockGuard & guard, const GenerationHandler::Guard & bucketizerGuard, size_t offset, const ProcessedChunk & tmp, const Chunk & active) { - (void) guard; + assert(guard.locks(_lock)); size_t dataLen = tmp.getBuf().getDataLen(); const ChunkMeta cmeta(offset, tmp.getPayLoad(), active.getLastSerial(), active.count()); assert((size_t(tmp.getBuf().getData())%_alignment) == 0); @@ -432,7 +432,7 @@ WriteableFileChunk::computeChunkMeta(ProcessedChunkQ & chunks, size_t startPos, cmetaV.reserve(chunks.size()); uint64_t lastSerial(_lastPersistedSerialNum); (void) lastSerial; - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); if (!_pendingChunks.empty()) { const PendingChunk & pc = *_pendingChunks.back(); @@ -541,7 +541,7 @@ WriteableFileChunk::fileWriter(const uint32_t firstChunkId) vespalib::system_time WriteableFileChunk::getModificationTime() const { - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); return _modificationTime; } @@ -595,7 +595,7 @@ size_t WriteableFileChunk::getMemoryFootprint() const { size_t sz(0); - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); for (const auto & it : _chunkMap) { sz += it.second->size(); } @@ -613,7 +613,7 @@ WriteableFileChunk::getMemoryMetaFootprint() const vespalib::MemoryUsage WriteableFileChunk::getMemoryUsage() const { - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); vespalib::MemoryUsage result; for (const auto &chunk : _chunkMap) { result.merge(chunk.second->getMemoryUsage()); @@ -843,7 +843,7 @@ WriteableFileChunk::updateCurrentDiskFootprint() { */ void WriteableFileChunk::flushPendingChunks(uint64_t serialNum) { - LockGuard flushGuard(_flushLock); + std::unique_lock flushGuard(_flushLock); if (frozen()) return; uint64_t datFileLen = _dataFile.getSize(); @@ -851,15 +851,14 @@ WriteableFileChunk::flushPendingChunks(uint64_t serialNum) { if (needFlushPendingChunks(serialNum, datFileLen)) { timeStamp = unconditionallyFlushPendingChunks(flushGuard, serialNum, datFileLen); } - LockGuard guard(_lock); + vespalib::LockGuard guard(_lock); _modificationTime = std::max(timeStamp, _modificationTime); } vespalib::system_time -WriteableFileChunk::unconditionallyFlushPendingChunks(const vespalib::LockGuard &flushGuard, uint64_t serialNum, uint64_t datFileLen) +WriteableFileChunk::unconditionallyFlushPendingChunks(const LockGuard &flushGuard, uint64_t serialNum, uint64_t datFileLen) { - (void) flushGuard; - assert(flushGuard.locks(_flushLock)); + assert((flushGuard.mutex() == &_flushLock) && flushGuard.owns_lock()); if ( ! _dataFile.Sync()) { throw SummaryException("Failed fsync of dat file", _dataFile, VESPA_STRLOC); } diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h index 65dd822ac1f..1c6dd58d585 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h @@ -90,7 +90,7 @@ private: void writeDataHeader(const common::FileHeaderContext &fileHeaderContext); bool needFlushPendingChunks(uint64_t serialNum, uint64_t datFileLen); bool needFlushPendingChunks(const vespalib::MonitorGuard & guard, uint64_t serialNum, uint64_t datFileLen); - vespalib::system_time unconditionallyFlushPendingChunks(const vespalib::LockGuard & flushGuard, uint64_t serialNum, uint64_t datFileLen); + vespalib::system_time unconditionallyFlushPendingChunks(const LockGuard & flushGuard, uint64_t serialNum, uint64_t datFileLen); static void insertChunks(ProcessedChunkMap & orderedChunks, ProcessedChunkQ & newChunks, const uint32_t nextChunkId); static ProcessedChunkQ fetchNextChain(ProcessedChunkMap & orderedChunks, const uint32_t firstChunkId); ChunkMeta computeChunkMeta(const vespalib::LockGuard & guard, @@ -108,8 +108,8 @@ private: bool _frozen; // Lock order is _writeLock, _flushLock, _lock vespalib::Monitor _lock; - vespalib::Lock _writeLock; - vespalib::Lock _flushLock; + std::mutex _writeLock; + std::mutex _flushLock; FastOS_File _dataFile; using ChunkMap = std::map; ChunkMap _chunkMap; -- cgit v1.2.3