Add leveldb block cache memory dumps with appropriate sizes

The block cache is shared across multiple databases. So, calculate the
correct sizes of each leveldb database for the memory dumps.

Bug: 758267
Change-Id: I4ca76422754c0a7fce46354f79631362d5553cdc
Reviewed-on: https://chromium-review.googlesource.com/667916
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509677}

base/trace_event/memory_infra_background_whitelist.cc

@@ -79,10 +79,13 @@ const char* const kAllocatorDumpNameWhitelist[] = {
     "gpu/gl/textures/share_group_0x?",
     "java_heap",
     "java_heap/allocated_objects",
-    "leveldatabase/0x?",
-    "leveldb/leveldb_proto/0x?",
-    "leveldb/mojo/0x?",
-    "leveldb/mojo/0x?/block_cache",
+    "leveldatabase",
+    "leveldatabase/block_cache/browser",
+    "leveldatabase/block_cache/in_memory",
+    "leveldatabase/block_cache/unified",
+    "leveldatabase/block_cache/web",
+    "leveldatabase/db_0x?",
+    "leveldatabase/db_0x?/block_cache",
     "malloc",
     "malloc/allocated_objects",
     "malloc/metadata_fragmentation_caches",

content/browser/dom_storage/session_storage_database.cc

@@ -360,33 +360,23 @@ bool SessionStorageDatabase::ReadNamespacesAndOrigins(
 
 void SessionStorageDatabase::OnMemoryDump(
     base::trace_event::ProcessMemoryDump* pmd) {
-  std::string db_memory_usage;
-  {
-    base::AutoLock lock(db_lock_);
-    if (!db_)
-      return;
-
-    bool res =
-        db_->GetProperty("leveldb.approximate-memory-usage", &db_memory_usage);
-    DCHECK(res);
-  }
-
-  uint64_t size;
-  bool res = base::StringToUint64(db_memory_usage, &size);
-  DCHECK(res);
+  base::AutoLock lock(db_lock_);
+  if (!db_)
+    return;
+  // All leveldb databases are already dumped by leveldb_env::DBTracker. Add
+  // an edge to the existing dump.
+  auto* tracker_dump =
+      leveldb_env::DBTracker::GetOrCreateAllocatorDump(pmd, db_.get());
+  if (!tracker_dump)
+    return;
 
   auto* mad = pmd->CreateAllocatorDump(
       base::StringPrintf("site_storage/session_storage_0x%" PRIXPTR,
                          reinterpret_cast<uintptr_t>(this)));
+  pmd->AddOwnershipEdge(mad->guid(), tracker_dump->guid());
   mad->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
-                 base::trace_event::MemoryAllocatorDump::kUnitsBytes, size);
-
-  // All leveldb databases are already dumped by leveldb_env::DBTracker. Add
-  // an edge to avoid double counting.
-  auto* tracker_dump =
-      leveldb_env::DBTracker::GetOrCreateAllocatorDump(pmd, db_.get());
-  if (tracker_dump)
-    pmd->AddOwnershipEdge(mad->guid(), tracker_dump->guid());
+                 base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                 tracker_dump->GetSizeInternal());
 }
 
 bool SessionStorageDatabase::LazyOpen(bool create_if_needed) {

content/browser/indexed_db/leveldb/leveldb_database.cc

@@ -467,18 +467,20 @@ bool LevelDBDatabase::OnMemoryDump(
     base::trace_event::ProcessMemoryDump* pmd) {
   if (!db_)
     return false;
-
-  std::string value;
-  uint64_t size;
-  bool res = db_->GetProperty("leveldb.approximate-memory-usage", &value);
-  DCHECK(res);
-  base::StringToUint64(value, &size);
+  // All leveldb databases are already dumped by leveldb_env::DBTracker. Add
+  // an edge to the existing database.
+  auto* tracker_dump =
+      leveldb_env::DBTracker::GetOrCreateAllocatorDump(pmd, db_.get());
+  if (!tracker_dump)
+    return true;
 
   auto* dump = pmd->CreateAllocatorDump(
       base::StringPrintf("site_storage/index_db/0x%" PRIXPTR,
                          reinterpret_cast<uintptr_t>(db_.get())));
   dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
-                  base::trace_event::MemoryAllocatorDump::kUnitsBytes, size);
+                  base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                  tracker_dump->GetSizeInternal());
+  pmd->AddOwnershipEdge(dump->guid(), tracker_dump->guid());
 
   // Dumps in BACKGROUND mode cannot have strings or edges in order to minimize
   // trace size and instrumentation overhead.
@@ -488,14 +490,6 @@ bool LevelDBDatabase::OnMemoryDump(
   }
 
   dump->AddString("file_name", "", file_name_for_tracing);
-
-  // All leveldb databases are already dumped by leveldb_env::DBTracker. Add
-  // an edge to avoid double counting.
-  auto* tracker_dump =
-      leveldb_env::DBTracker::GetOrCreateAllocatorDump(pmd, db_.get());
-  if (tracker_dump)
-    pmd->AddOwnershipEdge(dump->guid(), tracker_dump->guid());
-
   return true;
 }
 

