Adding discardable memory dump provider.

content/child/child_discardable_shared_memory_manager.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/child/child_discardable_shared_memory_manager.h"
 
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
 #include "base/debug/crash_logging.h"
 #include "base/memory/discardable_memory.h"
 #include "base/memory/discardable_shared_memory.h"
 #include "base/metrics/histogram.h"
 #include "base/process/memory.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "content/common/child_process_messages.h"
 
 namespace content {
 namespace {
 
 // Default allocation size.
 #if defined(OS_ANDROID)
 // Larger allocation size on Android to avoid reaching the FD-limit.
 const size_t kAllocationSize = 32 * 1024 * 1024;
 #else
 const size_t kAllocationSize = 4 * 1024 * 1024;
 #endif
 
+const char kMemoryDumperFriendlyName[] = "ChildDiscardable";

[reveman, 2015/04/23 17:32:30]

is "Discardable" enough? we're not using the "Child" prefix for the trace
counter or UMA histogram.

[ssid, 2015/04/24 11:23:04]

Done.

+
 // Global atomic to generate unique discardable shared memory IDs.
 base::StaticAtomicSequenceNumber g_next_discardable_shared_memory_id;
 
 class DiscardableMemoryImpl : public base::DiscardableMemory {
  public:
   DiscardableMemoryImpl(ChildDiscardableSharedMemoryManager* manager,
                         scoped_ptr<DiscardableSharedMemoryHeap::Span> span)
       : manager_(manager), span_(span.Pass()), is_locked_(true) {}
 
   ~DiscardableMemoryImpl() override {
@@ skipping 36 matching lines @@
     scoped_refptr<ThreadSafeSender> sender,
     DiscardableSharedMemoryId id) {
   sender->Send(new ChildProcessHostMsg_DeletedDiscardableSharedMemory(id));
 }
 
 }  // namespace
 
 ChildDiscardableSharedMemoryManager::ChildDiscardableSharedMemoryManager(
     ThreadSafeSender* sender)
     : heap_(base::GetPageSize()), sender_(sender) {
+  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+      this);
 }
 
 ChildDiscardableSharedMemoryManager::~ChildDiscardableSharedMemoryManager() {
+  base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(

[Primiano Tucci (use gerrit), 2015/04/23 16:11:55]

Considering that this is not a singleton, you should specify the task_runner in
the parent ctor to make sure that all the calls to this happen on the child
thread.
I do understand that the ChildDSMM is able to work on all threads. The problem
however is that you can causes thread races during the shutdown path. Imagine
the following scenario:

- Child thread is shutting down
- The ChildDSMM is about to be destroyed and unregistered.
- Another thread requests the dump
- The MDM will invoke the DUmpInto onto whatever thread, while the CDSMM is
being destroyed.

Even if you don't hit the race, the MDM will DCHECK that if you unregister a
provider, the provider has a task_runner and you are on the right thread.

[ssid, 2015/04/24 11:23:03]

Thanks. done

+      this);
   // TODO(reveman): Determine if this DCHECK can be enabled. crbug.com/430533
   // DCHECK_EQ(heap_.GetSize(), heap_.GetSizeOfFreeLists());
   if (heap_.GetSize())
     MemoryUsageChanged(0, 0);
 }
 
 scoped_ptr<base::DiscardableMemory>
 ChildDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
     size_t size) {
   base::AutoLock lock(lock_);
@@ skipping 183 matching lines @@
   static const char kDiscardableMemoryAllocatedKey[] =
       "discardable-memory-allocated";
   base::debug::SetCrashKeyValue(kDiscardableMemoryAllocatedKey,
                                 base::Uint64ToString(new_bytes_total));
 
   static const char kDiscardableMemoryFreeKey[] = "discardable-memory-free";
   base::debug::SetCrashKeyValue(kDiscardableMemoryFreeKey,
                                 base::Uint64ToString(new_bytes_free));
 }
 
+bool ChildDiscardableSharedMemoryManager::DumpInto(
+    base::trace_event::ProcessMemoryDump* pmd) {
+  DiscardableSharedMemoryHeap::MemoryStatistics stats;
+  heap_.GetMemoryStatistics(&stats);
+  for (size_t segment = 0; segment < stats.segments_stats.size(); segment++) {
+    std::string segment_id = base::StringPrintf("segment_%zu", segment);

[reveman, 2015/04/23 17:32:30]

nit: looking at process_memory_dump.h it seems like the current convention is
"segment%zu". So no underscore.

[ssid, 2015/04/24 11:23:03]

I used '_' in V8 provider. Primiano, WDYT?

+    base::trace_event::MemoryAllocatorDump* segment_dump =
+        pmd->CreateAllocatorDump("child_discardable_memory",

[reveman, 2015/04/23 17:32:30]

nit: is "discardable" enough? omitting "child_" to be consistent with other
stats such as "DiscardableMemoryUsage" trace counter and "_memory" suffix as
that's implicit from the fact that we're creating a memory allocator dump.

[ssid, 2015/04/24 11:23:03]

Yes discardable is enough.

+                                 segment_id.c_str());
+    segment_dump->set_physical_size_in_bytes(
+        static_cast<int>(stats.segments_stats[segment].size));
+    segment_dump->set_allocated_objects_count(
+        static_cast<int>(stats.segments_stats[segment].num_of_objects));
+    segment_dump->set_allocated_objects_size_in_bytes(
+        static_cast<int>(stats.segments_stats[segment].used_size));
+  }
+  return true;
+}
+
+const char* ChildDiscardableSharedMemoryManager::GetFriendlyName() const {
+  return kMemoryDumperFriendlyName;
+}
+
 }  // namespace content