[tracing] Turn StackFrame into struct.

base/trace_event/heap_profiler_allocation_context.cc

Index: base/trace_event/heap_profiler_allocation_context.cc
diff --git a/base/trace_event/heap_profiler_allocation_context.cc b/base/trace_event/heap_profiler_allocation_context.cc
index 038c083f0fa83241f9fef46c0ed239572d282d62..ad0f4c8365f0e99f9e65bc2c3595dd8eb46f5b5c 100644
--- a/base/trace_event/heap_profiler_allocation_context.cc
+++ b/base/trace_event/heap_profiler_allocation_context.cc
@@ -12,6 +12,18 @@
 namespace base {
 namespace trace_event {
 
+bool operator < (const StackFrame& lhs, const StackFrame& rhs) {
+  return std::memcmp(&lhs, &rhs, sizeof(StackFrame)) < 0;

[Primiano Tucci (use gerrit), 2016/04/14 10:15:33]

can we just compare the value? We'll never have two stack frame with same value
but different type, right?

+}
+
+bool operator == (const StackFrame& lhs, const StackFrame& rhs) {
+  return std::memcmp(&lhs, &rhs, sizeof(StackFrame)) == 0;

[Primiano Tucci (use gerrit), 2016/04/14 10:15:33]

same here

+}
+
+bool operator != (const StackFrame& lhs, const StackFrame& rhs) {
+  return !(lhs.value == rhs.value);
+}
+
 // Constructor that does not initialize members.
 AllocationContext::AllocationContext() {}
 
@@ -19,18 +31,17 @@ AllocationContext::AllocationContext() {}
 AllocationContext AllocationContext::Empty() {
   AllocationContext ctx;
 
-  for (size_t i = 0; i < arraysize(ctx.backtrace.frames); i++)
-    ctx.backtrace.frames[i] = nullptr;

[Primiano Tucci (use gerrit), 2016/04/14 10:15:33]

if you omit the null initialization of frames here, your equiality operator
below will be flaky. YOu will have no guarantee then that two Backtraces with
the same amount of frames will be bit identical.
I'd keep this nullptr initialization (And ditch the frame_count)

[Dmitry Skiba, 2016/04/15 07:01:39]

Equality operator was changed to compare only frame_count number of elements, so
no problem here.

-
+  ctx.backtrace.frame_count = 0;
   ctx.type_name = nullptr;
 
   return ctx;
 }
 
 bool operator==(const Backtrace& lhs, const Backtrace& rhs) {
-  // Pointer equality of the stack frames is assumed, so instead of doing a deep
-  // string comparison on all of the frames, a |memcmp| suffices.
-  return std::memcmp(lhs.frames, rhs.frames, sizeof(lhs.frames)) == 0;
+  if (lhs.frame_count != rhs.frame_count) return false;
+  return std::memcmp(lhs.frames,
+                     rhs.frames,
+                     lhs.frame_count * sizeof(StackFrame)) == 0;
 }
 
 bool operator==(const AllocationContext& lhs, const AllocationContext& rhs) {
@@ -43,10 +54,16 @@ bool operator==(const AllocationContext& lhs, const AllocationContext& rhs) {
 namespace BASE_HASH_NAMESPACE {
 using base::trace_event::AllocationContext;
 using base::trace_event::Backtrace;
+using base::trace_event::StackFrame;
+
+size_t hash<StackFrame>::operator()(const StackFrame& frame) const {
+  return base::SuperFastHash(reinterpret_cast<const char*>(&frame),

[Primiano Tucci (use gerrit), 2016/04/14 10:15:33]

as above, here i'd just return std::hash of the |value|, the type is just extra
information, but value itself is enough to disambiguate.

+                             sizeof(StackFrame));
+}
 
 size_t hash<Backtrace>::operator()(const Backtrace& backtrace) const {
   return base::SuperFastHash(reinterpret_cast<const char*>(backtrace.frames),
-                             sizeof(backtrace.frames));
+                             backtrace.frame_count * sizeof(StackFrame));
 }
 
 size_t hash<AllocationContext>::operator()(const AllocationContext& ctx) const {