Adding tracing of freelist size to ClientDiscardableSharedMemoryManager

Bug: 1106364
Change-Id: If62ae6d1efdd76fd12f7550c5828c61798a61914
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310899Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Thiabaud Engelbrecht <thiabaud@google.com>
Cr-Commit-Position: refs/heads/master@{#792098}

components/discardable_memory/client/client_discardable_shared_memory_manager.cc

@@ -135,7 +135,7 @@ ClientDiscardableSharedMemoryManager::ClientDiscardableSharedMemoryManager(
     : io_task_runner_(std::move(io_task_runner)),
       manager_mojo_(std::make_unique<
                     mojo::Remote<mojom::DiscardableSharedMemoryManager>>()),
-      heap_(new DiscardableSharedMemoryHeap(base::GetPageSize())) {
+      heap_(std::make_unique<DiscardableSharedMemoryHeap>()) {
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
       this, "ClientDiscardableSharedMemoryManager",
       base::ThreadTaskRunnerHandle::Get());
@@ -184,7 +184,7 @@ ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
                static_cast<size_t>(1));
 
   static const size_t allocation_size = GetDefaultAllocationSize();
-  DCHECK(allocation_size % base::GetPageSize() == 0);
+  DCHECK_EQ(allocation_size % base::GetPageSize(), 0u);
   // Default allocation size in pages.
   size_t allocation_pages = allocation_size / base::GetPageSize();
 
@@ -286,24 +286,7 @@ bool ClientDiscardableSharedMemoryManager::OnMemoryDump(
     const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {
   base::AutoLock lock(lock_);
-  if (args.level_of_detail ==
-      base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND) {
-    base::trace_event::MemoryAllocatorDump* total_dump =
-        pmd->CreateAllocatorDump(
-            base::StringPrintf("discardable/child_0x%" PRIXPTR,
-                               reinterpret_cast<uintptr_t>(this)));
-    const size_t total_size = heap_->GetSize();
-    const size_t freelist_size = heap_->GetSizeOfFreeLists();
-    total_dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
-                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                          total_size - freelist_size);
-    total_dump->AddScalar("freelist_size",
-                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                          freelist_size);
-    return true;
-  }
-
-  return heap_->OnMemoryDump(pmd);
+  return heap_->OnMemoryDump(args, pmd);
 }
 
 size_t ClientDiscardableSharedMemoryManager::GetBytesAllocated() const {

components/discardable_memory/client/client_discardable_shared_memory_manager.h

@@ -64,6 +64,7 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
     size_t freelist_size;
   };
 
+  // Overridden from base::DiscardableMemoryAllocator:
   size_t GetBytesAllocated() const override;
   void SetBytesAllocatedLimitForTesting(size_t limit) {
     bytes_allocated_limit_for_testing_ = limit;

components/discardable_memory/common/BUILD.gn

@@ -26,6 +26,7 @@ source_set("unit_tests") {
   deps = [
     ":common",
     "//base",
+    "//testing/gmock",
     "//testing/gtest",
   ]
 }

components/discardable_memory/common/discardable_shared_memory_heap.cc

@@ -91,8 +91,8 @@ void DiscardableSharedMemoryHeap::ScopedMemorySegment::OnMemoryDump(
   heap_->OnMemoryDump(shared_memory_.get(), size_, id_, pmd);
 }
 
-DiscardableSharedMemoryHeap::DiscardableSharedMemoryHeap(size_t block_size)
-    : block_size_(block_size), num_blocks_(0), num_free_blocks_(0) {
+DiscardableSharedMemoryHeap::DiscardableSharedMemoryHeap()
+    : block_size_(base::GetPageSize()) {
   DCHECK_NE(block_size_, 0u);
   DCHECK(base::bits::IsPowerOfTwo(block_size_));
 }
@@ -251,7 +251,29 @@ size_t DiscardableSharedMemoryHeap::GetSizeOfFreeLists() const {
 }
 
 bool DiscardableSharedMemoryHeap::OnMemoryDump(
+    const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {
+  // Keep track of some metrics that are specific to the
+  // DiscardableSharedMemoryHeap, which aren't covered by the individual dumps
+  // for each segment below.
+  auto* total_dump = pmd->CreateAllocatorDump(base::StringPrintf(
+      "discardable/child_0x%" PRIXPTR, reinterpret_cast<uintptr_t>(this)));
+  const size_t freelist_size = GetSizeOfFreeLists();
+  total_dump->AddScalar("freelist_size",
+                        base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                        freelist_size);
+  if (args.level_of_detail ==
+      base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND) {
+    const size_t total_size = GetSize();
+    total_dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
+                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                          total_size - freelist_size);
+    return true;
+  }
+
+  // This iterates over all the memory allocated by the heap, and calls
+  // |OnMemoryDump| for each. It does not contain any information about the
+  // DiscardableSharedMemoryHeap itself.
   std::for_each(memory_segments_.begin(), memory_segments_.end(),
                 [pmd](const std::unique_ptr<ScopedMemorySegment>& segment) {
                   segment->OnMemoryDump(pmd);

components/discardable_memory/common/discardable_shared_memory_heap.h

@@ -52,7 +52,7 @@ class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryHeap {
     DISALLOW_COPY_AND_ASSIGN(Span);
   };
 
-  explicit DiscardableSharedMemoryHeap(size_t block_size);
+  DiscardableSharedMemoryHeap();
   ~DiscardableSharedMemoryHeap();
 
   // Grow heap using |shared_memory| and return a span for this new memory.
@@ -94,7 +94,8 @@ class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryHeap {
   size_t GetSizeOfFreeLists() const;
 
   // Dumps memory statistics for chrome://tracing.
-  bool OnMemoryDump(base::trace_event::ProcessMemoryDump* pmd);
+  bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
+                    base::trace_event::ProcessMemoryDump* pmd);
 
   // Returns a MemoryAllocatorDump for a given span on |pmd| with the size of
   // the span.
@@ -155,9 +156,9 @@ class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryHeap {
                     int32_t segment_id,
                     base::trace_event::ProcessMemoryDump* pmd);
 
-  size_t block_size_;
-  size_t num_blocks_;
-  size_t num_free_blocks_;
+  const size_t block_size_;
+  size_t num_blocks_ = 0;
+  size_t num_free_blocks_ = 0;
 
   // Vector of memory segments.
   std::vector<std::unique_ptr<ScopedMemorySegment>> memory_segments_;

components/discardable_memory/common/discardable_shared_memory_heap_perftest.cc

@@ -27,12 +27,11 @@ const int kTimeCheckInterval = 8192;
 void NullTask() {}
 
 TEST(DiscardableSharedMemoryHeapTest, SearchFreeLists) {
-  size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
 
   const size_t kBlocks = 4096;
   const size_t kSegments = 16;
-  size_t segment_size = block_size * kBlocks;
+  size_t segment_size = base::GetPageSize() * kBlocks;
   int next_discardable_shared_memory_id = 0;
 
   for (size_t i = 0; i < kSegments; ++i) {

components/discardable_memory/common/discardable_shared_memory_heap_unittest.cc

@@ -4,12 +4,15 @@
 
 #include "components/discardable_memory/common/discardable_shared_memory_heap.h"
 
+#include <inttypes.h>
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/memory/discardable_shared_memory.h"
 #include "base/process/process_metrics.h"
+#include "base/strings/stringprintf.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace discardable_memory {
@@ -19,7 +22,7 @@ void NullTask() {}
 
 TEST(DiscardableSharedMemoryHeapTest, Basic) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
 
   // Initial size should be 0.
   EXPECT_EQ(0u, heap.GetSize());
@@ -75,7 +78,7 @@ TEST(DiscardableSharedMemoryHeapTest, Basic) {
 
 TEST(DiscardableSharedMemoryHeapTest, SplitAndMerge) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
 
   const size_t kBlocks = 6;
   size_t memory_size = block_size * kBlocks;
@@ -132,7 +135,7 @@ TEST(DiscardableSharedMemoryHeapTest, SplitAndMerge) {
 
 TEST(DiscardableSharedMemoryHeapTest, MergeSingleBlockSpan) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
 
   const size_t kBlocks = 6;
   size_t memory_size = block_size * kBlocks;
@@ -159,7 +162,8 @@ TEST(DiscardableSharedMemoryHeapTest, MergeSingleBlockSpan) {
 
 TEST(DiscardableSharedMemoryHeapTest, Grow) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
+
   int next_discardable_shared_memory_id = 0;
 
   std::unique_ptr<base::DiscardableSharedMemory> memory1(
@@ -197,7 +201,8 @@ TEST(DiscardableSharedMemoryHeapTest, Grow) {
 
 TEST(DiscardableSharedMemoryHeapTest, ReleaseFreeMemory) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
+
   int next_discardable_shared_memory_id = 0;
 
   std::unique_ptr<base::DiscardableSharedMemory> memory(
@@ -225,7 +230,8 @@ TEST(DiscardableSharedMemoryHeapTest, ReleaseFreeMemory) {
 
 TEST(DiscardableSharedMemoryHeapTest, ReleasePurgedMemory) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
+
   int next_discardable_shared_memory_id = 0;
 
   std::unique_ptr<base::DiscardableSharedMemory> memory(
@@ -252,7 +258,7 @@ TEST(DiscardableSharedMemoryHeapTest, ReleasePurgedMemory) {
 
 TEST(DiscardableSharedMemoryHeapTest, Slack) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
 
   const size_t kBlocks = 6;
   size_t memory_size = block_size * kBlocks;
@@ -287,7 +293,8 @@ void OnDeleted(bool* deleted) {
 
 TEST(DiscardableSharedMemoryHeapTest, DeletedCallback) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
+
   int next_discardable_shared_memory_id = 0;
 
   std::unique_ptr<base::DiscardableSharedMemory> memory(
@@ -306,7 +313,8 @@ TEST(DiscardableSharedMemoryHeapTest, DeletedCallback) {
 
 TEST(DiscardableSharedMemoryHeapTest, CreateMemoryAllocatorDumpTest) {
   size_t block_size = base::GetPageSize();
-  DiscardableSharedMemoryHeap heap(block_size);
+  DiscardableSharedMemoryHeap heap;
+
   int next_discardable_shared_memory_id = 0;
 
   std::unique_ptr<base::DiscardableSharedMemory> memory(
@@ -334,5 +342,48 @@ TEST(DiscardableSharedMemoryHeapTest, CreateMemoryAllocatorDumpTest) {
                                              pmd.get()));
 }
 
+TEST(DiscardableSharedMemoryHeapTest, OnMemoryDumpTest) {
+  size_t block_size = base::GetPageSize();
+  using testing::ByRef;
+  using testing::Contains;
+  using testing::Eq;
+  DiscardableSharedMemoryHeap heap;
+
+  int next_discardable_shared_memory_id = 0;
+
+  base::trace_event::MemoryDumpArgs args = {
+      base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND};
+  {
+    base::trace_event::ProcessMemoryDump pmd(args);
+    heap.OnMemoryDump(args, &pmd);
+    auto* dump = pmd.GetAllocatorDump(base::StringPrintf(
+        "discardable/child_0x%" PRIXPTR, reinterpret_cast<uintptr_t>(&heap)));
+    ASSERT_NE(nullptr, dump);
+
+    base::trace_event::MemoryAllocatorDump::Entry freelist("freelist_size",
+                                                           "bytes", 0);
+    EXPECT_THAT(dump->entries(), Contains(Eq(ByRef(freelist))));
+  }
+
+  {
+    std::unique_ptr<base::DiscardableSharedMemory> memory(
+        new base::DiscardableSharedMemory);
+    ASSERT_TRUE(memory->CreateAndMap(block_size));
+    std::unique_ptr<DiscardableSharedMemoryHeap::Span> span = heap.Grow(
+        std::move(memory), block_size, next_discardable_shared_memory_id++,
+        base::BindOnce(NullTask));
+    heap.MergeIntoFreeLists(std::move(span));
+    base::trace_event::ProcessMemoryDump pmd(args);
+    heap.OnMemoryDump(args, &pmd);
+    auto* dump = pmd.GetAllocatorDump(base::StringPrintf(
+        "discardable/child_0x%" PRIXPTR, reinterpret_cast<uintptr_t>(&heap)));
+    ASSERT_NE(nullptr, dump);
+
+    base::trace_event::MemoryAllocatorDump::Entry freelist("freelist_size",
+                                                           "bytes", block_size);
+    EXPECT_THAT(dump->entries(), Contains(Eq(ByRef(freelist))));
+  }
+}
+
 }  // namespace
-}  // namespace content
+}  // namespace discardable_memory