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

@@ -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->getTickerCount(NO_FILE_OPENS),
-          dbstats->getNumFileCloses(),
+          dbstats->getTickerCount(NO_FILE_CLOSES),
-          dbstats->getNumFileErrors(),
+          dbstats->getTickerCount(NO_FILE_ERRORS),
           dbstats->getTickerCount(BLOCK_CACHE_HIT),
           dbstats->getTickerCount(BLOCK_CACHE_MISS),
           dbstats->getTickerCount(BLOOM_FILTER_USEFUL),

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<Ticker> allTickers_;
   std::vector<HistogramImpl> allHistograms_;
 };

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> table;
 };
 
-static class DBStatistics* dbstatistics;
+static class Statistics* dbstatistics;
 
 static void DeleteEntry(const Slice& key, void* value) {
   TableAndFile* tf = reinterpret_cast<TableAndFile*>(value);
-  if (dbstatistics) {
+  RecordTick(dbstatistics, NO_FILE_CLOSES);
-    dbstatistics->incNumFileCloses();
-  }
   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 (*handle == nullptr) {
-    if (tableIO != NULL) {
+    if (tableIO != nullptr) {
       *tableIO = true;    // we had to do IO from storage
     }
     std::string fname = TableFileName(dbname_, file_number);
     unique_ptr<RandomAccessFile> file;
     unique_ptr<Table> 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);
+      assert(table == nullptr);
-      stats ? stats->incNumFileErrors() : (void)0;
+      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) {
+  if (tableptr != nullptr) {
-    *tableptr = NULL;
+    *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<TableAndFile*>(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 =

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

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->getTickerCount(NO_FILE_OPENS),
-              dbstats->getNumFileCloses(),
+              dbstats->getTickerCount(NO_FILE_CLOSES),
-              dbstats->getNumFileErrors());
+              dbstats->getTickerCount(NO_FILE_ERRORS));
     }
   }