Sampling heap profiler: use map instead of vector for children.

src/profiler/sampling-heap-profiler.h

Index: src/profiler/sampling-heap-profiler.h
diff --git a/src/profiler/sampling-heap-profiler.h b/src/profiler/sampling-heap-profiler.h
index e0dc004d5576b7cca55593152d85e8a6a2fc4992..d36d5ee5cef53b7979dfd5291744a50aa9949f42 100644
--- a/src/profiler/sampling-heap-profiler.h
+++ b/src/profiler/sampling-heap-profiler.h
@@ -80,13 +80,25 @@ class SamplingHeapProfiler {
           pinned_(false) {}
     ~AllocationNode() {
       for (auto child : children_) {
-        delete child;
+        delete child.second;
       }
     }
 
    private:
+    typedef uint64_t FunctionId;
+    static FunctionId function_id(int script_id, int start_position,

[ulan, 2016/05/13 09:05:00]

Please add a comment describing what this function returns and assumptions it
makes:
1. If script_id != kNoScriptId then start_position uniquely determines the node.
2. If script_id == kNoScriptId then name uniquely determines the node. Names
derived from VM state must not collide with the builtin names.

[alph, 2016/05/13 18:34:35]

Done.

+                                  const char* name) {

[ulan, 2016/05/12 10:01:20]

Instead of relying on name pointers, can we use "impossible" start_position to
encode the case with "(system)" names?

We already assume that start_position is at most 30-bits (see SharedFunctionInfo
comment).

So instead of storing script_id_ and script_position_ directly in node,
we could store NodeId (or NodePosition).
Something like:

struct NodeId {
 public:
  static NodeId fromFrame(script_id, script_position) {
     CHECK_LT(script_position, kMaxScriptPosition);
     ...
  }
  static NodeId fromSystemState(SystemState system_state) {
     NodeId result;
     result.script_id_ = kNoScriptId;
     result.script_position_ = kMaxScriptPosition +
static_cast<int>(system_state);
     ...
  }
  int script_id() { return script_id_; }
  int script_position() {
    return script_position_ < kMaxScriptPosition ? script_position_ : 0;
  }
 private:
  int script_id_;
  int script_position_;
};

It can be passed directly to std::map assuming that there is operator< defined.

wdyt?

+      return script_id != v8::UnboundScript::kNoScriptId
+                 ? (static_cast<uint64_t>(script_id) << 32) +
+                       (start_position << 1)

[ulan, 2016/05/13 09:05:00]

DCHECK(start_position >= 0 && start_position < (1 << 30));

[alph, 2016/05/13 18:34:35]

Done.

+                 : reinterpret_cast<intptr_t>(name) | 1;
+    }
+    AllocationNode* FindOrAddChildNode(const char* name, int script_id,
+                                       int start_position);
+    // TODO(alph): make use of unordered_map's here. Pay attention to
+    // iterator invalidation during TranslateAllocationNode.
     std::map<size_t, unsigned int> allocations_;
-    std::vector<AllocationNode*> children_;
+    std::map<FunctionId, AllocationNode*> children_;
     AllocationNode* const parent_;
     const int script_id_;
     const int script_position_;
@@ -118,8 +130,6 @@ class SamplingHeapProfiler {
   v8::AllocationProfile::Allocation ScaleSample(size_t size,
                                                 unsigned int count);
   AllocationNode* AddStack();
-  AllocationNode* FindOrAddChildNode(AllocationNode* parent, const char* name,
-                                     int script_id, int start_position);
 
   Isolate* const isolate_;
   Heap* const heap_;