diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2020-04-20 22:30:38 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2020-04-20 22:30:38 +0000 |
commit | e7bf367d844ff3da38e63921235a46e148f48ee2 (patch) | |
tree | 4e56a43c3feb3feb14d2f04059560892d27101f0 /storageapi | |
parent | 2526a60f701a7b2fdd338ccabaa3d0ab669ca274 (diff) |
Use atomic counter instead of locks for a counter.
Diffstat (limited to 'storageapi')
-rw-r--r-- | storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp | 29 | ||||
-rw-r--r-- | storageapi/src/vespa/storageapi/messageapi/storagemessage.h | 7 |
2 files changed, 9 insertions, 27 deletions
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp index 0e871720ad0..93030f699cc 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp @@ -8,21 +8,13 @@ #include <vespa/vespalib/stllike/asciistream.h> #include <sstream> #include <cassert> +#include <atomic> namespace storage::api { namespace { -/** - * TODO - * From @vekterli - * I have no idea why the _lastMsgId update code masks away the 8 MSB, but if we assume it's probably for no - * overwhelmingly good reason we could replace this mutex with just a std::atomic<uint64_t> and do a relaxed - * fetch_add (shouldn't be any need for any barriers; ID increments have no other memory dependencies). U64 overflows - * here come under the category "never gonna happen in the real world". - * @balder agree - @vekterli fix in separate pull request :) - */ -vespalib::Lock _G_msgIdLock; +std::atomic<uint64_t> _G_lastMsgId(1000); } @@ -122,8 +114,8 @@ MessageType::MessageType::get(Id id) return *it->second; } MessageType::MessageType(vespalib::stringref name, Id id, - const MessageType* replyOf) - : _name(name), _id(id), _reply(NULL), _replyOf(replyOf) + const MessageType* replyOf) + : _name(name), _id(id), _reply(NULL), _replyOf(replyOf) { _codes[id] = this; if (_replyOf != 0) { @@ -134,7 +126,7 @@ MessageType::MessageType(vespalib::stringref name, Id id, } } -MessageType::~MessageType() {} +MessageType::~MessageType() = default; void MessageType::print(std::ostream& out, bool verbose, const std::string& indent) const @@ -255,15 +247,10 @@ StorageMessageAddress::print(vespalib::asciistream & out) const TransportContext::~TransportContext() = default; -StorageMessage::Id StorageMessage::_lastMsgId = 1000; - StorageMessage::Id StorageMessage::generateMsgId() { - vespalib::LockGuard sync(_G_msgIdLock); - Id msgId = _lastMsgId++; - _lastMsgId &= ((Id(-1) << 8) >> 8); - return msgId; + return _G_lastMsgId.fetch_add(1, std::memory_order_relaxed); } StorageMessage::StorageMessage(const MessageType& type, Id id) @@ -290,9 +277,7 @@ StorageMessage::~StorageMessage() = default; void StorageMessage::setNewMsgId() { - vespalib::LockGuard sync(_G_msgIdLock); - _msgId = _lastMsgId++; - _lastMsgId &= ((Id(-1) << 8) >> 8); + _msgId = generateMsgId(); } vespalib::string diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h index ffbc24cd724..415bd7717f2 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h @@ -349,11 +349,6 @@ public: static const char* getPriorityString(Priority); private: - static Id _lastMsgId; - - StorageMessage& operator=(const StorageMessage&); - StorageMessage(const StorageMessage&); - mutable std::unique_ptr<TransportContext> _transportContext; protected: @@ -372,6 +367,8 @@ protected: static document::Bucket getDummyBucket() { return document::Bucket(document::BucketSpace::invalid(), document::BucketId()); } public: + StorageMessage& operator=(const StorageMessage&) = delete; + StorageMessage(const StorageMessage&) = delete; virtual ~StorageMessage(); Id getMsgId() const { return _msgId; } |