[tracing] Switch to new heap dump format.

base/trace_event/memory_dump_manager.cc

Index: base/trace_event/memory_dump_manager.cc
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc
index 31016f970e3155cd18d749595d5dc432d38c5486..463067ab88f718404f8b08f09e962dc93642d733 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -22,8 +22,7 @@
 #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/heap_profiler_event_writer.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"
@@ -90,33 +89,6 @@ void OnGlobalDumpDone(MemoryDumpCallback wrapped_callback,
   }
 }
 
-// Proxy class which wraps a ConvertableToTraceFormat owned by the
-// |session_state| into a proxy object that can be added to the trace event log.
-// This is to solve the problem that the MemoryDumpSessionState is refcounted
-// but the tracing subsystem wants a std::unique_ptr<ConvertableToTraceFormat>.
-template <typename T>
-struct SessionStateConvertableProxy : public ConvertableToTraceFormat {
-  using GetterFunctPtr = T* (MemoryDumpSessionState::*)() const;
-
-  SessionStateConvertableProxy(
-      scoped_refptr<MemoryDumpSessionState> session_state,
-      GetterFunctPtr getter_function)
-      : session_state(session_state), getter_function(getter_function) {}
-
-  void AppendAsTraceFormat(std::string* out) const override {
-    return (session_state.get()->*getter_function)()->AppendAsTraceFormat(out);
-  }
-
-  void EstimateTraceMemoryOverhead(
-      TraceEventMemoryOverhead* overhead) override {
-    return (session_state.get()->*getter_function)()
-        ->EstimateTraceMemoryOverhead(overhead);
-  }
-
-  scoped_refptr<MemoryDumpSessionState> session_state;
-  GetterFunctPtr const getter_function;
-};
-
 }  // namespace
 
 // static
@@ -759,6 +731,31 @@ void MemoryDumpManager::FinalizeDumpAndAddToTrace(
         TRACE_EVENT_FLAG_HAS_ID);
   }
 
+  const size_t kHeapProfileNumArgs = 1;

[Primiano Tucci (use gerrit), 2017/03/09 11:47:45]

I need to dig the email thread about this discussion. What is the reason to not
keep having the heap dumps as part of the memory-infra dump? At the end they are
emitted at the same time and the names (malloc, partition_alloc, blink_gc) must
match the other columns.
So don't feel there is too much separation here.

+  const char* kHeapProfileArgNames[] = {"data"};
+  const unsigned char kHeapProfileArgTypes[] = {TRACE_VALUE_TYPE_CONVERTABLE};
+  for (const auto& kv : pmd_async_state->process_dumps) {
+    ProcessId pid = kv.first;
+    ProcessMemoryDump* process_memory_dump = kv.second.get();
+    if (pid != kNullProcessId) {
+      // We expect heap dumps only from the current process.
+      DCHECK(process_memory_dump->heap_dumps().empty());
+      continue;
+    }
+
+    auto event_data = ExportHeapProfileEventData(
+        process_memory_dump->heap_dumps(), *pmd_async_state->session_state);
+
+    std::unique_ptr<ConvertableToTraceFormat> values[kHeapProfileNumArgs] = {
+        std::move(event_data)};
+    TRACE_EVENT_API_ADD_TRACE_EVENT_WITH_PROCESS_ID(
+        TRACE_EVENT_PHASE_SAMPLE,
+        TraceLog::GetCategoryGroupEnabled(kTraceCategory), "heap_profile",
+        trace_event_internal::kGlobalScope, dump_guid, pid, kHeapProfileNumArgs,
+        kHeapProfileArgNames, kHeapProfileArgTypes, nullptr /* arg_values */,
+        values, TRACE_EVENT_FLAG_HAS_ID);
+  }
+
   bool tracing_still_enabled;
   TRACE_EVENT_CATEGORY_GROUP_ENABLED(kTraceCategory, &tracing_still_enabled);
   if (!tracing_still_enabled) {
@@ -804,26 +801,11 @@ void MemoryDumpManager::OnTraceLogEnabled() {
   session_state->set_heap_profiler_breakdown_threshold_bytes(
       memory_dump_config.heap_profiler_options.breakdown_threshold_bytes);
   if (heap_profiling_enabled_) {
-    // If heap profiling is enabled, the stack frame deduplicator and type name
-    // deduplicator will be in use. Add a metadata events to write the frames
-    // and type IDs.
-    session_state->SetStackFrameDeduplicator(
-        WrapUnique(new StackFrameDeduplicator));
-
-    session_state->SetTypeNameDeduplicator(
-        WrapUnique(new TypeNameDeduplicator));
-
-    TRACE_EVENT_API_ADD_METADATA_EVENT(
-        TraceLog::GetCategoryGroupEnabled("__metadata"), "stackFrames",
-        "stackFrames",
-        MakeUnique<SessionStateConvertableProxy<StackFrameDeduplicator>>(
-            session_state, &MemoryDumpSessionState::stack_frame_deduplicator));
-
-    TRACE_EVENT_API_ADD_METADATA_EVENT(
-        TraceLog::GetCategoryGroupEnabled("__metadata"), "typeNames",
-        "typeNames",
-        MakeUnique<SessionStateConvertableProxy<TypeNameDeduplicator>>(
-            session_state, &MemoryDumpSessionState::type_name_deduplicator));
+    // TODO(dskiba): support continuous mode
+    DCHECK(trace_config.GetTraceRecordMode() != RECORD_CONTINUOUSLY)

[Primiano Tucci (use gerrit), 2017/03/09 11:47:45]

I think  this should be a LOG_IF(ERROR, trace_config.GetTraceRecordMode() !=
RECORD_CONTINUOUSLY) not a dcheck.
would be great (but let's keep for another patchset) if we could inject some
metadata in the trace that generates a UI message in the traceviewer importer
that says something like: "you did record a trace with heap profiling in
continuous mode but that's really not supported".
Maybe can you file a bug abobut this and cc hjd?

[DmitrySkiba, 2017/03/14 22:12:48]

Done.

+        << "Heap profile format is incremental and doesn't yet support "
+        << "continuous mode.";
+    session_state->CreateDeduplicators();
   }
 
   std::unique_ptr<MemoryDumpScheduler> dump_scheduler(