From: Radoslaw Zarzynski Date: Mon, 14 Oct 2024 23:16:04 +0000 (+0000) Subject: kv: avoid memcpy around key() in OMAP iterator of KeyValueDB X-Git-Tag: v19.2.3~383^2~5 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=e1487e3f28b10d3de497b05fadd36ef36876d7d2;p=ceph.git kv: avoid memcpy around key() in OMAP iterator of KeyValueDB Signed-off-by: Radoslaw Zarzynski (cherry picked from commit a7c81953d07ab78dfb7c5ccacca07e972a06394c) Conflicts: src/os/bluestore/BlueStore.cc trivial conflict with Onode::finish_write() that doesn't exist in squid. --- diff --git a/src/kv/KeyValueDB.h b/src/kv/KeyValueDB.h index ba3686fe4c8..d02ba7c434a 100644 --- a/src/kv/KeyValueDB.h +++ b/src/kv/KeyValueDB.h @@ -206,6 +206,9 @@ public: return ""; } virtual ceph::buffer::list value() = 0; + // When valid() returns true, value returned as string-view + // is guaranteed to be valid until iterator is moved to another + // position; that is until call to next() / seek_to_first() / etc. virtual std::string_view value_as_sv() = 0; virtual int status() = 0; virtual ~SimplestIteratorImpl() {} @@ -216,7 +219,12 @@ public: virtual ~IteratorImpl() {} virtual int seek_to_last() = 0; virtual int prev() = 0; + // When valid() returns true, key returned as string-view + // is guaranteed to be valid until iterator is moved to another + // position; that is until call to next() / seek_to_first() / etc. + virtual std::string_view key_as_sv() = 0; virtual std::pair raw_key() = 0; + virtual std::pair raw_key_as_sv() = 0; virtual ceph::buffer::ptr value_as_ptr() { ceph::buffer::list bl = value(); if (bl.length() == 1) { @@ -243,7 +251,9 @@ public: virtual int next() = 0; virtual int prev() = 0; virtual std::string key() = 0; + virtual std::string_view key_as_sv() = 0; virtual std::pair raw_key() = 0; + virtual std::pair raw_key_as_sv() = 0; virtual bool raw_key_is_prefixed(const std::string &prefix) = 0; virtual ceph::buffer::list value() = 0; virtual ceph::buffer::ptr value_as_ptr() { @@ -312,9 +322,15 @@ private: std::string key() override { return generic_iter->key(); } + std::string_view key_as_sv() override { + return generic_iter->key_as_sv(); + } std::pair raw_key() override { return generic_iter->raw_key(); } + std::pair raw_key_as_sv() override { + return generic_iter->raw_key_as_sv(); + } ceph::buffer::list value() override { return generic_iter->value(); } diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc index e553e199c87..b92f76ea7f4 100644 --- a/src/kv/RocksDBStore.cc +++ b/src/kv/RocksDBStore.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ using std::ostream; using std::pair; using std::set; using std::string; +using std::string_view; using std::unique_ptr; using std::vector; @@ -1974,7 +1976,7 @@ int RocksDBStore::split_key(rocksdb::Slice in, string *prefix, string *key) // Find separator inside Slice char* separator = (char*) memchr(in.data(), 0, in.size()); - if (separator == NULL) + if (separator == nullptr) return -EINVAL; prefix_len = size_t(separator - in.data()); if (prefix_len >= in.size()) @@ -1988,6 +1990,27 @@ int RocksDBStore::split_key(rocksdb::Slice in, string *prefix, string *key) return 0; } +// TODO: deduplicate the code, preferrably by removing the string variant +int RocksDBStore::split_key(rocksdb::Slice in, string_view *prefix, string_view *key) +{ + size_t prefix_len = 0; + + // Find separator inside Slice + char* separator = (char*) memchr(in.data(), 0, in.size()); + if (separator == nullptr) + return -EINVAL; + prefix_len = size_t(separator - in.data()); + if (prefix_len >= in.size()) + return -EINVAL; + + // Fetch prefix and/or key directly from Slice + if (prefix) + *prefix = string_view(in.data(), prefix_len); + if (key) + *key = string_view(separator + 1, in.size() - prefix_len - 1); + return 0; +} + void RocksDBStore::compact() { dout(2) << __func__ << " starting" << dendl; @@ -2208,7 +2231,13 @@ int RocksDBStore::RocksDBWholeSpaceIteratorImpl::prev() string RocksDBStore::RocksDBWholeSpaceIteratorImpl::key() { string out_key; - split_key(dbiter->key(), 0, &out_key); + split_key(dbiter->key(), nullptr, &out_key); + return out_key; +} +string_view RocksDBStore::RocksDBWholeSpaceIteratorImpl::key_as_sv() +{ + string_view out_key; + split_key(dbiter->key(), nullptr, &out_key); return out_key; } pair RocksDBStore::RocksDBWholeSpaceIteratorImpl::raw_key() @@ -2217,6 +2246,12 @@ pair RocksDBStore::RocksDBWholeSpaceIteratorImpl::raw_key() split_key(dbiter->key(), &prefix, &key); return make_pair(prefix, key); } +pair RocksDBStore::RocksDBWholeSpaceIteratorImpl::raw_key_as_sv() +{ + string_view prefix, key; + split_key(dbiter->key(), &prefix, &key); + return make_pair(prefix, key); +} bool RocksDBStore::RocksDBWholeSpaceIteratorImpl::raw_key_is_prefixed(const string &prefix) { // Look for "prefix\0" right in rocksb::Slice @@ -2336,9 +2371,15 @@ public: string key() override { return dbiter->key().ToString(); } + string_view key_as_sv() override { + return dbiter->key().ToStringView(); + } std::pair raw_key() override { return make_pair(prefix, key()); } + std::pair raw_key_as_sv() override { + return make_pair(prefix, dbiter->key().ToStringView()); + } bufferlist value() override { return to_bufferlist(dbiter->value()); } @@ -2660,6 +2701,15 @@ public: } } + std::string_view key_as_sv() override + { + if (smaller == on_main) { + return main->key_as_sv(); + } else { + return current_shard->second->key_as_sv(); + } + } + std::pair raw_key() override { if (smaller == on_main) { @@ -2669,6 +2719,15 @@ public: } } + std::pair raw_key_as_sv() override + { + if (smaller == on_main) { + return main->raw_key_as_sv(); + } else { + return { current_shard->first, current_shard->second->key_as_sv() }; + } + } + bool raw_key_is_prefixed(const std::string &prefix) override { if (smaller == on_main) { @@ -3018,9 +3077,15 @@ public: string key() override { return iters[0]->key().ToString(); } + string_view key_as_sv() override { + return iters[0]->key().ToStringView(); + } std::pair raw_key() override { return make_pair(prefix, key()); } + std::pair raw_key_as_sv() override { + return make_pair(prefix, iters[0]->key().ToStringView()); + } bufferlist value() override { return to_bufferlist(iters[0]->value()); } diff --git a/src/kv/RocksDBStore.h b/src/kv/RocksDBStore.h index 24330a1c78d..b372e31d120 100644 --- a/src/kv/RocksDBStore.h +++ b/src/kv/RocksDBStore.h @@ -380,7 +380,9 @@ public: int next() override; int prev() override; std::string key() override; + std::string_view key_as_sv() override; std::pair raw_key() override; + std::pair raw_key_as_sv() override; bool raw_key_is_prefixed(const std::string &prefix) override; ceph::bufferlist value() override; ceph::bufferptr value_as_ptr() override; @@ -414,6 +416,7 @@ public: } static int split_key(rocksdb::Slice in, std::string *prefix, std::string *key); + static int split_key(rocksdb::Slice in, std::string_view *prefix, std::string_view *key); static std::string past_prefix(const std::string &prefix); diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 0fd122cdce1..1c482c505f1 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -4818,6 +4818,19 @@ void BlueStore::Onode::decode_omap_key(const string& key, string *user_key) *user_key = key.substr(pos); } +void BlueStore::Onode::decode_omap_key(const std::string_view& key, std::string_view *user_key) +{ + size_t pos = sizeof(uint64_t) + 1; + if (!onode.is_pgmeta_omap()) { + if (onode.is_perpg_omap()) { + pos += sizeof(uint64_t) + sizeof(uint32_t); + } else if (onode.is_perpool_omap()) { + pos += sizeof(uint64_t); + } + } + *user_key = key.substr(pos); +} + // ======================================================= // WriteContext @@ -13521,12 +13534,12 @@ int BlueStore::omap_iterate( ceph::timespan next_lat_acc{0}; o->get_omap_tail(&tail); while (it->valid()) { - std::string user_key; - if (const auto& db_key = it->raw_key().second; db_key >= tail) { + const auto& db_key = it->raw_key_as_sv().second; + if (db_key >= tail) { break; - } else { - o->decode_omap_key(db_key, &user_key); } + std::string_view user_key; + o->decode_omap_key(db_key, &user_key); omap_iter_ret_t ret = f(user_key, it->value_as_sv()); if (ret == omap_iter_ret_t::STOP) { break; diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index fe2257d375a..c92ce9448c0 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -1429,6 +1429,7 @@ public: void rewrite_omap_key(const std::string& old, std::string *out); void decode_omap_key(const std::string& key, std::string *user_key); + void decode_omap_key(const std::string_view& key, std::string_view *user_key); private: void _decode(const ceph::buffer::list& v); diff --git a/src/test/ObjectMap/KeyValueDBMemory.cc b/src/test/ObjectMap/KeyValueDBMemory.cc index 1dd9070d028..cfe25930d6a 100644 --- a/src/test/ObjectMap/KeyValueDBMemory.cc +++ b/src/test/ObjectMap/KeyValueDBMemory.cc @@ -132,12 +132,26 @@ public: return ""; } + string_view key_as_sv() override { + if (valid()) + return (*it).first.second; + else + return ""; + } + pair raw_key() override { if (valid()) return (*it).first; else return make_pair("", ""); } + + pair raw_key_as_sv() override { + if (valid()) + return (*it).first; + else + return make_pair("", ""); + } bool raw_key_is_prefixed(const string &prefix) override { return prefix == (*it).first.first;