[tracing] Turn StackFrame into struct.

base/trace_event/heap_profiler_allocation_context.h

 // 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.
 
 #ifndef BASE_TRACE_EVENT_HEAP_PROFILER_ALLOCATION_CONTEXT_H_
 #define BASE_TRACE_EVENT_HEAP_PROFILER_ALLOCATION_CONTEXT_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 17 matching lines @@
 // The number of stack frames stored in the backtrace is a trade off between
 // memory used for tracing and accuracy. Measurements done on a prototype
 // revealed that:
 //
 // - In 60 percent of the cases, stack depth <= 7.
 // - In 87 percent of the cases, stack depth <= 9.
 // - In 95 percent of the cases, stack depth <= 11.
 //
 // 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.
 struct AllocationMetrics {
   size_t size;
   size_t count;
 };
 
 bool BASE_EXPORT operator==(const Backtrace& lhs, const Backtrace& rhs);
 
@@ skipping 26 matching lines @@
 
 bool BASE_EXPORT operator==(const AllocationContext& lhs,
                             const AllocationContext& rhs);
 
 }  // namespace trace_event
 }  // namespace base
 
 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;
 };
 
 template <>
 struct BASE_EXPORT hash<base::trace_event::AllocationContext> {
   size_t operator()(const base::trace_event::AllocationContext& context) const;
 };
 
 }  // BASE_HASH_NAMESPACE
 
 #endif  // BASE_TRACE_EVENT_HEAP_PROFILER_ALLOCATION_CONTEXT_H_