extensions/browser/value_store/leveldb_value_store.cc

@@ -246,25 +246,20 @@ bool LeveldbValueStore::OnMemoryDump(
   if (!db())
     return true;
 
-  std::string value;
-  uint64_t size;
-  bool res = db()->GetProperty("leveldb.approximate-memory-usage", &value);
-  DCHECK(res);
-  res = base::StringToUint64(value, &size);
-  DCHECK(res);
+  // All leveldb databases are already dumped by leveldb_env::DBTracker. Add
+  // an edge to the existing dump.
+  auto* tracker_dump =
+      leveldb_env::DBTracker::GetOrCreateAllocatorDump(pmd, db());
+  if (!tracker_dump)
+    return true;
 
   auto* dump = pmd->CreateAllocatorDump(base::StringPrintf(
       "extensions/value_store/%s/0x%" PRIXPTR, open_histogram_name().c_str(),
       reinterpret_cast<uintptr_t>(this)));
   dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
-                  base::trace_event::MemoryAllocatorDump::kUnitsBytes, size);
-
-  // All leveldb databases are already dumped by leveldb_env::DBTracker. Add
-  // an edge to avoid double counting.
-  auto* tracker_db =
-      leveldb_env::DBTracker::GetOrCreateAllocatorDump(pmd, db());
-  if (tracker_db)
-    pmd->AddOwnershipEdge(dump->guid(), tracker_db->guid());
+                  base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                  tracker_dump->GetSizeInternal());
+  pmd->AddOwnershipEdge(dump->guid(), tracker_dump->guid());
 
   return true;
 }

third_party/leveldatabase/env_chromium.h

@@ -12,6 +12,7 @@
 
 #include "base/callback.h"
 #include "base/containers/circular_deque.h"
+#include "base/containers/flat_map.h"
 #include "base/containers/linked_list.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
@@ -277,6 +278,9 @@ class LEVELDB_EXPORT DBTracker {
    public:
     // Name that OpenDatabase() was called with.
     virtual const std::string& name() const = 0;
+
+    // Options used when opening the database.
+    virtual SharedReadCacheUse block_cache_type() const = 0;
   };
 
   // Opens a database and starts tracking it. As long as the opened database
