From a9866b721beb6286c478055226eb68f234169378 Mon Sep 17 00:00:00 2001 From: Abhishek Kona Date: Mon, 25 Feb 2013 13:58:34 -0800 Subject: [PATCH] Refactor statistics. Remove individual functions like incNumFileOpens Summary: Use only the counter mechanism. Do away with incNumFileOpens, incNumFileClose, incNumFileErrors s/NULL/nullptr/g in db/table_cache.cc Test Plan: make clean check Reviewers: dhruba, heyongqiang, emayanke Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D8841 --- db/db_bench.cc | 6 +++--- db/db_statistics.h | 16 ---------------- db/table_cache.cc | 32 +++++++++++++++----------------- include/leveldb/statistics.h | 21 ++++----------------- tools/db_stress.cc | 6 +++--- 5 files changed, 25 insertions(+), 56 deletions(-) diff --git a/db/db_bench.cc b/db/db_bench.cc index 4e9d9a905..2bcc8032c 100644 --- a/db/db_bench.cc +++ b/db/db_bench.cc @@ -629,9 +629,9 @@ class Benchmark { "Bloom Filter Useful: %ld \n" "Compaction key_drop_newer_entry: %ld key_drop_obsolete: %ld " "Compaction key_drop_user: %ld\n", - dbstats->getNumFileOpens(), - dbstats->getNumFileCloses(), - dbstats->getNumFileErrors(), + dbstats->getTickerCount(NO_FILE_OPENS), + dbstats->getTickerCount(NO_FILE_CLOSES), + dbstats->getTickerCount(NO_FILE_ERRORS), dbstats->getTickerCount(BLOCK_CACHE_HIT), dbstats->getTickerCount(BLOCK_CACHE_MISS), dbstats->getTickerCount(BLOOM_FILTER_USEFUL), diff --git a/db/db_statistics.h b/db/db_statistics.h index 76f3435f1..de61edd5e 100644 --- a/db/db_statistics.h +++ b/db/db_statistics.h @@ -24,21 +24,6 @@ class DBStatistics: public Statistics { virtual ~DBStatistics() {} - virtual void incNumFileOpens() { - MutexLock l(&mu_); - numFileOpens_++; - } - - virtual void incNumFileCloses() { - MutexLock l(&mu_); - numFileCloses_++; - } - - virtual void incNumFileErrors() { - MutexLock l(&mu_); - numFileErrors_++; - } - virtual long getTickerCount(Tickers tickerType) { assert(tickerType < TICKER_ENUM_MAX); return allTickers_[tickerType].getCount(); @@ -65,7 +50,6 @@ class DBStatistics: public Statistics { allHistograms_[histogramType].Data(data); } - port::Mutex mu_; std::vector allTickers_; std::vector allHistograms_; }; diff --git a/db/table_cache.cc b/db/table_cache.cc index 1fd949c44..9311bffa9 100644 --- a/db/table_cache.cc +++ b/db/table_cache.cc @@ -5,9 +5,10 @@ #include "db/table_cache.h" #include "db/filename.h" -#include "db/db_statistics.h" + #include "leveldb/env.h" #include "leveldb/table.h" +#include "leveldb/statistics.h" #include "util/coding.h" namespace leveldb { @@ -17,13 +18,11 @@ struct TableAndFile { unique_ptr table; }; -static class DBStatistics* dbstatistics; +static class Statistics* dbstatistics; static void DeleteEntry(const Slice& key, void* value) { TableAndFile* tf = reinterpret_cast(value); - if (dbstatistics) { - dbstatistics->incNumFileCloses(); - } + RecordTick(dbstatistics, NO_FILE_CLOSES); delete tf; } @@ -40,7 +39,7 @@ TableCache::TableCache(const std::string& dbname, dbname_(dbname), options_(options), cache_(NewLRUCache(entries, options->table_cache_numshardbits)) { - dbstatistics = (DBStatistics*)options->statistics; + dbstatistics = options->statistics; } TableCache::~TableCache() { @@ -52,24 +51,23 @@ Status TableCache::FindTable(uint64_t file_number, uint64_t file_size, char buf[sizeof(file_number)]; EncodeFixed64(buf, file_number); Slice key(buf, sizeof(buf)); - DBStatistics* stats = (DBStatistics*) options_->statistics; *handle = cache_->Lookup(key); - if (*handle == NULL) { - if (tableIO != NULL) { + if (*handle == nullptr) { + if (tableIO != nullptr) { *tableIO = true; // we had to do IO from storage } std::string fname = TableFileName(dbname_, file_number); unique_ptr file; unique_ptr
table; s = env_->NewRandomAccessFile(fname, &file); - stats ? stats->incNumFileOpens() : (void)0; + RecordTick(options_->statistics, NO_FILE_OPENS); if (s.ok()) { s = Table::Open(*options_, std::move(file), file_size, &table); } if (!s.ok()) { - assert(table == NULL); - stats ? stats->incNumFileErrors() : (void)0; + assert(table == nullptr); + RecordTick(options_->statistics, NO_FILE_ERRORS); // We do not cache error results so that if the error is transient, // or somebody repairs the file, we recover automatically. } else { @@ -86,11 +84,11 @@ Iterator* TableCache::NewIterator(const ReadOptions& options, uint64_t file_number, uint64_t file_size, Table** tableptr) { - if (tableptr != NULL) { - *tableptr = NULL; + if (tableptr != nullptr) { + *tableptr = nullptr; } - Cache::Handle* handle = NULL; + Cache::Handle* handle = nullptr; Status s = FindTable(file_number, file_size, &handle); if (!s.ok()) { return NewErrorIterator(s); @@ -100,7 +98,7 @@ Iterator* TableCache::NewIterator(const ReadOptions& options, reinterpret_cast(cache_->Value(handle))->table.get(); Iterator* result = table->NewIterator(options); result->RegisterCleanup(&UnrefEntry, cache_.get(), handle); - if (tableptr != NULL) { + if (tableptr != nullptr) { *tableptr = table; } return result; @@ -113,7 +111,7 @@ Status TableCache::Get(const ReadOptions& options, void* arg, void (*saver)(void*, const Slice&, const Slice&, bool), bool* tableIO) { - Cache::Handle* handle = NULL; + Cache::Handle* handle = nullptr; Status s = FindTable(file_number, file_size, &handle, tableIO); if (s.ok()) { Table* t = diff --git a/include/leveldb/statistics.h b/include/leveldb/statistics.h index fe983ba48..db441c0f3 100644 --- a/include/leveldb/statistics.h +++ b/include/leveldb/statistics.h @@ -36,7 +36,10 @@ enum Tickers { // Bytes written / read BYTES_WRITTEN = 8, BYTES_READ = 9, - TICKER_ENUM_MAX = 10, + NO_FILE_CLOSES = 10, + NO_FILE_OPENS = 11, + NO_FILE_ERRORS = 12, + TICKER_ENUM_MAX = 13, }; @@ -110,18 +113,6 @@ class Ticker { // Analyze the performance of a db class Statistics { public: - // Create an Statistics object with default values for all fields. - Statistics() : numFileOpens_(0), numFileCloses_(0), - numFileErrors_(0) {} - - virtual void incNumFileOpens() = 0; - virtual void incNumFileCloses() = 0; - virtual void incNumFileErrors() = 0; - - virtual long getNumFileOpens() { return numFileOpens_;} - virtual long getNumFileCloses() { return numFileCloses_;} - virtual long getNumFileErrors() { return numFileErrors_;} - ~Statistics() {} virtual long getTickerCount(Tickers tickerType) = 0; virtual void recordTick(Tickers tickerType, uint64_t count = 0) = 0; @@ -130,10 +121,6 @@ class Statistics { virtual void histogramData(Histograms type, HistogramData * const data) = 0; - protected: - long numFileOpens_; - long numFileCloses_; - long numFileErrors_; }; // Ease of Use functions diff --git a/tools/db_stress.cc b/tools/db_stress.cc index 900b6f38c..aeaafb42e 100644 --- a/tools/db_stress.cc +++ b/tools/db_stress.cc @@ -918,9 +918,9 @@ class StressTest { void PrintStatistics() { if (dbstats) { fprintf(stdout, "File opened:%ld closed:%ld errors:%ld\n", - dbstats->getNumFileOpens(), - dbstats->getNumFileCloses(), - dbstats->getNumFileErrors()); + dbstats->getTickerCount(NO_FILE_OPENS), + dbstats->getTickerCount(NO_FILE_CLOSES), + dbstats->getTickerCount(NO_FILE_ERRORS)); } }