[tracing] Turn StackFrame into struct.

base/trace_event/heap_profiler_allocation_context.h

Index: base/trace_event/heap_profiler_allocation_context.h
diff --git a/base/trace_event/heap_profiler_allocation_context.h b/base/trace_event/heap_profiler_allocation_context.h
index 98d835181d014fa57a29e2b193fcd9457c17b026..70ec7596661c8b2100e6d8083dbaddab36c0a4a7 100644
--- a/base/trace_event/heap_profiler_allocation_context.h
+++ b/base/trace_event/heap_profiler_allocation_context.h
@@ -35,14 +35,39 @@ namespace trace_event {
 //
 // See the design doc (https://goo.gl/4s7v7b) for more details.
 
-using StackFrame = const char*;
+// Represents (pseudo) stack frame. Used in Backtrace class below.
+//
+// Conceptually stack frame is identified by its value, and type is used
+// mostly to properly format the value. Value is expected to be a valid
+// pointer from process' address space.
+struct BASE_EXPORT StackFrame {
+  enum class Type {
+    TRACE_EVENT_NAME,   // const char* string
+    THREAD_NAME,        // const char* thread name
+  };
+
+  static StackFrame FromTraceEventName(const char* name) {
+    return {Type::TRACE_EVENT_NAME, name};
+  }
+  static StackFrame FromThreadName(const char* name) {
+    return {Type::THREAD_NAME, name};
+  }
+
+  Type type;
+  const void* value;

[Primiano Tucci (use gerrit), 2016/04/19 19:45:06]

since both of these are const char* should this also stay const char* so you can
avoid the casting below?

[Dmitry Skiba, 2016/04/19 22:14:14]

Yeah, but my next CL adds PROGRAM_COUNTER which is 'const void*' type. So it's
better to use 'const void*' from the start to avoid unnecessary changes.

[Primiano Tucci (use gerrit), 2016/04/20 13:17:59]

yeah I realized this too late. ignore my comment.

+};
+
+bool BASE_EXPORT operator < (const StackFrame& lhs, const StackFrame& rhs);
+bool BASE_EXPORT operator == (const StackFrame& lhs, const StackFrame& rhs);
+bool BASE_EXPORT operator != (const StackFrame& lhs, const StackFrame& rhs);
 
 struct BASE_EXPORT Backtrace {
-  // Unused backtrace frames are filled with nullptr frames. If the stack is
-  // higher than what can be stored here, the bottom frames are stored. Based
-  // on the data above, a depth of 12 captures the full stack in the vast
-  // majority of the cases.
-  StackFrame frames[12];
+  // If the stack is higher than what can be stored here, the bottom frames

[Primiano Tucci (use gerrit), 2016/04/19 19:45:06]

small req, not really related with your change: can you add a "(the one closer
to main() ) to "the bottom frames"? When it comes to stack for me is always a
pain to work out what people mean with top and bottom when it comes to a stack
:)

[Dmitry Skiba, 2016/04/19 22:14:14]

Actually, that is not true when it comes to native traces, hmm... The way it
works now is that I request (kMaxFrameCount + kKnownFrameCount) frames and get
top-bottom frames. I.e. to get bottom kMaxFrameCount ones I'll have to ask for
larger number of frames and then position to the bottom.

However, I think that the way it works now (reporting kMaxFrameCount top frames)
is more useful, because you see exact trace which caused an allocation.

[Primiano Tucci (use gerrit), 2016/04/20 13:17:59]

should we keep it consistent and just bump the depth when we use native frames?
Otherwise if you have two functions that reach inside the same final path, but
start for different points, they won't align.
I'd keep this this so that main() (or whatever) is always there. If necessary
bump the MaxFrameCount. THe all clustering logic  might get screwed if you keep
only the most leaf functions.

+  // are stored. Based on the data above, a depth of 12 captures the full
+  // stack in the vast majority of the cases.
+  enum { kMaxFrameCount = 12 };
+  StackFrame frames[kMaxFrameCount];
+  size_t frame_count;
 };
 
 // Struct to store the size and count of the allocations.
@@ -89,6 +114,11 @@ bool BASE_EXPORT operator==(const AllocationContext& lhs,
 namespace BASE_HASH_NAMESPACE {
 
 template <>
+struct BASE_EXPORT hash<base::trace_event::StackFrame> {
+  size_t operator()(const base::trace_event::StackFrame& frame) const;
+};
+
+template <>
 struct BASE_EXPORT hash<base::trace_event::Backtrace> {
   size_t operator()(const base::trace_event::Backtrace& backtrace) const;
 };