This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 04c5a922 Make lock guard movable & accept generalized arguments (#1636)
04c5a922 is described below
commit 04c5a922a71850d2e143297c30baf87d99e9e075
Author: Twice <[email protected]>
AuthorDate: Sun Aug 6 07:11:40 2023 +0800
Make lock guard movable & accept generalized arguments (#1636)
---
.clang-tidy | 2 +-
src/common/lock_manager.h | 130 ++++++++++++++++++++++++++++++++++++++++++++
src/storage/lock_manager.cc | 62 ---------------------
src/storage/lock_manager.h | 88 ------------------------------
src/types/redis_set.cc | 5 +-
src/types/redis_zset.cc | 13 +++--
6 files changed, 141 insertions(+), 159 deletions(-)
diff --git a/.clang-tidy b/.clang-tidy
index ccc27aec..09e05a96 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -1,5 +1,5 @@
# refer to https://clang.llvm.org/extra/clang-tidy/checks/list.html
-Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*,
clang-analyzer-deadcode.*, clang-analyzer-nullability.*,
clang-analyzer-security.*, clang-analyzer-unix.*, clang-analyzer-valist.*,
cppcoreguidelines-init-variables, cppcoreguidelines-macro-usage,
cppcoreguidelines-interfaces-global-init,
cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc,
cppcoreguidelines-prefer-member-initializer,
cppcoreguidelines-special-member-functions, cppcoreguidelines-slicing, goog
[...]
+Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*,
-clang-analyzer-cplusplus.InnerPointer, clang-analyzer-deadcode.*,
clang-analyzer-nullability.*, clang-analyzer-security.*, clang-analyzer-unix.*,
clang-analyzer-valist.*, cppcoreguidelines-init-variables,
cppcoreguidelines-macro-usage, cppcoreguidelines-interfaces-global-init,
cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc,
cppcoreguidelines-prefer-member-initializer,
cppcoreguidelines-special-member-fu [...]
WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand,
google-*, performance-*, cppcoreguidelines-*, modernize-*, readability-*,
bugprone-*
diff --git a/src/common/lock_manager.h b/src/common/lock_manager.h
new file mode 100644
index 00000000..9e4d98ad
--- /dev/null
+++ b/src/common/lock_manager.h
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#pragma once
+
+#include <rocksdb/db.h>
+
+#include <functional>
+#include <mutex>
+#include <set>
+#include <string>
+#include <vector>
+
+class LockManager {
+ public:
+ explicit LockManager(unsigned hash_power) : hash_power_(hash_power),
hash_mask_((1U << hash_power) - 1) {
+ mutex_pool_.reserve(Size());
+ for (unsigned i = 0; i < Size(); i++) {
+ mutex_pool_.emplace_back(new std::mutex{});
+ }
+ }
+ ~LockManager() = default;
+
+ LockManager(const LockManager &) = delete;
+ LockManager &operator=(const LockManager &) = delete;
+
+ unsigned Size() const { return (1U << hash_power_); }
+
+ void Lock(std::string_view key) { mutex_pool_[hash(key)]->lock(); }
+ void UnLock(std::string_view key) { mutex_pool_[hash(key)]->unlock(); }
+ void Lock(rocksdb::Slice key) { Lock(key.ToStringView()); }
+ void UnLock(rocksdb::Slice key) { UnLock(key.ToStringView()); }
+
+ template <typename Keys>
+ std::vector<std::mutex *> MultiGet(const Keys &keys) {
+ std::set<unsigned, std::greater<unsigned>> to_acquire_indexes;
+ // We are using the `set` to avoid retrieving the mutex, as well as
guarantee to retrieve
+ // the order of locks.
+ //
+ // For example, we need lock the key `A` and `B` and they have the same
lock hash
+ // index, it will be deadlock if lock the same mutex twice. Besides, we
also need
+ // to order the mutex before acquiring locks since different threads may
acquire
+ // same keys with different order.
+ for (const auto &key : keys) {
+ to_acquire_indexes.insert(hash(key));
+ }
+
+ std::vector<std::mutex *> locks;
+ locks.reserve(to_acquire_indexes.size());
+ for (auto index : to_acquire_indexes) {
+ locks.emplace_back(mutex_pool_[index].get());
+ }
+ return locks;
+ }
+
+ private:
+ unsigned hash_power_;
+ unsigned hash_mask_;
+ std::vector<std::unique_ptr<std::mutex>> mutex_pool_;
+
+ unsigned hash(std::string_view key) const { return
std::hash<std::string_view>{}(key)&hash_mask_; }
+};
+
+template <typename KeyType>
+class BasicLockGuard {
+ public:
+ explicit BasicLockGuard(LockManager *lock_mgr, KeyType key) :
lock_mgr_(lock_mgr), key_(std::move(key)) {
+ lock_mgr->Lock(key_);
+ }
+ ~BasicLockGuard() {
+ if (lock_mgr_) lock_mgr_->UnLock(key_);
+ }
+
+ BasicLockGuard(const BasicLockGuard &) = delete;
+ BasicLockGuard &operator=(const BasicLockGuard &) = delete;
+
+ BasicLockGuard(BasicLockGuard &&guard) : lock_mgr_(guard.lock_mgr_),
key_(std::move(guard.key_)) {
+ guard.lock_mgr_ = nullptr;
+ }
+
+ private:
+ LockManager *lock_mgr_ = nullptr;
+ KeyType key_;
+};
+
+using LockGuard = BasicLockGuard<rocksdb::Slice>;
+
+class MultiLockGuard {
+ public:
+ template <typename Keys>
+ explicit MultiLockGuard(LockManager *lock_mgr, const Keys &keys)
+ : lock_mgr_(lock_mgr), locks_(lock_mgr_->MultiGet(keys)) {
+ for (const auto &iter : locks_) {
+ iter->lock();
+ }
+ }
+
+ ~MultiLockGuard() {
+ // Lock with order `A B C` and unlock should be `C B A`
+ for (auto iter = locks_.rbegin(); iter != locks_.rend(); ++iter) {
+ (*iter)->unlock();
+ }
+ }
+
+ MultiLockGuard(const MultiLockGuard &) = delete;
+ MultiLockGuard &operator=(const MultiLockGuard &) = delete;
+
+ MultiLockGuard(MultiLockGuard &&guard) : lock_mgr_(guard.lock_mgr_),
locks_(std::move(guard.locks_)) {}
+
+ private:
+ LockManager *lock_mgr_ = nullptr;
+ std::vector<std::mutex *> locks_;
+};
diff --git a/src/storage/lock_manager.cc b/src/storage/lock_manager.cc
deleted file mode 100644
index 6a1d550d..00000000
--- a/src/storage/lock_manager.cc
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-
-#include "lock_manager.h"
-
-#include <set>
-#include <string>
-#include <thread>
-
-LockManager::LockManager(int hash_power) : hash_power_(hash_power),
hash_mask_((1U << hash_power) - 1) {
- for (unsigned i = 0; i < Size(); i++) {
- mutex_pool_.emplace_back(new std::mutex{});
- }
-}
-
-unsigned LockManager::hash(const rocksdb::Slice &key) const {
- return std::hash<std::string_view>{}(std::string_view{key.data(),
key.size()}) & hash_mask_;
-}
-
-unsigned LockManager::Size() const { return (1U << hash_power_); }
-
-void LockManager::Lock(const rocksdb::Slice &key) {
mutex_pool_[hash(key)]->lock(); }
-
-void LockManager::UnLock(const rocksdb::Slice &key) {
mutex_pool_[hash(key)]->unlock(); }
-
-std::vector<std::mutex *> LockManager::MultiGet(const std::vector<std::string>
&keys) {
- std::set<unsigned, std::greater<unsigned>> to_acquire_indexes;
- // We are using the `set` to avoid retrieving the mutex, as well as
guarantee to retrieve
- // the order of locks.
- //
- // For example, we need lock the key `A` and `B` and they have the same lock
hash
- // index, it will be deadlock if lock the same mutex twice. Besides, we also
need
- // to order the mutex before acquiring locks since different threads may
acquire
- // same keys with different order.
- for (const auto &key : keys) {
- to_acquire_indexes.insert(hash(key));
- }
-
- std::vector<std::mutex *> locks;
- locks.reserve(to_acquire_indexes.size());
- for (auto index : to_acquire_indexes) {
- locks.emplace_back(mutex_pool_[index].get());
- }
- return locks;
-}
diff --git a/src/storage/lock_manager.h b/src/storage/lock_manager.h
deleted file mode 100644
index 43a28eb5..00000000
--- a/src/storage/lock_manager.h
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-
-#pragma once
-
-#include <rocksdb/db.h>
-
-#include <functional>
-#include <mutex>
-#include <string>
-#include <vector>
-
-class LockManager {
- public:
- explicit LockManager(int hash_power);
- ~LockManager() = default;
-
- LockManager(const LockManager &) = delete;
- LockManager &operator=(const LockManager &) = delete;
-
- unsigned Size() const;
- void Lock(const rocksdb::Slice &key);
- void UnLock(const rocksdb::Slice &key);
- std::vector<std::mutex *> MultiGet(const std::vector<std::string> &keys);
-
- private:
- int hash_power_;
- unsigned hash_mask_;
- std::vector<std::unique_ptr<std::mutex>> mutex_pool_;
-
- unsigned hash(const rocksdb::Slice &key) const;
-};
-
-class LockGuard {
- public:
- explicit LockGuard(LockManager *lock_mgr, rocksdb::Slice key) :
lock_mgr_(lock_mgr), key_(key) {
- lock_mgr->Lock(key_);
- }
- ~LockGuard() { lock_mgr_->UnLock(key_); }
-
- LockGuard(const LockGuard &) = delete;
- LockGuard &operator=(const LockGuard &) = delete;
-
- private:
- LockManager *lock_mgr_ = nullptr;
- rocksdb::Slice key_;
-};
-
-class MultiLockGuard {
- public:
- explicit MultiLockGuard(LockManager *lock_mgr, const
std::vector<std::string> &keys) : lock_mgr_(lock_mgr) {
- locks_ = lock_mgr_->MultiGet(keys);
- for (const auto &iter : locks_) {
- iter->lock();
- }
- }
-
- ~MultiLockGuard() {
- // Lock with order `A B C` and unlock should be `C B A`
- for (auto iter = locks_.rbegin(); iter != locks_.rend(); ++iter) {
- (*iter)->unlock();
- }
- }
-
- MultiLockGuard(const MultiLockGuard &) = delete;
- MultiLockGuard &operator=(const MultiLockGuard &) = delete;
-
- private:
- LockManager *lock_mgr_ = nullptr;
- std::vector<std::mutex *> locks_;
-};
diff --git a/src/types/redis_set.cc b/src/types/redis_set.cc
index 588b0e3a..31970ff1 100644
--- a/src/types/redis_set.cc
+++ b/src/types/redis_set.cc
@@ -22,6 +22,7 @@
#include <map>
#include <memory>
+#include <optional>
#include "db_util.h"
@@ -203,8 +204,8 @@ rocksdb::Status Set::Take(const Slice &user_key,
std::vector<std::string> *membe
std::string ns_key;
AppendNamespacePrefix(user_key, &ns_key);
- std::unique_ptr<LockGuard> lock_guard;
- if (pop) lock_guard =
std::make_unique<LockGuard>(storage_->GetLockManager(), ns_key);
+ std::optional<LockGuard> lock_guard;
+ if (pop) lock_guard.emplace(storage_->GetLockManager(), ns_key);
SetMetadata metadata(false);
rocksdb::Status s = GetMetadata(ns_key, &metadata);
diff --git a/src/types/redis_zset.cc b/src/types/redis_zset.cc
index e7a1585d..d0994b06 100644
--- a/src/types/redis_zset.cc
+++ b/src/types/redis_zset.cc
@@ -24,6 +24,7 @@
#include <limits>
#include <map>
#include <memory>
+#include <optional>
#include <set>
#include "db_util.h"
@@ -229,8 +230,8 @@ rocksdb::Status ZSet::RangeByRank(const Slice &user_key,
const RangeRankSpec &sp
std::string ns_key;
AppendNamespacePrefix(user_key, &ns_key);
- std::unique_ptr<LockGuard> lock_guard;
- if (spec.with_deletion) lock_guard =
std::make_unique<LockGuard>(storage_->GetLockManager(), ns_key);
+ std::optional<LockGuard> lock_guard;
+ if (spec.with_deletion) lock_guard.emplace(storage_->GetLockManager(),
ns_key);
ZSetMetadata metadata(false);
rocksdb::Status s = GetMetadata(ns_key, &metadata);
if (!s.ok()) return s.IsNotFound() ? rocksdb::Status::OK() : s;
@@ -311,8 +312,8 @@ rocksdb::Status ZSet::RangeByScore(const Slice &user_key,
const RangeScoreSpec &
std::string ns_key;
AppendNamespacePrefix(user_key, &ns_key);
- std::unique_ptr<LockGuard> lock_guard;
- if (spec.with_deletion) lock_guard =
std::make_unique<LockGuard>(storage_->GetLockManager(), ns_key);
+ std::optional<LockGuard> lock_guard;
+ if (spec.with_deletion) lock_guard.emplace(storage_->GetLockManager(),
ns_key);
ZSetMetadata metadata(false);
rocksdb::Status s = GetMetadata(ns_key, &metadata);
if (!s.ok()) return s.IsNotFound() ? rocksdb::Status::OK() : s;
@@ -433,9 +434,9 @@ rocksdb::Status ZSet::RangeByLex(const Slice &user_key,
const RangeLexSpec &spec
std::string ns_key;
AppendNamespacePrefix(user_key, &ns_key);
- std::unique_ptr<LockGuard> lock_guard;
+ std::optional<LockGuard> lock_guard;
if (spec.with_deletion) {
- lock_guard = std::make_unique<LockGuard>(storage_->GetLockManager(),
ns_key);
+ lock_guard.emplace(storage_->GetLockManager(), ns_key);
}
ZSetMetadata metadata(false);
rocksdb::Status s = GetMetadata(ns_key, &metadata);
