memory-infra: Fill the memory dump callback result (1/2)

base/trace_event/memory_dump_manager.cc

 // Copyright 2015 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 "base/trace_event/memory_dump_manager.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/allocator/features.h"
 #include "base/atomic_sequence_num.h"
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
 #include "base/debug/debugging_flags.h"
 #include "base/debug/stack_trace.h"
 #include "base/debug/thread_heap_usage_tracker.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_piece.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/heap_profiler_allocation_context_tracker.h"
 #include "base/trace_event/heap_profiler_event_filter.h"
 #include "base/trace_event/heap_profiler_stack_frame_deduplicator.h"
 #include "base/trace_event/heap_profiler_type_name_deduplicator.h"
 #include "base/trace_event/malloc_dump_provider.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "base/trace_event/memory_dump_scheduler.h"
@@ skipping 682 matching lines @@
   // disabled (unregistered). This is to avoid taking lock while polling.
   for (const auto& mdpinfo : dump_providers_for_polling_) {
     uint64_t value = 0;
     mdpinfo->dump_provider->PollFastMemoryTotal(&value);
     *memory_total += value;
   }
   return true;
 }
 
 // static
+uint32_t MemoryDumpManager::GetDumpsSumKb(const std::string& prefix,
+                                          const ProcessMemoryDump* pmd) {
+  uint64_t sum = 0;
+  for (const auto& kv : pmd->allocator_dumps()) {
+    auto name = StringPiece(kv.first);
+    if (name.starts_with(prefix)) {
+      sum += kv.second->GetSize();
+    }
+    // We shouldn't be summing things with suballocations.
+    DCHECK(name.find("/__") == name.npos);

[ssid, 2017/03/23 03:11:38]

So, this is supposed to work only for BACKGROUND mode?
If so, maybe update the description to say fill in the details only for
background mode?

[hjd, 2017/03/23 14:50:31]

I removed this now that it should work in all modes, thanks!

+  }
+  return sum / 1024;
+}
+
+// static
 void MemoryDumpManager::FinalizeDumpAndAddToTrace(
     std::unique_ptr<ProcessMemoryDumpAsyncState> pmd_async_state) {
   HEAP_PROFILER_SCOPED_IGNORE;
   DCHECK(pmd_async_state->pending_dump_providers.empty());
   const uint64_t dump_guid = pmd_async_state->req_args.dump_guid;
   if (!pmd_async_state->callback_task_runner->BelongsToCurrentThread()) {
     scoped_refptr<SingleThreadTaskRunner> callback_task_runner =
         pmd_async_state->callback_task_runner;
     callback_task_runner->PostTask(
         FROM_HERE, Bind(&MemoryDumpManager::FinalizeDumpAndAddToTrace,
                         Passed(&pmd_async_state)));
     return;
   }
 
   TRACE_EVENT_WITH_FLOW0(kTraceCategory,
                          "MemoryDumpManager::FinalizeDumpAndAddToTrace",
                          TRACE_ID_MANGLE(dump_guid), TRACE_EVENT_FLAG_FLOW_IN);
 
+  // The results struct to fill.
+  // TODO(hjd): Transitional until we send the full PMD. See crbug.com/704203
+  MemoryDumpCallbackResult result;
+
   for (const auto& kv : pmd_async_state->process_dumps) {
     ProcessId pid = kv.first;  // kNullProcessId for the current process.
     ProcessMemoryDump* process_memory_dump = kv.second.get();
     std::unique_ptr<TracedValue> traced_value(new TracedValue);
     process_memory_dump->AsValueInto(traced_value.get());
     traced_value->SetString("level_of_detail",
                             MemoryDumpLevelOfDetailToString(
                                 pmd_async_state->req_args.level_of_detail));
     const char* const event_name =
         MemoryDumpTypeToString(pmd_async_state->req_args.dump_type);
 
     std::unique_ptr<ConvertableToTraceFormat> event_value(
         std::move(traced_value));
     TRACE_EVENT_API_ADD_TRACE_EVENT_WITH_PROCESS_ID(
         TRACE_EVENT_PHASE_MEMORY_DUMP,
         TraceLog::GetCategoryGroupEnabled(kTraceCategory), event_name,
         trace_event_internal::kGlobalScope, dump_guid, pid,
         kTraceEventNumArgs, kTraceEventArgNames,
         kTraceEventArgTypes, nullptr /* arg_values */, &event_value,
         TRACE_EVENT_FLAG_HAS_ID);
+
+    // TODO(hjd): Transitional until we send the full PMD. See crbug.com/704203
+    if (pmd_async_state->req_args.level_of_detail ==
+        MemoryDumpLevelOfDetail::BACKGROUND) {
+      if (pid == kNullProcessId) {
+        result.chrome_dump.malloc_total_kb =
+            GetDumpsSumKb("malloc/", process_memory_dump);

[ssid, 2017/03/23 03:11:38]

This seems very fragile. the double counting depends on just the difference
between counting "malloc" and "malloc/".

[Primiano Tucci (use gerrit), 2017/03/23 12:27:58]

In this case I'd just count "malloc" (without any *, i.e. any recursion), which
is the outer total size.
This would remove any ambiguity about suballocations

[hjd, 2017/03/23 14:50:30]

Done.

+        result.chrome_dump.v8_total_kb =
+            GetDumpsSumKb("v8/", process_memory_dump);
+        // partition_alloc reports sizes for both allocated_objects and
+        // partitions. The memory allocated_objects uses is a subset of
+        // the partitions memory so to avoid double counting we only
+        // count partitions memory.
+        result.chrome_dump.partition_alloc_total_kb =
+            GetDumpsSumKb("partition_alloc/partitions", process_memory_dump);
+        result.chrome_dump.blink_gc_total_kb =
+            GetDumpsSumKb("blink_gc/", process_memory_dump);

[ssid, 2017/03/23 03:11:38]

So, in this case we would be counting the "allocated_objects" only and not
counting the total. Whereas in other cases like malloc we count total. Basically
this depends on if we create an explicit dump for the "unspecified" part.
I think a better solution here is to have pattern matching. Use
base::MatchPattern or similar.

Use malloc/* for the case of malloc.
Use blink_gc for blink gc's total.

Or if we are only interested in allocated_objects size then we should just use
malloc/allcoated_objects and blink_gc/allcoated_objects and
partition_alloc/allcoated_objects.

[Primiano Tucci (use gerrit), 2017/03/23 12:27:58]

We don't for blink_gc.
Since we have an explicit total dump, here I'd get only "blink_gc" (Again, with
no recursion). and like in the preivcous case, this should be fine regardless of
BACKGROUND / DETAILED.

Instead for PA we need partition_alloc/partitions/*

[hjd, 2017/03/23 14:50:31]

Done.

+      }
+    }
   }
 
   bool tracing_still_enabled;
   TRACE_EVENT_CATEGORY_GROUP_ENABLED(kTraceCategory, &tracing_still_enabled);
   if (!tracing_still_enabled) {
     pmd_async_state->dump_successful = false;
     VLOG(1) << kLogPrefix << " failed because tracing was disabled before"
             << " the dump was completed";
   }
 
@@ skipping 191 matching lines @@
   if (iter == process_dumps.end()) {
     std::unique_ptr<ProcessMemoryDump> new_pmd(
         new ProcessMemoryDump(session_state, dump_args));
     iter = process_dumps.insert(std::make_pair(pid, std::move(new_pmd))).first;
   }
   return iter->second.get();
 }
 
 }  // namespace trace_event
 }  // namespace base