@@ -296,15 +300,11 @@ class LEVELDB_EXPORT DBTracker {
 
   friend class ChromiumEnvDBTrackerTest;
   FRIEND_TEST_ALL_PREFIXES(ChromiumEnvDBTrackerTest, IsTrackedDB);
-  FRIEND_TEST_ALL_PREFIXES(ChromiumEnvDBTrackerTest, GetOrCreateAllocatorDump);
+  FRIEND_TEST_ALL_PREFIXES(ChromiumEnvDBTrackerTest, MemoryDumpCreation);
 
   DBTracker();
   ~DBTracker();
 
-  static base::trace_event::MemoryAllocatorDump* GetOrCreateAllocatorDump(
-      base::trace_event::ProcessMemoryDump* pmd,
-      TrackedDB* db);
-
   // Calls |visitor| for each live database. The database is live from the
   // point it was returned from OpenDatabase() and up until its instance is
   // destroyed.
@@ -319,12 +319,10 @@ class LEVELDB_EXPORT DBTracker {
   void DatabaseOpened(TrackedDBImpl* database, SharedReadCacheUse cache_use);
   void DatabaseDestroyed(TrackedDBImpl* database, SharedReadCacheUse cache_use);
 
-  std::unique_ptr<MemoryDumpProvider> mdp_;
-
-  // Protect databases_ and database_use_count_.
+  // Protect databases_ and mdp_ members.
   mutable base::Lock databases_lock_;
   base::LinkedList<TrackedDBImpl> databases_;
-  int database_use_count_[SharedReadCacheUse_NumCacheUses] = {};
+  std::unique_ptr<MemoryDumpProvider> mdp_;
 
   DISALLOW_COPY_AND_ASSIGN(DBTracker);
 };

third_party/leveldatabase/env_chromium_unittest.cc

@@ -19,6 +19,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/leveldatabase/env_chromium.h"
 #include "third_party/leveldatabase/leveldb_chrome.h"
+#include "third_party/leveldatabase/src/include/leveldb/cache.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 
 #define FPL FILE_PATH_LITERAL
@@ -296,30 +297,6 @@ class ChromiumEnvDBTrackerTest : public ::testing::Test {
   base::ScopedTempDir scoped_temp_dir_;
 };
 
-TEST_F(ChromiumEnvDBTrackerTest, GetOrCreateAllocatorDump) {
-  Options options;
-  options.create_if_missing = true;
-  std::string name = temp_path().AsUTF8Unsafe();
-  DBTracker::TrackedDB* tracked_db;
-  Status s = DBTracker::GetInstance()->OpenDatabase(options, name, &tracked_db);
-  ASSERT_TRUE(s.ok()) << s.ToString();
-
-  const MemoryDumpArgs detailed_args = {MemoryDumpLevelOfDetail::DETAILED};
-  ProcessMemoryDump pmd(nullptr, detailed_args);
-  auto* mad =
-      DBTracker::GetInstance()->GetOrCreateAllocatorDump(&pmd, tracked_db);
-  delete tracked_db;
-  ASSERT_TRUE(mad != nullptr);
-
-  // Check that the size was added.
-  auto& entries = mad->entries();
-  ASSERT_EQ(1ul, entries.size());
-  EXPECT_EQ(base::trace_event::MemoryAllocatorDump::kNameSize, entries[0].name);
-  EXPECT_EQ(base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-            entries[0].units);
-  EXPECT_GE(entries[0].value_uint64, 0ul);
-}
-
 TEST_F(ChromiumEnvDBTrackerTest, OpenDatabase) {
   struct KeyValue {
     const char* key;
@@ -444,6 +421,65 @@ TEST_F(ChromiumEnvDBTrackerTest, CheckMemEnv) {
   EXPECT_TRUE(leveldb_chrome::IsMemEnv(memenv.get()));
 }
 
+TEST_F(ChromiumEnvDBTrackerTest, MemoryDumpCreation) {
+  Options options;
+  options.create_if_missing = true;
+  leveldb::Cache* web_cache = leveldb_chrome::GetSharedWebBlockCache();
+  leveldb::Cache* browser_cache = leveldb_chrome::GetSharedBrowserBlockCache();
+  options.block_cache = web_cache;
+  std::unique_ptr<leveldb::DB> db1;
+  base::ScopedTempDir temp_dir1;
+  ASSERT_TRUE(temp_dir1.CreateUniqueTempDir());
+  base::ScopedTempDir temp_dir2;
+  ASSERT_TRUE(temp_dir2.CreateUniqueTempDir());
+  base::ScopedTempDir temp_dir3;
+  ASSERT_TRUE(temp_dir3.CreateUniqueTempDir());
+
+  auto status =
+      leveldb_env::OpenDB(options, temp_dir1.GetPath().AsUTF8Unsafe(), &db1);
+  ASSERT_TRUE(status.ok()) << status.ToString();
+
+  std::unique_ptr<leveldb::DB> db2;
+  status =
+      leveldb_env::OpenDB(options, temp_dir2.GetPath().AsUTF8Unsafe(), &db2);
+  ASSERT_TRUE(status.ok()) << status.ToString();
+
+  std::unique_ptr<leveldb::DB> db3;
+  options.block_cache = browser_cache;
+  status =
+      leveldb_env::OpenDB(options, temp_dir3.GetPath().AsUTF8Unsafe(), &db3);
+  ASSERT_TRUE(status.ok()) << status.ToString();
+
+  auto db_visitor = [](DBTracker::TrackedDB* db) {
+    leveldb::Cache* db_cache =
+        (db->block_cache_type() == DBTracker::SharedReadCacheUse_Browser)
+            ? leveldb_chrome::GetSharedBrowserBlockCache()
+            : leveldb_chrome::GetSharedWebBlockCache();
+    size_t initial_cache_size = db_cache->TotalCharge();
+    auto status = db->Put(WriteOptions(), "key", "value");
+    EXPECT_TRUE(status.ok()) << status.ToString();
+    db->CompactRange(nullptr, nullptr);
+    std::string value;
+    status = db->Get(ReadOptions(), "key", &value);
+    ASSERT_TRUE(status.ok()) << status.ToString();
+    EXPECT_GT(db_cache->TotalCharge(), initial_cache_size);
+  };
+  DBTracker::GetInstance()->VisitDatabases(base::BindRepeating(db_visitor));
+  ASSERT_EQ(browser_cache->TotalCharge() * 2, web_cache->TotalCharge());
+
+  MemoryDumpArgs dump_args = {MemoryDumpLevelOfDetail::BACKGROUND};
+  base::trace_event::ProcessMemoryDump pmd(nullptr, dump_args);
+  auto* mad1 = DBTracker::GetOrCreateAllocatorDump(&pmd, db1.get());
+  auto* mad2 = DBTracker::GetOrCreateAllocatorDump(&pmd, db2.get());
+  auto* mad3 = DBTracker::GetOrCreateAllocatorDump(&pmd, db3.get());
+
+  // All databases should have the same size since we made the same changes.
+  size_t db_size = mad1->GetSizeInternal();
+  EXPECT_GT(db_size, 0ul);
+  EXPECT_EQ(db_size, mad2->GetSizeInternal());
+  EXPECT_EQ(db_size, mad3->GetSizeInternal());
+}
+
 }  // namespace leveldb_env
 
 int main(int argc, char** argv) { return base::TestSuite(argc, argv).Run(); }