Heap Profiler: Add trace category group names as type names for allocations

base/trace_event/heap_profiler_allocation_context_tracker.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/heap_profiler_allocation_context_tracker.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/atomicops.h"
@@ skipping 67 matching lines @@
     g_tls_alloc_ctx_tracker.Set(tracker);
   }
 
   return tracker;
 }
 
 AllocationContextTracker::AllocationContextTracker()
     : thread_name_(nullptr), ignore_scope_depth_(0) {
   pseudo_stack_.reserve(kMaxStackDepth);
   task_contexts_.reserve(kMaxTaskDepth);
+  trace_categories_.reserve(kMaxStackDepth);
 }
 AllocationContextTracker::~AllocationContextTracker() {}
 
 // static
 void AllocationContextTracker::SetCurrentThreadName(const char* name) {
   if (name && capture_mode() != CaptureMode::DISABLED) {
     GetInstanceForCurrentThread()->thread_name_ = name;
   }
 }
 
 // static
 void AllocationContextTracker::SetCaptureMode(CaptureMode mode) {
   // When enabling capturing, also initialize the TLS slot. This does not create
   // a TLS instance yet.
   if (mode != CaptureMode::DISABLED && !g_tls_alloc_ctx_tracker.initialized())
     g_tls_alloc_ctx_tracker.Initialize(DestructAllocationContextTracker);
 
   // Release ordering ensures that when a thread observes |capture_mode_| to
   // be true through an acquire load, the TLS slot has been initialized.
   subtle::Release_Store(&capture_mode_, static_cast<int32_t>(mode));
 }
 
 void AllocationContextTracker::PushPseudoStackFrame(
+    const char* category,

[Primiano Tucci (use gerrit), 2016/08/25 16:31:15]

s/category/trace_event_category/ for consistency with trace_event_name below

[ssid, 2016/08/25 19:05:39]

I just wanted the declaration in header file to fit in one line :P now it is 2.

     const char* trace_event_name) {
   // Impose a limit on the height to verify that every push is popped, because
   // in practice the pseudo stack never grows higher than ~20 frames.
   if (pseudo_stack_.size() < kMaxStackDepth)
     pseudo_stack_.push_back(trace_event_name);
   else
     NOTREACHED();
+
+  // Ignore the category name if it has "," since it is confusing in UI.
+  if (strchr(category, ',') == nullptr &&

[Primiano Tucci (use gerrit), 2016/08/25 16:31:15]

Can we do this check in the UI? This is going to slow down every trace event
filtered.
Also, isn't this going to make the trace_categories_ stack out of sync?

[ssid, 2016/08/25 19:05:39]

Yess, that is the reason I have a separate stack for categories :P instead of
making the "pseudo_stack_" a vector of structs.
Ideally the check can actually be done in deduplicator itself instead of UI. The
reason I avoided doing this is because the allocations if they get a category
that has ",", then it is difficult to choose which one to take. If I ignore the
categories that have "," then we would still be left with some [unknown]. This
is better because the category is taken from the previous event in stack.

The other option would be to just take the first category if there are 2. Only
issue is we have both kinds of trace events like ones with "browser,startup" and
"startup,browser". So, strchr seems like a better option.

I have one other complex idea of having an array of boolean and the TraceLog
sends us the category_index aklong with category name. Using this the check for
"," can be done only once and every time we can read the array.

+      trace_categories_.size() < kMaxStackDepth) {
+    trace_categories_.push_back(category);
+  } else {
+    DCHECK_LE(trace_categories_.size(), kMaxStackDepth);
+  }
 }
 
 void AllocationContextTracker::PopPseudoStackFrame(
+    const char* category,
     const char* trace_event_name) {
   // Guard for stack underflow. If tracing was started with a TRACE_EVENT in
   // scope, the frame was never pushed, so it is possible that pop is called
   // on an empty stack.
   if (pseudo_stack_.empty())
     return;
 
   // Assert that pushes and pops are nested correctly. This DCHECK can be
   // hit if some TRACE_EVENT macro is unbalanced (a TRACE_EVENT_END* call
   // without a corresponding TRACE_EVENT_BEGIN).
   DCHECK_EQ(trace_event_name, pseudo_stack_.back())
       << "Encountered an unmatched TRACE_EVENT_END";
 
   pseudo_stack_.pop_back();
+
+  if (!trace_categories_.empty() && trace_categories_.back() == category)
+    trace_categories_.pop_back();
+  else
+    DCHECK(strchr(category, ','));
 }
 
 void AllocationContextTracker::PushCurrentTaskContext(const char* context) {
   DCHECK(context);
   if (task_contexts_.size() < kMaxTaskDepth)
     task_contexts_.push_back(context);
   else
     NOTREACHED();
 }
 
@@ skipping 41 matching lines @@
     *backtrace++ = StackFrame::FromThreadName(thread_name_);
   }
 
   switch (mode) {
     case CaptureMode::DISABLED:
       {
         break;
       }
     case CaptureMode::PSEUDO_STACK:
       {
-        for (const char* event_name: pseudo_stack_) {
+        for (const char* event_name : pseudo_stack_) {
           if (backtrace == backtrace_end) {
             break;
           }
           *backtrace++ = StackFrame::FromTraceEventName(event_name);
         }
         break;
       }
     case CaptureMode::NATIVE_STACK:
       {
         // Backtrace contract requires us to return bottom frames, i.e.
@@ skipping 23 matching lines @@
           *backtrace++ = StackFrame::FromProgramCounter(frame);
         }
         break;
       }
   }
 
   ctx.backtrace.frame_count = backtrace - std::begin(ctx.backtrace.frames);
 
   // TODO(ssid): Fix crbug.com/594803 to add file name as 3rd dimension
   // (component name) in the heap profiler and not piggy back on the type name.
-  ctx.type_name = task_contexts_.empty() ? nullptr : task_contexts_.back();
+  if (!task_contexts_.empty()) {
+    ctx.type_name = task_contexts_.back();
+  } else if (!trace_categories_.empty()) {
+    ctx.type_name = trace_categories_.back();
+  }
 
   return ctx;
 }
 
 }  // namespace trace_event
 }  // namespace base