leveldb: Add DBTracker for exposing databases to Chrome's memory-infra.

third_party/leveldatabase/env_chromium.cc

 // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
 // 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.
 
 #include "third_party/leveldatabase/env_chromium.h"
 
+#include <inttypes.h>
+
 #include <utility>
 
 #if defined(OS_POSIX)
 #include <dirent.h>
 #include <sys/types.h>
 #endif
 
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
+#include "base/logging.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/process/process_metrics.h"
 #include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/memory_dump_provider.h"
+#include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "third_party/leveldatabase/chromium_logger.h"
 #include "third_party/leveldatabase/src/include/leveldb/options.h"
 #include "third_party/re2/src/re2/re2.h"
 
 using base::FilePath;
 using leveldb::FileLock;
 using leveldb::Slice;
 using leveldb::Status;
 
@@ skipping 1053 matching lines @@
     return LEVELDB_STATUS_CORRUPTION;
   if (s.IsNotSupportedError())
     return LEVELDB_STATUS_NOT_SUPPORTED;
   if (s.IsIOError())
     return LEVELDB_STATUS_IO_ERROR;
   // TODO(cmumford): IsInvalidArgument() was just added to leveldb. Use this
   // function once that change goes to the public repository.
   return LEVELDB_STATUS_INVALID_ARGUMENT;
 }
 
+// DBRegistry's leveldb::DB wrapper. Forwards all calls to the underlying
+// leveldb::DB instance. Registers / unregisters itself in the registry.
+class DBRegistry::DBWrapperImpl : public base::LinkNode<DBWrapperImpl>,
+                                  public DBWrapper {
+ public:
+  DBWrapperImpl(const std::string name, leveldb::DB* db)

[pwnall, 2017/06/22 02:50:22]

I think it's cleaner to pass a DBRegistry pointer to the constructor, store it
in the instance, and call UnregisterDatabase on it.

[DmitrySkiba, 2017/06/26 20:45:18]

Done.

+      : name_(name), db_(db) {
+    DBRegistry::GetInstance()->RegisterDatabase(this);
+  }
+
+  ~DBWrapperImpl() override {
+    DBRegistry::GetInstance()->UnregisterDatabase(this);
+  }
+
+  const std::string& name() const override { return name_; }
+
+  leveldb::Status Put(const leveldb::WriteOptions& options,
+                      const leveldb::Slice& key,
+                      const leveldb::Slice& value) override {
+    return db_->Put(options, key, value);
+  }
+
+  leveldb::Status Delete(const leveldb::WriteOptions& options,
+                         const leveldb::Slice& key) override {
+    return db_->Delete(options, key);
+  }
+
+  leveldb::Status Write(const leveldb::WriteOptions& options,
+                        leveldb::WriteBatch* updates) override {
+    return db_->Write(options, updates);
+  }
+
+  leveldb::Status Get(const leveldb::ReadOptions& options,
+                      const leveldb::Slice& key,
+                      std::string* value) override {
+    return db_->Get(options, key, value);
+  }
+
+  const leveldb::Snapshot* GetSnapshot() override { return db_->GetSnapshot(); }
+
+  void ReleaseSnapshot(const leveldb::Snapshot* snapshot) override {
+    return db_->ReleaseSnapshot(snapshot);
+  }
+
+  bool GetProperty(const leveldb::Slice& property,
+                   std::string* value) override {
+    return db_->GetProperty(property, value);
+  }
+
+  void GetApproximateSizes(const leveldb::Range* range,
+                           int n,
+                           uint64_t* sizes) override {
+    return db_->GetApproximateSizes(range, n, sizes);
+  }
+
+  void CompactRange(const leveldb::Slice* begin,
+                    const leveldb::Slice* end) override {
+    return db_->CompactRange(begin, end);
+  }
+
+  leveldb::Iterator* NewIterator(const leveldb::ReadOptions& options) override {
+    return db_->NewIterator(options);
+  }
+
+ private:
+  std::string name_;
+  std::unique_ptr<leveldb::DB> db_;
+};
+
+// DBRegistry MemoryDumpProvider implementation. Declared here to minimize

