memory manage statistics

Summary:
Earlier Statistics object was a raw pointer. This meant the user had to clear up
the Statistics object after creating the database. In most use cases the database is created in a function and the statistics pointer is out of scope. Hence the statistics object would never be deleted.
Now Using a shared_ptr to manage this.

Want this in before the next release.

Test Plan: make all check.

Reviewers: dhruba, emayanke

Reviewed By: emayanke

CC: leveldb

Differential Revision: https://reviews.facebook.net/D9735

db/db_bench.cc

@@ -156,7 +156,7 @@ static bool FLAGS_verify_checksum = false;
 
 // Database statistics
 static bool FLAGS_statistics = false;
-static class leveldb::DBStatistics* dbstats = nullptr;
+static class std::shared_ptr<leveldb::Statistics> dbstats;
 
 // Number of write operations to do.  If negative, do FLAGS_num reads.
 static long FLAGS_writes = -1;
@@ -1771,7 +1771,7 @@ int main(int argc, char** argv) {
     } else if (sscanf(argv[i], "--statistics=%d%c", &n, &junk) == 1 &&
                (n == 0 || n == 1)) {
       if (n == 1) {
-        dbstats = new leveldb::DBStatistics();
+        dbstats.reset(new leveldb::DBStatistics());
         FLAGS_statistics = true;
       }
     } else if (sscanf(argv[i], "--writes=%d%c", &n, &junk) == 1) {

db/table_cache.cc

@@ -16,11 +16,9 @@ struct TableAndFile {
   unique_ptr<Table> table;
 };
 
-static class Statistics* dbstatistics;
 
 static void DeleteEntry(const Slice& key, void* value) {
   TableAndFile* tf = reinterpret_cast<TableAndFile*>(value);
-  RecordTick(dbstatistics, NO_FILE_CLOSES);
   delete tf;
 }
 
@@ -38,9 +36,7 @@ TableCache::TableCache(const std::string& dbname,
       dbname_(dbname),
       options_(options),
       storage_options_(storage_options),
-      cache_(NewLRUCache(entries, options->table_cache_numshardbits)) {
+      cache_(NewLRUCache(entries, options->table_cache_numshardbits)) {}
-  dbstatistics = options->statistics;
-}
 
 TableCache::~TableCache() {
 }

include/leveldb/options.h

@@ -11,6 +11,7 @@
 #include <vector>
 #include <stdint.h>
 #include "leveldb/slice.h"
+#include "leveldb/statistics.h"
 
 namespace leveldb {
 
@@ -20,7 +21,6 @@ class Env;
 class FilterPolicy;
 class Logger;
 class Snapshot;
-class Statistics;
 
 using std::shared_ptr;
 
@@ -258,7 +258,7 @@ struct Options {
   // If non-null, then we should collect metrics about database operations
   // Statistics objects should not be shared between DB instances as
   // it does not use any locks to prevent concurrent updates.
-  Statistics* statistics;
+  shared_ptr<Statistics> statistics;
 
   // If true, then the contents of data files are not synced
   // to stable storage. Their contents remain in the OS buffers till the

include/leveldb/statistics.h

@@ -124,10 +124,10 @@ class Statistics {
 };
 
 // Ease of Use functions
-inline void RecordTick(Statistics* const statistics,
+inline void RecordTick(std::shared_ptr<Statistics> statistics,
                        Tickers ticker,
                        uint64_t count = 1) {
-  if (statistics != nullptr) {
+  if (statistics) {
     statistics->recordTick(ticker, count);
   }
 }

table/table.cc

@@ -196,7 +196,7 @@ Iterator* Table::BlockReader(void* arg,
                              bool* didIO) {
   Table* table = reinterpret_cast<Table*>(arg);
   Cache* block_cache = table->rep_->options.block_cache.get();
-  Statistics* const statistics = table->rep_->options.statistics;
+  std::shared_ptr<Statistics> statistics = table->rep_->options.statistics;
   Block* block = nullptr;
   Cache::Handle* cache_handle = nullptr;
 

tools/db_stress.cc

@@ -101,7 +101,7 @@ static const char* FLAGS_db = nullptr;
 static bool FLAGS_verify_checksum = false;
 
 // Database statistics
-static class leveldb::DBStatistics* dbstats;
+static std::shared_ptr<leveldb::Statistics> dbstats;
 
 // Sync all writes to disk
 static bool FLAGS_sync = false;
@@ -1045,7 +1045,7 @@ int main(int argc, char** argv) {
     } else if (sscanf(argv[i], "--statistics=%d%c", &n, &junk) == 1 &&
                (n == 0 || n == 1)) {
       if (n == 1) {
-        dbstats =  new leveldb::DBStatistics();
+        dbstats.reset(new leveldb::DBStatistics());
       }
     } else if (sscanf(argv[i], "--sync=%d%c", &n, &junk) == 1 &&
                (n == 0 || n == 1)) {
@@ -1131,8 +1131,5 @@ int main(int argc, char** argv) {
 
   leveldb::StressTest stress;
   stress.Run();
-  if (dbstats) {
-    delete dbstats;
-  }
   return 0;
 }

util/options.cc

@@ -39,7 +39,6 @@ Options::Options()
       expanded_compaction_factor(25),
       source_compaction_factor(1),
       max_grandparent_overlap_factor(10),
-      statistics(nullptr),
       disableDataSync(false),
       use_fsync(false),
       db_stats_log_interval(1800),

util/stop_watch.h

@@ -24,7 +24,7 @@ class DoNothingStopWatch : public StopWatch {
 class ScopedRecordingStopWatch : public StopWatch {
  public:
   ScopedRecordingStopWatch(Env * const env,
-                           Statistics * const statistics,
+                           std::shared_ptr<Statistics> statistics,
                            const Histograms histogram_name) :
                             env_(env),
                             start_time_(env->NowMicros()),
@@ -43,7 +43,7 @@ class ScopedRecordingStopWatch : public StopWatch {
  private:
   Env* const env_;
   const uint64_t start_time_;
-  Statistics* const statistics_;
+  std::shared_ptr<Statistics> statistics_;
   const Histograms histogram_name_;
 
 };
@@ -51,13 +51,13 @@ class ScopedRecordingStopWatch : public StopWatch {
 namespace stats {
 // Helper method
 std::unique_ptr<StopWatch> StartStopWatch(Env * const env,
-                                          Statistics * const statistics,
+                                          std::shared_ptr<Statistics> stats,
                                           Histograms histogram_name) {
   assert(env);
-  if (statistics != nullptr) {
+  if (stats) {
     return std::unique_ptr<StopWatch>(new ScopedRecordingStopWatch(
                                            env,
-                                           statistics,
+                                           stats,
                                            histogram_name));
   } else {
     return std::unique_ptr<StopWatch>(new DoNothingStopWatch());