From 256672f89e890ebdcaa25357d881ced6316ad0e5 Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 13:44:25 +0800 Subject: [PATCH 1/8] dbformat.h: IterKey: use getter `buf()` instead of `buf_` --- db/dbformat.h | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/db/dbformat.h b/db/dbformat.h index d9fadea1ca9..62f56a0a63f 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -483,13 +483,13 @@ class IterKey { if (IsKeyPinned() /* key is not in buf_ */) { // Copy the key from external memory to buf_ (copy shared_len bytes) EnlargeBufferIfNeeded(total_size); - memcpy(buf_, key_, shared_len); + memcpy(buf(), key_, shared_len); } else if (total_size > buf_size_) { // Need to allocate space, delete previous space char* p = new char[total_size]; memcpy(p, key_, shared_len); - if (buf_ != space_) { + if (buf_size_ != sizeof(space_)) { delete[] buf_; } @@ -497,8 +497,8 @@ class IterKey { buf_size_ = total_size; } - memcpy(buf_ + shared_len, non_shared_data, non_shared_len); - key_ = buf_; + memcpy(buf() + shared_len, non_shared_data, non_shared_len); + key_ = buf(); key_size_ = total_size; } @@ -535,8 +535,8 @@ class IterKey { assert(IsKeyPinned() == true); Reserve(key_size_); - memcpy(buf_, key_, key_size_); - key_ = buf_; + memcpy(buf(), key_, key_size_); + key_ = buf(); } // Update the sequence number in the internal key. Guarantees not to @@ -546,14 +546,14 @@ class IterKey { assert(key_size_ >= kNumInternalBytes); if (ts) { assert(key_size_ >= kNumInternalBytes + ts->size()); - memcpy(&buf_[key_size_ - kNumInternalBytes - ts->size()], ts->data(), + memcpy(&buf()[key_size_ - kNumInternalBytes - ts->size()], ts->data(), ts->size()); } uint64_t newval = (seq << 8) | t; - EncodeFixed64(&buf_[key_size_ - kNumInternalBytes], newval); + EncodeFixed64(&buf()[key_size_ - kNumInternalBytes], newval); } - bool IsKeyPinned() const { return (key_ != buf_); } + bool IsKeyPinned() const { return (key_ != buf()); } // If `ts` is provided, user_key should not contain timestamp, // and `ts` is appended after user_key. @@ -567,16 +567,16 @@ class IterKey { size_t ts_sz = (ts != nullptr ? ts->size() : 0); EnlargeBufferIfNeeded(psize + usize + sizeof(uint64_t) + ts_sz); if (psize > 0) { - memcpy(buf_, key_prefix.data(), psize); + memcpy(buf(), key_prefix.data(), psize); } - memcpy(buf_ + psize, user_key.data(), usize); + memcpy(buf() + psize, user_key.data(), usize); if (ts) { - memcpy(buf_ + psize + usize, ts->data(), ts_sz); + memcpy(buf() + psize + usize, ts->data(), ts_sz); } - EncodeFixed64(buf_ + usize + psize + ts_sz, + EncodeFixed64(buf() + usize + psize + ts_sz, PackSequenceAndType(s, value_type)); - key_ = buf_; + key_ = buf(); key_size_ = psize + usize + sizeof(uint64_t) + ts_sz; is_user_key_ = false; } @@ -605,9 +605,9 @@ class IterKey { void EncodeLengthPrefixedKey(const Slice& key) { auto size = key.size(); EnlargeBufferIfNeeded(size + static_cast(VarintLength(size))); - char* ptr = EncodeVarint32(buf_, static_cast(size)); + char* ptr = EncodeVarint32(buf(), static_cast(size)); memcpy(ptr, key.data(), size); - key_ = buf_; + key_ = buf(); is_user_key_ = true; } @@ -621,13 +621,17 @@ class IterKey { char space_[32]; // Avoid allocation for short keys bool is_user_key_; + + char* buf() { return buf_size_ <= sizeof(space_) ? space_ : buf_ ; } + const char* buf() const { return buf_size_ <= sizeof(space_) ? space_ : buf_ ; } + Slice SetKeyImpl(const Slice& key, bool copy) { size_t size = key.size(); if (copy) { // Copy key to buf_ EnlargeBufferIfNeeded(size); - memcpy(buf_, key.data(), size); - key_ = buf_; + memcpy(buf(), key.data(), size); + key_ = buf(); } else { // Update key_ to point to external memory key_ = key.data(); From 9660728ad5f841156a0a33c35e6ac0ae362ac186 Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 13:46:10 +0800 Subject: [PATCH 2/8] dbformat.h: IterKey: use union{buf_, space_} to reduce memory(for CPU cache) --- db/dbformat.h | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/db/dbformat.h b/db/dbformat.h index 62f56a0a63f..94f3401ee33 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -432,8 +432,7 @@ inline uint64_t GetInternalKeySeqno(const Slice& internal_key) { class IterKey { public: IterKey() - : buf_(space_), - key_(buf_), + : key_(space_), key_size_(0), buf_size_(sizeof(space_)), is_user_key_(true) {} @@ -614,13 +613,14 @@ class IterKey { bool IsUserKey() const { return is_user_key_; } private: - char* buf_; const char* key_; - size_t key_size_; - size_t buf_size_; - char space_[32]; // Avoid allocation for short keys - bool is_user_key_; - + uint32_t key_size_; + uint32_t buf_size_ : 31; + uint32_t is_user_key_ : 1; + union { + char* buf_; + char space_[48]; // Avoid allocation for short keys + }; char* buf() { return buf_size_ <= sizeof(space_) ? space_ : buf_ ; } const char* buf() const { return buf_size_ <= sizeof(space_) ? space_ : buf_ ; } @@ -641,9 +641,8 @@ class IterKey { } void ResetBuffer() { - if (buf_ != space_) { + if (sizeof(space_) != buf_size_) { delete[] buf_; - buf_ = space_; } buf_size_ = sizeof(space_); key_size_ = 0; From 10580b970315e3bb91cecf44615eab0741b27736 Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 14:10:03 +0800 Subject: [PATCH 3/8] make format --- db/dbformat.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/db/dbformat.h b/db/dbformat.h index 94f3401ee33..7408c253cdb 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -622,8 +622,10 @@ class IterKey { char space_[48]; // Avoid allocation for short keys }; - char* buf() { return buf_size_ <= sizeof(space_) ? space_ : buf_ ; } - const char* buf() const { return buf_size_ <= sizeof(space_) ? space_ : buf_ ; } + char* buf() { return buf_size_ <= sizeof(space_) ? space_ : buf_; } + const char* buf() const { + return buf_size_ <= sizeof(space_) ? space_ : buf_; + } Slice SetKeyImpl(const Slice& key, bool copy) { size_t size = key.size(); From 949bb8b4e82ef63f4ca76a7b0da6004c17d9ad88 Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 14:43:49 +0800 Subject: [PATCH 4/8] dbformat.h: IterKey: suppress compiler false warn --- db/dbformat.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/db/dbformat.h b/db/dbformat.h index 7408c253cdb..64a0fbc6987 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -495,8 +495,15 @@ class IterKey { buf_ = p; buf_size_ = total_size; } - + #if defined(__GNUC__) + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Warray-bounds" + #pragma GCC diagnostic ignored "-Wstringop-overflow" + #endif memcpy(buf() + shared_len, non_shared_data, non_shared_len); + #if defined(__GNUC__) + #pragma GCC diagnostic pop + #endif key_ = buf(); key_size_ = total_size; } @@ -644,7 +651,14 @@ class IterKey { void ResetBuffer() { if (sizeof(space_) != buf_size_) { +#if defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif delete[] buf_; +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif } buf_size_ = sizeof(space_); key_size_ = 0; From 66235f6fb6bbd2b4e506898ee4625d50f7d4617b Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 14:45:18 +0800 Subject: [PATCH 5/8] make format --- db/dbformat.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/db/dbformat.h b/db/dbformat.h index 64a0fbc6987..5f29409c864 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -495,15 +495,15 @@ class IterKey { buf_ = p; buf_size_ = total_size; } - #if defined(__GNUC__) - #pragma GCC diagnostic push - #pragma GCC diagnostic ignored "-Warray-bounds" - #pragma GCC diagnostic ignored "-Wstringop-overflow" - #endif +#if defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds" +#pragma GCC diagnostic ignored "-Wstringop-overflow" +#endif memcpy(buf() + shared_len, non_shared_data, non_shared_len); - #if defined(__GNUC__) - #pragma GCC diagnostic pop - #endif +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif key_ = buf(); key_size_ = total_size; } From 92e61b2f583536eb94f53d1b9f772124dc46f167 Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 15:55:10 +0800 Subject: [PATCH 6/8] IterKey: fool the compiler to do not warn shorten 64 to 32 --- db/dbformat.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/dbformat.h b/db/dbformat.h index 5f29409c864..e44e150eab4 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -621,9 +621,9 @@ class IterKey { private: const char* key_; - uint32_t key_size_; - uint32_t buf_size_ : 31; - uint32_t is_user_key_ : 1; + size_t key_size_ : 32; + size_t buf_size_ : 31; + size_t is_user_key_ : 1; union { char* buf_; char space_[48]; // Avoid allocation for short keys From 9155169e45de27f726726f48d68f8eabc2bfa2b5 Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 16:04:29 +0800 Subject: [PATCH 7/8] suppress clang complain --- db/dbformat.h | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/db/dbformat.h b/db/dbformat.h index e44e150eab4..7fea53f7ed9 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -423,6 +423,18 @@ inline uint64_t GetInternalKeySeqno(const Slice& internal_key) { return num >> 8; } +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds" +#pragma GCC diagnostic ignored "-Wstringop-overflow" +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunknown-warning-option" +#pragma clang diagnostic ignored "-Warray-bounds" +#pragma clang diagnostic ignored "-Wstringop-overflow" +#pragma clang diagnostic ignored "-Wmaybe-uninitialized" +#endif // The class to store keys in an efficient way. It allows: // 1. Users can either copy the key into it, or have it point to an unowned // address. @@ -495,15 +507,7 @@ class IterKey { buf_ = p; buf_size_ = total_size; } -#if defined(__GNUC__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Warray-bounds" -#pragma GCC diagnostic ignored "-Wstringop-overflow" -#endif memcpy(buf() + shared_len, non_shared_data, non_shared_len); -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif key_ = buf(); key_size_ = total_size; } @@ -651,14 +655,7 @@ class IterKey { void ResetBuffer() { if (sizeof(space_) != buf_size_) { -#if defined(__GNUC__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" -#endif delete[] buf_; -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif } buf_size_ = sizeof(space_); key_size_ = 0; @@ -679,6 +676,11 @@ class IterKey { void EnlargeBuffer(size_t key_size); }; +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic pop +#elif defined(__clang__) +#pragma clang diagnostic pop +#endif // Convert from a SliceTransform of user keys, to a SliceTransform of // internal keys. From e8d4698030b360dac3009e3019e9700ef5404eef Mon Sep 17 00:00:00 2001 From: leipeng Date: Sat, 28 Jan 2023 16:26:46 +0800 Subject: [PATCH 8/8] Move IterKey::TrimAppend() definition to dbformat.cc --- db/dbformat.cc | 26 ++++++++++++++++++++++++++ db/dbformat.h | 25 +------------------------ 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/db/dbformat.cc b/db/dbformat.cc index 2c3581ca005..f5cd28395a2 100644 --- a/db/dbformat.cc +++ b/db/dbformat.cc @@ -201,6 +201,32 @@ LookupKey::LookupKey(const Slice& _user_key, SequenceNumber s, end_ = dst; } +void IterKey::TrimAppend(const size_t shared_len, const char* non_shared_data, + const size_t non_shared_len) { + assert(shared_len <= key_size_); + size_t total_size = shared_len + non_shared_len; + + if (IsKeyPinned() /* key is not in buf_ */) { + // Copy the key from external memory to buf_ (copy shared_len bytes) + EnlargeBufferIfNeeded(total_size); + memcpy(buf(), key_, shared_len); + } else if (total_size > buf_size_) { + // Need to allocate space, delete previous space + char* p = new char[total_size]; + memcpy(p, key_, shared_len); + + if (buf_size_ != sizeof(space_)) { + delete[] buf_; + } + + buf_ = p; + buf_size_ = total_size; + } + memcpy(buf() + shared_len, non_shared_data, non_shared_len); + key_ = buf(); + key_size_ = total_size; +} + void IterKey::EnlargeBuffer(size_t key_size) { // If size is smaller than buffer size, continue using current buffer, // or the static allocated one, as default diff --git a/db/dbformat.h b/db/dbformat.h index 7fea53f7ed9..7b86a705de0 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -487,30 +487,7 @@ class IterKey { // shared_len: bytes in [0, shard_len-1] would be remained // non_shared_data: data to be append, its length must be >= non_shared_len void TrimAppend(const size_t shared_len, const char* non_shared_data, - const size_t non_shared_len) { - assert(shared_len <= key_size_); - size_t total_size = shared_len + non_shared_len; - - if (IsKeyPinned() /* key is not in buf_ */) { - // Copy the key from external memory to buf_ (copy shared_len bytes) - EnlargeBufferIfNeeded(total_size); - memcpy(buf(), key_, shared_len); - } else if (total_size > buf_size_) { - // Need to allocate space, delete previous space - char* p = new char[total_size]; - memcpy(p, key_, shared_len); - - if (buf_size_ != sizeof(space_)) { - delete[] buf_; - } - - buf_ = p; - buf_size_ = total_size; - } - memcpy(buf() + shared_len, non_shared_data, non_shared_len); - key_ = buf(); - key_size_ = total_size; - } + const size_t non_shared_len); Slice SetKey(const Slice& key, bool copy = true) { // is_user_key_ expected to be set already via SetIsUserKey