[pwnall, 2017/06/22 02:50:22]

Both sentences in the comment seem redundant with the code. Can you please
remove, and possibly replace with an explanation of what this tracks? Example
output might be sufficient.

[DmitrySkiba, 2017/06/26 20:45:18]

Done.

+// header file dependencies.
+class DBRegistry::MDP : public base::trace_event::MemoryDumpProvider {
+ public:
+  bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
+                    base::trace_event::ProcessMemoryDump* pmd) override {
+    auto db_visitor = [&](DBWrapper* db) {
+      std::string db_dump_name = base::StringPrintf(
+          "leveldatabase/0x%" PRIXPTR, reinterpret_cast<uintptr_t>(db));
+      auto* db_dump = pmd->CreateAllocatorDump(db_dump_name.c_str());
+
+      uint64_t db_memory_usage = 0;
+      {
+        std::string usage_string;
+        bool success = db->GetProperty("leveldb.approximate-memory-usage",
+                                       &usage_string) &&
+                       base::StringToUint64(usage_string, &db_memory_usage);
+        DCHECK(success);
+      }
+      db_dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
+                         base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                         db_memory_usage);
+
+      if (args.level_of_detail !=
+          base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND) {
+        db_dump->AddString("name", "", db->name());
+      }
+
+      const char* system_allocator_name =
+          base::trace_event::MemoryDumpManager::GetInstance()
+              ->system_allocator_pool_name();
+      if (system_allocator_name) {
+        pmd->AddSuballocation(db_dump->guid(), system_allocator_name);
+      }
+    };
+
+    DBRegistry::GetInstance()->VisitDatabases(db_visitor);
+    return true;
+  }
+};
+
+DBRegistry::DBRegistry() : mdp_(new MDP()) {
+  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+      mdp_.get(), "LevelDB", nullptr);
+}
+
+DBRegistry::~DBRegistry() {
+  NOTREACHED();  // DBRegistry is a singleton
+}
+
+DBRegistry* DBRegistry::GetInstance() {
+  static DBRegistry* instance = new DBRegistry();

[pwnall, 2017/06/22 02:50:22]

Is this threadsafe? (Asking because DBRegistry's comment says the class is
threadsafe.)

I haven't kept up with how Chromium deals with thread safety, but I vaguely
remember that more ceremony was involved.

[DmitrySkiba, 2017/06/26 20:45:18]

Yes, Chrome switched back to using treadsafe static locals (since MSVC 2015
started supporting them). This is now the pattern.

+  return instance;
+}
+
+leveldb::Status DBRegistry::Open(const leveldb::Options& options,
+                                 const std::string& name,
+                                 leveldb::DB** dbptr) {
+  auto status = leveldb::DB::Open(options, name, dbptr);
+  if (status.ok()) {
+    // DBWrapperImpl ctor registers the instance in DBRegistry
+    *dbptr = new DBWrapperImpl(name, *dbptr);
+  }
+  return status;
+}
+
+void DBRegistry::VisitDatabases(const DatabaseVisitor& visitor) {
+  base::AutoLock lock(databases_lock_);
+  for (auto* i = databases_.head(); i != databases_.end(); i = i->next()) {
+    visitor(i->value());
+  }
+}
+
+void DBRegistry::RegisterDatabase(DBWrapperImpl* database) {
+  base::AutoLock lock(databases_lock_);
+  databases_.Append(database);
+}
+
+void DBRegistry::UnregisterDatabase(DBWrapperImpl* database) {
+  base::AutoLock lock(databases_lock_);
+  database->RemoveFromList();
+}
+
 }  // namespace leveldb_env
 
 namespace leveldb {
 
 Env* Env::Default() {
   return leveldb_env::default_env.Pointer();
 }
 
 }  // namespace leveldb