From b405cb886bd975db72d982a8f51f335ea304d049 Mon Sep 17 00:00:00 2001 From: sdong Date: Mon, 7 Apr 2014 16:56:26 -0700 Subject: [PATCH] PlainTableIterator not to store copied key in std::string Summary: Move PlainTableIterator's copied key from std::string local buffer to avoid paying the extra costs in std::string related to sharing. Reuse the same buffer class in DbIter. Move the class to dbformat.h. This patch improves iterator performance significantly. Running this benchmark: ./table_reader_bench --num_keys2=17 --iterator --plain_table --time_unit=nanosecond The average latency is improved to about 750 nanoseconds from 1100 nanoseconds. Test Plan: Add a unit test. make all check Reviewers: haobo, ljin Reviewed By: haobo CC: igor, yhchiang, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D17547 --- db/db_iter.cc | 67 +---------------------------------- db/dbformat.h | 70 +++++++++++++++++++++++++++++++++++++ db/plain_table_db_test.cc | 42 ++++++++++++++++++++++ table/plain_table_reader.cc | 9 ++--- 4 files changed, 116 insertions(+), 72 deletions(-) diff --git a/db/db_iter.cc b/db/db_iter.cc index 5329e5297..1b1aa539f 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -39,71 +39,6 @@ static void DumpInternalIter(Iterator* iter) { namespace { -class IterLookupKey { - public: - IterLookupKey() : key_(space_), buf_size_(sizeof(space_)), key_size_(0) {} - - ~IterLookupKey() { Clear(); } - - Slice GetKey() const { - if (key_ != nullptr) { - return Slice(key_, key_size_); - } else { - return Slice(); - } - } - - bool Valid() const { return key_ != nullptr; } - - void Clear() { - if (key_ != nullptr && key_ != space_) { - delete[] key_; - } - key_ = space_; - buf_size_ = sizeof(buf_size_); - } - - // Enlarge the buffer size if needed based on key_size. - // By default, static allocated buffer is used. Once there is a key - // larger than the static allocated buffer, another buffer is dynamically - // allocated, until a larger key buffer is requested. In that case, we - // reallocate buffer and delete the old one. - void EnlargeBufferIfNeeded(size_t key_size) { - // If size is smaller than buffer size, continue using current buffer, - // or the static allocated one, as default - if (key_size > buf_size_) { - // Need to enlarge the buffer. - Clear(); - key_ = new char[key_size]; - buf_size_ = key_size; - } - key_size_ = key_size; - } - - void SetUserKey(const Slice& user_key) { - size_t size = user_key.size(); - EnlargeBufferIfNeeded(size); - memcpy(key_, user_key.data(), size); - } - - void SetInternalKey(const Slice& user_key, SequenceNumber s) { - size_t usize = user_key.size(); - EnlargeBufferIfNeeded(usize + sizeof(uint64_t)); - memcpy(key_, user_key.data(), usize); - EncodeFixed64(key_ + usize, PackSequenceAndType(s, kValueTypeForSeek)); - } - - private: - char* key_; - size_t buf_size_; - size_t key_size_; - char space_[32]; // Avoid allocation for short keys - - // No copying allowed - IterLookupKey(const IterLookupKey&) = delete; - void operator=(const LookupKey&) = delete; -}; - // Memtables and sstables that make the DB representation contain // (userkey,seq,type) => uservalue entries. DBIter // combines multiple entries for the same userkey found in the DB @@ -191,7 +126,7 @@ class DBIter: public Iterator { SequenceNumber const sequence_; Status status_; - IterLookupKey saved_key_; // == current key when direction_==kReverse + IterKey saved_key_; // == current key when direction_==kReverse std::string saved_value_; // == current raw value when direction_==kReverse std::string skip_key_; Direction direction_; diff --git a/db/dbformat.h b/db/dbformat.h index 3c5ea6958..06341be63 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -235,4 +235,74 @@ inline LookupKey::~LookupKey() { if (start_ != space_) delete[] start_; } +class IterKey { + public: + IterKey() : key_(space_), buf_size_(sizeof(space_)), key_size_(0) {} + + ~IterKey() { Clear(); } + + Slice GetKey() const { + if (key_ != nullptr) { + return Slice(key_, key_size_); + } else { + return Slice(); + } + } + + bool Valid() const { return key_ != nullptr; } + + void Clear() { + if (key_ != nullptr && key_ != space_) { + delete[] key_; + } + key_ = space_; + buf_size_ = sizeof(buf_size_); + } + + // Enlarge the buffer size if needed based on key_size. + // By default, static allocated buffer is used. Once there is a key + // larger than the static allocated buffer, another buffer is dynamically + // allocated, until a larger key buffer is requested. In that case, we + // reallocate buffer and delete the old one. + void EnlargeBufferIfNeeded(size_t key_size) { + // If size is smaller than buffer size, continue using current buffer, + // or the static allocated one, as default + if (key_size > buf_size_) { + // Need to enlarge the buffer. + Clear(); + key_ = new char[key_size]; + buf_size_ = key_size; + } + key_size_ = key_size; + } + + void SetUserKey(const Slice& user_key) { + size_t size = user_key.size(); + EnlargeBufferIfNeeded(size); + memcpy(key_, user_key.data(), size); + } + + void SetInternalKey(const Slice& user_key, SequenceNumber s, + ValueType value_type = kValueTypeForSeek) { + size_t usize = user_key.size(); + EnlargeBufferIfNeeded(usize + sizeof(uint64_t)); + memcpy(key_, user_key.data(), usize); + EncodeFixed64(key_ + usize, PackSequenceAndType(s, value_type)); + } + + void SetInternalKey(const ParsedInternalKey& parsed_key) { + SetInternalKey(parsed_key.user_key, parsed_key.sequence, parsed_key.type); + } + + private: + char* key_; + size_t buf_size_; + size_t key_size_; + char space_[32]; // Avoid allocation for short keys + + // No copying allowed + IterKey(const IterKey&) = delete; + void operator=(const IterKey&) = delete; +}; + } // namespace rocksdb diff --git a/db/plain_table_db_test.cc b/db/plain_table_db_test.cc index 4f1563b94..6a95a2585 100644 --- a/db/plain_table_db_test.cc +++ b/db/plain_table_db_test.cc @@ -429,6 +429,48 @@ TEST(PlainTableDBTest, Iterator) { } } +std::string MakeLongKey(size_t length, char c) { + return std::string(length, c); +} + +TEST(PlainTableDBTest, IteratorLargeKeys) { + Options options = CurrentOptions(); + options.table_factory.reset(NewTotalOrderPlainTableFactory(0, 0, 16)); + options.create_if_missing = true; + options.prefix_extractor.reset(); + DestroyAndReopen(&options); + + std::string key_list[] = { + MakeLongKey(30, '0'), + MakeLongKey(16, '1'), + MakeLongKey(32, '2'), + MakeLongKey(60, '3'), + MakeLongKey(90, '4'), + MakeLongKey(50, '5'), + MakeLongKey(26, '6') + }; + + for (size_t i = 0; i < 7; i++) { + ASSERT_OK(Put(key_list[i], std::to_string(i))); + } + + dbfull()->TEST_FlushMemTable(); + + Iterator* iter = dbfull()->NewIterator(ro_); + iter->Seek(key_list[0]); + + for (size_t i = 0; i < 7; i++) { + ASSERT_TRUE(iter->Valid()); + ASSERT_EQ(key_list[i], iter->key().ToString()); + ASSERT_EQ(std::to_string(i), iter->value().ToString()); + iter->Next(); + } + + ASSERT_TRUE(!iter->Valid()); + + delete iter; +} + // A test comparator which compare two strings in this way: // (1) first compare prefix of 8 bytes in alphabet order, // (2) if two strings share the same prefix, sort the other part of the string diff --git a/table/plain_table_reader.cc b/table/plain_table_reader.cc index a6f8bed0e..436d13bf3 100644 --- a/table/plain_table_reader.cc +++ b/table/plain_table_reader.cc @@ -81,10 +81,9 @@ class PlainTableIterator : public Iterator { bool use_prefix_seek_; uint32_t offset_; uint32_t next_offset_; - Slice key_; + IterKey key_; Slice value_; Status status_; - std::string tmp_str_; // No copying allowed PlainTableIterator(const PlainTableIterator&) = delete; void operator=(const Iterator&) = delete; @@ -720,9 +719,7 @@ void PlainTableIterator::Next() { status_ = table_->Next(&next_offset_, &parsed_key, &value_); if (status_.ok()) { // Make a copy in this case. TODO optimize. - tmp_str_.clear(); - AppendInternalKey(&tmp_str_, parsed_key); - key_ = Slice(tmp_str_); + key_.SetInternalKey(parsed_key); } else { offset_ = next_offset_ = table_->data_end_offset_; } @@ -735,7 +732,7 @@ void PlainTableIterator::Prev() { Slice PlainTableIterator::key() const { assert(Valid()); - return key_; + return key_.GetKey(); } Slice PlainTableIterator::value() const {