From db5a9dd002bf272798ea3bd3a728441d718122c4 Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Sat, 12 Oct 2024 09:15:22 +0000 Subject: [PATCH] kv: avoid memcpy in OMAP iterator of KeyValueDB MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit ``` - 63.07% _ZN12PrimaryLogPG19prepare_transactionEPNS_9OpContextE ▒ - 63.06% _ZN12PrimaryLogPG10do_osd_opsEPNS_9OpContextERSt6vectorI5OSDOpSaIS3_EE ▒ - 20.19% _ZN9BlueStore16OmapIteratorImpl4nextEv ▒ - 12.21% _ZN14CFIteratorImpl4nextEv ▒ + 10.56% _ZN7rocksdb6DBIter4NextEv ▒ 1.02% _ZN7rocksdb18ArenaWrappedDBIter4NextEv ▒ + 3.11% clock_gettime@@GLIBC_2.17 ▒ + 2.44% _ZN9BlueStore11log_latencyEPKciRKNSt6chrono8durationImSt5ratioILl1ELl1000000000EEEEdS1_i ▒ 0.78% pthread_rwlock_rdlock@plt ▒ 0.69% pthread_rwlock_unlock@plt ▒ - 14.28% _ZN9BlueStore16OmapIteratorImpl5valueEv ▒ - 11.60% _ZN14CFIteratorImpl5valueEv ▒ - 11.41% _ZL13to_bufferlistN7rocksdb5SliceE ▒ - 10.50% _ZN4ceph6buffer7v15_2_03ptrC1EPKcj ▒ - _ZN4ceph6buffer7v15_2_04copyEPKcj ▒ - 10.01% _ZN4ceph6buffer7v15_2_014create_alignedEjj ▒ - _ZN4ceph6buffer7v15_2_025create_aligned_in_mempoolEjji ▒ 5.27% _ZN7mempool6pool_t12adjust_countEll ▒ + 3.72% tc_posix_memalign ▒ 0.54% _ZN4ceph6buffer7v15_2_04list6appendEONS1_3ptrE ▒ 1.25% pthread_rwlock_rdlock@plt ▒ 0.90% pthread_rwlock_unlock@plt ``` Signed-off-by: Radoslaw Zarzynski (cherry picked from commit f348ea3cf5328684398795990cc87b91c906609b) --- src/kv/KeyValueDB.h | 6 ++++++ src/kv/RocksDBStore.cc | 23 +++++++++++++++++++++++ src/kv/RocksDBStore.h | 1 + src/os/DBObjectMap.cc | 5 +++++ src/os/DBObjectMap.h | 2 ++ src/os/bluestore/BlueStore.cc | 7 +++++++ src/os/bluestore/BlueStore.h | 1 + src/os/kstore/KStore.cc | 7 +++++++ src/os/kstore/KStore.h | 1 + src/os/memstore/MemStore.cc | 4 ++++ src/test/ObjectMap/KeyValueDBMemory.cc | 7 +++++++ 11 files changed, 64 insertions(+) diff --git a/src/kv/KeyValueDB.h b/src/kv/KeyValueDB.h index 9cfb4482706..ba3686fe4c8 100644 --- a/src/kv/KeyValueDB.h +++ b/src/kv/KeyValueDB.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "include/encoding.h" #include "common/Formatter.h" @@ -205,6 +206,7 @@ public: return ""; } virtual ceph::buffer::list value() = 0; + virtual std::string_view value_as_sv() = 0; virtual int status() = 0; virtual ~SimplestIteratorImpl() {} }; @@ -252,6 +254,7 @@ public: return ceph::buffer::ptr(); } } + virtual std::string_view value_as_sv() = 0; virtual int status() = 0; virtual size_t key_size() { return 0; @@ -318,6 +321,9 @@ private: ceph::buffer::ptr value_as_ptr() override { return generic_iter->value_as_ptr(); } + std::string_view value_as_sv() override { + return generic_iter->value_as_sv(); + } int status() override { return generic_iter->status(); } diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc index a653fa6398c..e553e199c87 100644 --- a/src/kv/RocksDBStore.cc +++ b/src/kv/RocksDBStore.cc @@ -2249,6 +2249,12 @@ bufferptr RocksDBStore::RocksDBWholeSpaceIteratorImpl::value_as_ptr() return bufferptr(val.data(), val.size()); } +std::string_view RocksDBStore::RocksDBWholeSpaceIteratorImpl::value_as_sv() +{ + rocksdb::Slice val = dbiter->value(); + return std::string_view{val.data(), val.size()}; +} + int RocksDBStore::RocksDBWholeSpaceIteratorImpl::status() { return dbiter->status().ok() ? 0 : -1; @@ -2340,6 +2346,10 @@ public: rocksdb::Slice val = dbiter->value(); return bufferptr(val.data(), val.size()); } + std::string_view value_as_sv() override { + rocksdb::Slice val = dbiter->value(); + return std::string_view{val.data(), val.size()}; + } int status() override { return dbiter->status().ok() ? 0 : -1; } @@ -2677,6 +2687,15 @@ public: } } + std::string_view value_as_sv() override + { + if (smaller == on_main) { + return main->value_as_sv(); + } else { + return current_shard->second->value_as_sv(); + } + } + int status() override { //because we already had to inspect key, it must be ok @@ -3009,6 +3028,10 @@ public: rocksdb::Slice val = iters[0]->value(); return bufferptr(val.data(), val.size()); } + std::string_view value_as_sv() override { + rocksdb::Slice val = iters[0]->value(); + return std::string_view{val.data(), val.size()}; + } int status() override { return iters[0]->status().ok() ? 0 : -1; } diff --git a/src/kv/RocksDBStore.h b/src/kv/RocksDBStore.h index a8468a25d4d..24330a1c78d 100644 --- a/src/kv/RocksDBStore.h +++ b/src/kv/RocksDBStore.h @@ -384,6 +384,7 @@ public: bool raw_key_is_prefixed(const std::string &prefix) override; ceph::bufferlist value() override; ceph::bufferptr value_as_ptr() override; + std::string_view value_as_sv() override; int status() override; size_t key_size() override; size_t value_size() override; diff --git a/src/os/DBObjectMap.cc b/src/os/DBObjectMap.cc index 7da9a67be62..65627b5f818 100644 --- a/src/os/DBObjectMap.cc +++ b/src/os/DBObjectMap.cc @@ -519,6 +519,11 @@ bufferlist DBObjectMap::DBObjectMapIteratorImpl::value() return cur_iter->value(); } +std::string_view DBObjectMap::DBObjectMapIteratorImpl::value_as_sv() +{ + return cur_iter->value_as_sv(); +} + int DBObjectMap::DBObjectMapIteratorImpl::status() { return r; diff --git a/src/os/DBObjectMap.h b/src/os/DBObjectMap.h index 444f21eb815..1e1452010e7 100644 --- a/src/os/DBObjectMap.h +++ b/src/os/DBObjectMap.h @@ -393,6 +393,7 @@ private: int next() override { ceph_abort(); return 0; } std::string key() override { ceph_abort(); return ""; } ceph::buffer::list value() override { ceph_abort(); return ceph::buffer::list(); } + std::string_view value_as_sv() override { ceph_abort(); return std::string_view(); } int status() override { return 0; } }; @@ -431,6 +432,7 @@ private: int next() override; std::string key() override; ceph::buffer::list value() override; + std::string_view value_as_sv() override; int status() override; bool on_parent() { diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 4597b132b5c..7c263397153 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -5612,6 +5612,13 @@ bufferlist BlueStore::OmapIteratorImpl::value() return it->value(); } +std::string_view BlueStore::OmapIteratorImpl::value_as_sv() +{ + std::shared_lock l(c->lock); + ceph_assert(it->valid()); + return it->value_as_sv(); +} + // ===================================== diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index cf8222f53e0..06dc691b7f1 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -1710,6 +1710,7 @@ private: int next() override; std::string key() override; ceph::buffer::list value() override; + std::string_view value_as_sv() override; std::string tail_key() override { return tail; } diff --git a/src/os/kstore/KStore.cc b/src/os/kstore/KStore.cc index 7158486ca38..9cd1268023f 100644 --- a/src/os/kstore/KStore.cc +++ b/src/os/kstore/KStore.cc @@ -1651,6 +1651,13 @@ bufferlist KStore::OmapIteratorImpl::value() return it->value(); } +std::string_view KStore::OmapIteratorImpl::value_as_sv() +{ + std::shared_lock l{c->lock}; + ceph_assert(it->valid()); + return it->value_as_sv(); +} + int KStore::omap_get( CollectionHandle& ch, ///< [in] Collection containing oid const ghobject_t &oid, ///< [in] Object containing omap diff --git a/src/os/kstore/KStore.h b/src/os/kstore/KStore.h index 9a9d413c66a..4cef5b1039a 100644 --- a/src/os/kstore/KStore.h +++ b/src/os/kstore/KStore.h @@ -180,6 +180,7 @@ public: int next() override; std::string key() override; ceph::buffer::list value() override; + std::string_view value_as_sv() override; int status() override { return 0; } diff --git a/src/os/memstore/MemStore.cc b/src/os/memstore/MemStore.cc index 89cb09361cf..db8969b82c7 100644 --- a/src/os/memstore/MemStore.cc +++ b/src/os/memstore/MemStore.cc @@ -622,6 +622,10 @@ public: std::lock_guard lock{o->omap_mutex}; return it->second; } + std::string_view value_as_sv() override { + std::lock_guard lock{o->omap_mutex}; + return std::string_view{it->second.c_str(), it->second.length()}; + } int status() override { return 0; } diff --git a/src/test/ObjectMap/KeyValueDBMemory.cc b/src/test/ObjectMap/KeyValueDBMemory.cc index 234e963397e..1dd9070d028 100644 --- a/src/test/ObjectMap/KeyValueDBMemory.cc +++ b/src/test/ObjectMap/KeyValueDBMemory.cc @@ -150,6 +150,13 @@ public: return bufferlist(); } + std::string_view value_as_sv() override { + if (valid()) + return std::string_view{it->second.c_str(), it->second.length()}; + else + return std::string_view(); + } + int status() override { return 0; } -- 2.39.5