From 774ed89c2405ee058086b099cbc8b29e243739cc Mon Sep 17 00:00:00 2001 From: Kai Liu Date: Thu, 2 Jan 2014 11:26:57 -0800 Subject: [PATCH] Replace vector with autovector Summary: this diff only replace the cases when we need to frequently create vector with small amount of entries. This diff doesn't aim to improve performance of a specific area, but more like a small scale test for the autovector and see how it works in real life. Test Plan: make check I also ran the performance tests, however there is no performance gain/loss. All performance numbers are pretty much the same before/after the change. Reviewers: dhruba, haobo, sdong, igor CC: leveldb Differential Revision: https://reviews.facebook.net/D14985 --- db/db_impl.cc | 56 ++++++++++++++++++++---------------------- db/db_impl.h | 20 +++++++-------- db/memtable_list.cc | 10 ++++---- db/memtable_list.h | 18 ++++++++------ util/autovector.h | 60 +++++++++++++-------------------------------- util/cache.cc | 5 ++-- 6 files changed, 70 insertions(+), 99 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 093112857..2b70ff60f 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -47,6 +47,7 @@ #include "table/block_based_table_factory.h" #include "table/merger.h" #include "table/two_level_iterator.h" +#include "util/autovector.h" #include "util/auto_roll_logger.h" #include "util/build_version.h" #include "util/coding.h" @@ -299,8 +300,7 @@ DBImpl::DBImpl(const Options& options, const std::string& dbname) } DBImpl::~DBImpl() { - std::vector to_delete; - to_delete.reserve(options_.max_write_buffer_number); + autovector to_delete; // Wait for background work to finish if (flush_on_destroy_ && mem_->GetFirstSequenceNumber() != 0) { @@ -455,10 +455,6 @@ void DBImpl::MaybeDumpStats() { } // DBImpl::SuperVersion methods -DBImpl::SuperVersion::SuperVersion(const int num_memtables) { - to_delete.resize(num_memtables); -} - DBImpl::SuperVersion::~SuperVersion() { for (auto td : to_delete) { delete td; @@ -1114,7 +1110,7 @@ Status DBImpl::WriteLevel0TableForRecovery(MemTable* mem, VersionEdit* edit) { } -Status DBImpl::WriteLevel0Table(std::vector &mems, VersionEdit* edit, +Status DBImpl::WriteLevel0Table(autovector& mems, VersionEdit* edit, uint64_t* filenumber) { mutex_.AssertHeld(); const uint64_t start_micros = env_->NowMicros(); @@ -1131,15 +1127,15 @@ Status DBImpl::WriteLevel0Table(std::vector &mems, VersionEdit* edit, Status s; { mutex_.Unlock(); - std::vector list; + std::vector memtables; for (MemTable* m : mems) { Log(options_.info_log, "Flushing memtable with log file: %lu\n", (unsigned long)m->GetLogNumber()); - list.push_back(m->NewIterator()); + memtables.push_back(m->NewIterator()); } - Iterator* iter = NewMergingIterator(env_, &internal_comparator_, &list[0], - list.size()); + Iterator* iter = NewMergingIterator( + env_, &internal_comparator_, &memtables[0], memtables.size()); Log(options_.info_log, "Level-0 flush table #%lu: started", (unsigned long)meta.number); @@ -1214,7 +1210,7 @@ Status DBImpl::FlushMemTableToOutputFile(bool* madeProgress, // Save the contents of the earliest memtable as a new Table uint64_t file_number; - std::vector mems; + autovector mems; imm_.PickMemtablesToFlush(&mems); if (mems.empty()) { Log(options_.info_log, "Nothing in memstore to flush"); @@ -1316,8 +1312,7 @@ void DBImpl::ReFitLevel(int level, int target_level) { assert(level < NumberLevels()); SuperVersion* superversion_to_free = nullptr; - SuperVersion* new_superversion = - new SuperVersion(options_.max_write_buffer_number); + SuperVersion* new_superversion = new SuperVersion(); mutex_.Lock(); @@ -1750,7 +1745,7 @@ Status DBImpl::BackgroundFlush(bool* madeProgress, void DBImpl::BackgroundCallFlush() { bool madeProgress = false; - DeletionState deletion_state(options_.max_write_buffer_number, true); + DeletionState deletion_state(true); assert(bg_flush_scheduled_); MutexLock l(&mutex_); @@ -1796,7 +1791,7 @@ void DBImpl::TEST_PurgeObsoleteteWAL() { void DBImpl::BackgroundCallCompaction() { bool madeProgress = false; - DeletionState deletion_state(options_.max_write_buffer_number, true); + DeletionState deletion_state(true); MaybeDumpStats(); @@ -2591,16 +2586,16 @@ namespace { struct IterState { port::Mutex* mu; Version* version; - std::vector mem; // includes both mem_ and imm_ + autovector mem; // includes both mem_ and imm_ DBImpl *db; }; static void CleanupIteratorState(void* arg1, void* arg2) { IterState* state = reinterpret_cast(arg1); - DBImpl::DeletionState deletion_state(state->db->GetOptions(). - max_write_buffer_number); + DBImpl::DeletionState deletion_state; state->mu->Lock(); - for (unsigned int i = 0; i < state->mem.size(); i++) { + auto mems_size = state->mem.size(); + for (size_t i = 0; i < mems_size; i++) { MemTable* m = state->mem[i]->Unref(); if (m != nullptr) { deletion_state.memtables_to_free.push_back(m); @@ -2620,7 +2615,7 @@ Iterator* DBImpl::NewInternalIterator(const ReadOptions& options, SequenceNumber* latest_snapshot) { IterState* cleanup = new IterState; MemTable* mutable_mem; - std::vector immutables; + autovector immutables; Version* version; // Collect together all needed child iterators for mem @@ -2638,16 +2633,17 @@ Iterator* DBImpl::NewInternalIterator(const ReadOptions& options, version = versions_->current(); mutex_.Unlock(); - std::vector list; - list.push_back(mutable_mem->NewIterator(options)); + std::vector memtables; + memtables.push_back(mutable_mem->NewIterator(options)); cleanup->mem.push_back(mutable_mem); for (MemTable* m : immutables) { - list.push_back(m->NewIterator(options)); + memtables.push_back(m->NewIterator(options)); cleanup->mem.push_back(m); } - version->AddIterators(options, storage_options_, &list); - Iterator* internal_iter = - NewMergingIterator(env_, &internal_comparator_, &list[0], list.size()); + version->AddIterators(options, storage_options_, &memtables); + Iterator* internal_iter = NewMergingIterator( + env_, &internal_comparator_, memtables.data(), memtables.size() + ); cleanup->version = version; cleanup->mu = &mutex_; cleanup->db = this; @@ -2802,7 +2798,7 @@ std::vector DBImpl::MultiGet(const ReadOptions& options, StartPerfTimer(&snapshot_timer); SequenceNumber snapshot; - std::vector to_delete; + autovector to_delete; mutex_.Lock(); if (options.snapshot != nullptr) { @@ -3322,7 +3318,7 @@ Status DBImpl::MakeRoomForWrite(bool force, lfile->SetPreallocationBlockSize(1.1 * options_.write_buffer_size); new_mem = new MemTable( internal_comparator_, mem_rep_factory_, NumberLevels(), options_); - new_superversion = new SuperVersion(options_.max_write_buffer_number); + new_superversion = new SuperVersion(); } } mutex_.Lock(); @@ -3703,7 +3699,7 @@ Status DBImpl::DeleteFile(std::string name) { FileMetaData metadata; int maxlevel = NumberLevels(); VersionEdit edit(maxlevel); - DeletionState deletion_state(0, true); + DeletionState deletion_state(true); { MutexLock l(&mutex_); status = versions_->GetMetadataForFile(number, &level, &metadata); diff --git a/db/db_impl.h b/db/db_impl.h index 3b02ee9b1..b8056edd5 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -7,6 +7,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. See the AUTHORS file for names of contributors. #pragma once + #include #include #include @@ -16,13 +17,14 @@ #include "db/log_writer.h" #include "db/snapshot.h" #include "db/version_edit.h" +#include "memtable_list.h" +#include "port/port.h" #include "rocksdb/db.h" #include "rocksdb/env.h" #include "rocksdb/memtablerep.h" #include "rocksdb/transaction_log.h" -#include "port/port.h" +#include "util/autovector.h" #include "util/stats_logger.h" -#include "memtable_list.h" namespace rocksdb { @@ -138,10 +140,10 @@ class DBImpl : public DB { // We need to_delete because during Cleanup(), imm.UnrefAll() returns // all memtables that we need to free through this vector. We then // delete all those memtables outside of mutex, during destruction - std::vector to_delete; + autovector to_delete; // should be called outside the mutex - explicit SuperVersion(const int num_memtables = 0); + SuperVersion() = default; ~SuperVersion(); SuperVersion* Ref(); // Returns true if this was the last reference and caller should @@ -180,7 +182,7 @@ class DBImpl : public DB { std::vector log_delete_files; // a list of memtables to be free - std::vector memtables_to_free; + autovector memtables_to_free; SuperVersion* superversion_to_free; // if nullptr nothing to free @@ -190,15 +192,13 @@ class DBImpl : public DB { // that corresponds to the set of files in 'live'. uint64_t manifest_file_number, log_number, prev_log_number; - explicit DeletionState(const int num_memtables = 0, - bool create_superversion = false) { + explicit DeletionState(bool create_superversion = false) { manifest_file_number = 0; log_number = 0; prev_log_number = 0; - memtables_to_free.reserve(num_memtables); superversion_to_free = nullptr; new_superversion = - create_superversion ? new SuperVersion(num_memtables) : nullptr; + create_superversion ? new SuperVersion() : nullptr; } ~DeletionState() { @@ -283,7 +283,7 @@ class DBImpl : public DB { // for the entire period. The second method WriteLevel0Table supports // concurrent flush memtables to storage. Status WriteLevel0TableForRecovery(MemTable* mem, VersionEdit* edit); - Status WriteLevel0Table(std::vector &mems, VersionEdit* edit, + Status WriteLevel0Table(autovector& mems, VersionEdit* edit, uint64_t* filenumber); uint64_t SlowdownAmount(int n, int top, int bottom); diff --git a/db/memtable_list.cc b/db/memtable_list.cc index 27e12b945..7197a92ea 100644 --- a/db/memtable_list.cc +++ b/db/memtable_list.cc @@ -31,7 +31,7 @@ void MemTableList::RefAll() { // Drop reference count on all underling memtables. If the // refcount of an underlying memtable drops to zero, then // return it in to_delete vector. -void MemTableList::UnrefAll(std::vector* to_delete) { +void MemTableList::UnrefAll(autovector* to_delete) { for (auto &memtable : memlist_) { MemTable* m = memtable->Unref(); if (m != nullptr) { @@ -58,7 +58,7 @@ bool MemTableList::IsFlushPending(int min_write_buffer_number_to_merge) { } // Returns the memtables that need to be flushed. -void MemTableList::PickMemtablesToFlush(std::vector* ret) { +void MemTableList::PickMemtablesToFlush(autovector* ret) { for (auto it = memlist_.rbegin(); it != memlist_.rend(); it++) { MemTable* m = *it; if (!m->flush_in_progress_) { @@ -76,12 +76,12 @@ void MemTableList::PickMemtablesToFlush(std::vector* ret) { // Record a successful flush in the manifest file Status MemTableList::InstallMemtableFlushResults( - const std::vector &mems, + const autovector &mems, VersionSet* vset, Status flushStatus, port::Mutex* mu, Logger* info_log, uint64_t file_number, std::set& pending_outputs, - std::vector* to_delete) { + autovector* to_delete) { mu->AssertHeld(); // If the flush was not successful, then just reset state. @@ -213,7 +213,7 @@ bool MemTableList::Get(const LookupKey& key, std::string* value, Status* s, return false; } -void MemTableList::GetMemTables(std::vector* output) { +void MemTableList::GetMemTables(autovector* output) { for (auto &memtable : memlist_) { output->push_back(memtable); } diff --git a/db/memtable_list.h b/db/memtable_list.h index ed353c8b8..9831d7621 100644 --- a/db/memtable_list.h +++ b/db/memtable_list.h @@ -3,15 +3,17 @@ // LICENSE file in the root directory of this source tree. An additional grant // of patent rights can be found in the PATENTS file in the same directory. // - #pragma once + #include #include #include -#include "rocksdb/db.h" + #include "db/dbformat.h" +#include "db/memtable.h" #include "db/skiplist.h" -#include "memtable.h" +#include "rocksdb/db.h" +#include "util/autovector.h" namespace rocksdb { @@ -47,7 +49,7 @@ class MemTableList { // Drop reference count on all underling memtables. If the refcount // on an underlying memtable drops to zero, then return it in // to_delete vector. - void UnrefAll(std::vector* to_delete); + void UnrefAll(autovector* to_delete); // Returns the total number of memtables in the list int size(); @@ -58,15 +60,15 @@ class MemTableList { // Returns the earliest memtables that needs to be flushed. The returned // memtables are guaranteed to be in the ascending order of created time. - void PickMemtablesToFlush(std::vector* mems); + void PickMemtablesToFlush(autovector* mems); // Commit a successful flush in the manifest file - Status InstallMemtableFlushResults(const std::vector &m, + Status InstallMemtableFlushResults(const autovector &m, VersionSet* vset, Status flushStatus, port::Mutex* mu, Logger* info_log, uint64_t file_number, std::set& pending_outputs, - std::vector* to_delete); + autovector* to_delete); // New memtables are inserted at the front of the list. // Takes ownership of the referenced held on *m by the caller of Add(). @@ -81,7 +83,7 @@ class MemTableList { MergeContext& merge_context, const Options& options); // Returns the list of underlying memtables. - void GetMemTables(std::vector* list); + void GetMemTables(autovector* list); // Request a flush of all existing memtables to storage void FlushRequested() { flush_requested_ = true; } diff --git a/util/autovector.h b/util/autovector.h index 9998e2956..812a61795 100644 --- a/util/autovector.h +++ b/util/autovector.h @@ -57,11 +57,9 @@ class autovector { typedef std::random_access_iterator_tag iterator_category; iterator_impl(TAutoVector* vect, size_t index) - : vect_(vect) - , index_(index) { - }; + : vect_(vect), index_(index) {}; iterator_impl(const iterator_impl&) = default; - ~iterator_impl() { } + ~iterator_impl() {} iterator_impl& operator=(const iterator_impl&) = default; // -- Advancement @@ -130,9 +128,7 @@ class autovector { return index_ == other.index_; } - bool operator!=(const self_type& other) const { - return !(*this == other); - } + bool operator!=(const self_type& other) const { return !(*this == other); } bool operator>(const self_type& other) const { assert(vect_ == other.vect_); @@ -174,13 +170,9 @@ class autovector { return vect_.capacity() == 0; } - size_type size() const { - return num_stack_items_ + vect_.size(); - } + size_type size() const { return num_stack_items_ + vect_.size(); } - bool empty() const { - return size() == 0; - } + bool empty() const { return size() == 0; } // will not check boundry const_reference operator[](size_type n) const { @@ -235,11 +227,9 @@ class autovector { } } - void push_back(const T& item) { - push_back(value_type(item)); - } + void push_back(const T& item) { push_back(value_type(item)); } - template + template void emplace_back(Args&&... args) { push_back(value_type(args...)); } @@ -261,13 +251,9 @@ class autovector { // -- Copy and Assignment autovector& assign(const autovector& other); - autovector(const autovector& other) { - assign(other); - } + autovector(const autovector& other) { assign(other); } - autovector& operator=(const autovector& other) { - return assign(other); - } + autovector& operator=(const autovector& other) { return assign(other); } // move operation are disallowed since it is very hard to make sure both // autovectors are allocated from the same function stack. @@ -275,41 +261,29 @@ class autovector { autovector(autovector&& other) = delete; // -- Iterator Operations - iterator begin() { - return iterator(this, 0); - } + iterator begin() { return iterator(this, 0); } - const_iterator begin() const { - return const_iterator(this, 0); - } + const_iterator begin() const { return const_iterator(this, 0); } - iterator end() { - return iterator(this, this->size()); - } + iterator end() { return iterator(this, this->size()); } - const_iterator end() const { - return const_iterator(this, this->size()); - } + const_iterator end() const { return const_iterator(this, this->size()); } - reverse_iterator rbegin() { - return reverse_iterator(end()); - } + reverse_iterator rbegin() { return reverse_iterator(end()); } const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } - reverse_iterator rend() { - return reverse_iterator(begin()); - } + reverse_iterator rend() { return reverse_iterator(begin()); } const_reverse_iterator rend() const { return const_reverse_iterator(begin()); } private: - size_type num_stack_items_ = 0; // current number of items - value_type values_[kSize]; // the first `kSize` items + size_type num_stack_items_ = 0; // current number of items + value_type values_[kSize]; // the first `kSize` items // used only if there are more than `kSize` items. std::vector vect_; }; diff --git a/util/cache.cc b/util/cache.cc index ddd808b41..143c6957a 100644 --- a/util/cache.cc +++ b/util/cache.cc @@ -10,10 +10,10 @@ #include #include #include -#include #include "rocksdb/cache.h" #include "port/port.h" +#include "util/autovector.h" #include "util/hash.h" #include "util/mutexlock.h" @@ -264,8 +264,7 @@ Cache::Handle* LRUCache::Insert( LRUHandle* e = reinterpret_cast( malloc(sizeof(LRUHandle)-1 + key.size())); - std::vector last_reference_list; - last_reference_list.reserve(1); + autovector last_reference_list; e->value = value; e->deleter = deleter;