Sampling heap profiler data structure changes

src/profiler/sampling-heap-profiler.cc

 // Copyright 2015 the V8 project 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 "src/profiler/sampling-heap-profiler.h"
 
 #include <stdint.h>
 #include <memory>
 #include "src/api.h"
 #include "src/base/utils/random-number-generator.h"
@@ skipping 24 matching lines @@
 
 SamplingHeapProfiler::SamplingHeapProfiler(Heap* heap, StringsStorage* names,
                                            uint64_t rate, int stack_depth)
     : isolate_(heap->isolate()),
       heap_(heap),
       new_space_observer_(new SamplingAllocationObserver(
           heap_, rate, rate, this, heap->isolate()->random_number_generator())),
       other_spaces_observer_(new SamplingAllocationObserver(
           heap_, rate, rate, this, heap->isolate()->random_number_generator())),
       names_(names),
+      profile_root_("(root)", v8::UnboundScript::kNoScriptId, 0),
       samples_(),
       stack_depth_(stack_depth) {
   heap->new_space()->AddAllocationObserver(new_space_observer_.get());
   AllSpaces spaces(heap);
   for (Space* space = spaces.next(); space != NULL; space = spaces.next()) {
     if (space != heap->new_space()) {
       space->AddAllocationObserver(other_spaces_observer_.get());
     }
   }
 }
 
 
 SamplingHeapProfiler::~SamplingHeapProfiler() {
   heap_->new_space()->RemoveAllocationObserver(new_space_observer_.get());
   AllSpaces spaces(heap_);
   for (Space* space = spaces.next(); space != NULL; space = spaces.next()) {
     if (space != heap_->new_space()) {
       space->RemoveAllocationObserver(other_spaces_observer_.get());
     }
   }
 
-  // Clear samples and drop all the weak references we are keeping.
-  std::set<SampledAllocation*>::iterator it;
-  for (it = samples_.begin(); it != samples_.end(); ++it) {
-    delete *it;
+  for (auto sample : samples_) {
+    delete sample;
   }
-  std::set<SampledAllocation*> empty;
-  samples_.swap(empty);
+  samples_.clear();

[ofrobots, 2016/02/18 01:02:24]

nit: It is more efficient to do a swap with an empty set. See the code on the
left.

[mattloring, 2016/02/18 01:30:04]

Done.

 }
 
 
 void SamplingHeapProfiler::SampleObject(Address soon_object, size_t size) {
   DisallowHeapAllocation no_allocation;
 
   HandleScope scope(isolate_);
   HeapObject* heap_object = HeapObject::FromAddress(soon_object);
   Handle<Object> obj(heap_object, isolate_);
 
   // Mark the new block as FreeSpace to make sure the heap is iterable while we
   // are taking the sample.
   heap()->CreateFillerObjectAt(soon_object, static_cast<int>(size));
 
   Local<v8::Value> loc = v8::Utils::ToLocal(obj);
 
-  SampledAllocation* sample =
-      new SampledAllocation(this, isolate_, loc, size, stack_depth_);
+  AllocationNode* node = AddStack();
+  node->allocations_[size]++;
+  Sample* sample = new Sample(size, node, loc, this);
   samples_.insert(sample);
+  sample->global.SetWeak(sample, OnWeakCallback, WeakCallbackType::kParameter);
 }
 
-
-void SamplingHeapProfiler::SampledAllocation::OnWeakCallback(
-    const WeakCallbackInfo<SampledAllocation>& data) {
-  SampledAllocation* sample = data.GetParameter();
-  sample->sampling_heap_profiler_->samples_.erase(sample);
+void SamplingHeapProfiler::OnWeakCallback(
+    const WeakCallbackInfo<Sample>& data) {
+  Sample* sample = data.GetParameter();
+  AllocationNode* node = sample->owner;
+  node->allocations_[sample->size]--;

[ofrobots, 2016/02/18 01:02:24]

map::operator[] silently insert a new entry in the map if it doesn't already
exist. Add a DCHECK before this point to ensure node->allocations_[sample->size]
!= 0.

[mattloring, 2016/02/18 01:30:04]

Done.

+  sample->profiler->samples_.erase(sample);
   delete sample;
 }
 
-
-SamplingHeapProfiler::FunctionInfo::FunctionInfo(SharedFunctionInfo* shared,
-                                                 StringsStorage* names)
-    : name_(names->GetFunctionName(shared->DebugName())),
-      script_name_(""),
-      script_id_(v8::UnboundScript::kNoScriptId),
-      start_position_(shared->start_position()) {
-  if (shared->script()->IsScript()) {
-    Script* script = Script::cast(shared->script());
-    script_id_ = script->id();
-    if (script->name()->IsName()) {
-      Name* name = Name::cast(script->name());
-      script_name_ = names->GetName(name);
+SamplingHeapProfiler::AllocationNode* SamplingHeapProfiler::FindOrAddChildNode(
+    AllocationNode* parent, const char* name, int script_id,
+    int start_position) {
+  for (AllocationNode* child : parent->children_) {
+    if (child->script_id_ == script_id &&
+        child->script_position_ == start_position &&
+        strcmp(child->name_, name) == 0) {

[ofrobots, 2016/02/18 01:02:24]

I think the earlier comment from Ulan was to DCHECK that the names match.

I don't think you should do strcmp here. This should indeed be a DCHECK. If you
ever run into a situation where the script id and start position are the same
but the name is different, there is something fundamentally wrong with our
understanding of the universe. The best action would be to abort. Hence a
DCHECK.

[mattloring, 2016/02/18 01:30:04]

Actually, it is possible for two scripts to have the same id but different
names. Right now our custom entries for (JS), (GC), ... all have a script_id of
kNoScriptId. The strcmp was added because we were incorrectly attributing ticks
to the wrong node.

+      return child;
     }
   }
+  AllocationNode* child = new AllocationNode(name, script_id, start_position);
+  parent->children_.push_back(child);
+  return child;
 }
 
+SamplingHeapProfiler::AllocationNode* SamplingHeapProfiler::AddStack() {
+  AllocationNode* node = &profile_root_;
 
-SamplingHeapProfiler::SampledAllocation::SampledAllocation(
-    SamplingHeapProfiler* sampling_heap_profiler, Isolate* isolate,
-    Local<Value> local, size_t size, int max_frames)
-    : sampling_heap_profiler_(sampling_heap_profiler),
-      global_(reinterpret_cast<v8::Isolate*>(isolate), local),
-      size_(size) {
-  global_.SetWeak(this, OnWeakCallback, WeakCallbackType::kParameter);
-
-  StackTraceFrameIterator it(isolate);
+  std::vector<SharedFunctionInfo*> stack;
+  StackTraceFrameIterator it(isolate_);
   int frames_captured = 0;
-  while (!it.done() && frames_captured < max_frames) {
+  while (!it.done() && frames_captured < stack_depth_) {
     JavaScriptFrame* frame = it.frame();
     SharedFunctionInfo* shared = frame->function()->shared();
-    stack_.push_back(new FunctionInfo(shared, sampling_heap_profiler->names()));
+    stack.push_back(shared);
 
     frames_captured++;
     it.Advance();
   }
 
   if (frames_captured == 0) {
     const char* name = nullptr;
-    switch (isolate->current_vm_state()) {
+    switch (isolate_->current_vm_state()) {
       case GC:
         name = "(GC)";
         break;
       case COMPILER:
         name = "(COMPILER)";
         break;
       case OTHER:
         name = "(V8 API)";
         break;
       case EXTERNAL:
         name = "(EXTERNAL)";
         break;
       case IDLE:
         name = "(IDLE)";
         break;
       case JS:
         name = "(JS)";
         break;
     }
-    stack_.push_back(new FunctionInfo(name));
+    return FindOrAddChildNode(node, name, v8::UnboundScript::kNoScriptId, 0);
   }
-}
-
-v8::AllocationProfile::Node* SamplingHeapProfiler::AllocateNode(
-    AllocationProfile* profile, const std::map<int, Script*>& scripts,
-    FunctionInfo* function_info) {
-  DCHECK(function_info->get_name());
-  DCHECK(function_info->get_script_name());
-
-  int line = v8::AllocationProfile::kNoLineNumberInfo;
-  int column = v8::AllocationProfile::kNoColumnNumberInfo;
-
-  if (function_info->get_script_id() != v8::UnboundScript::kNoScriptId) {
-    // Cannot use std::map<T>::at because it is not available on android.
-    auto non_const_scripts = const_cast<std::map<int, Script*>&>(scripts);
-    Handle<Script> script(non_const_scripts[function_info->get_script_id()]);
-
-    line =
-        1 + Script::GetLineNumber(script, function_info->get_start_position());
-    column = 1 + Script::GetColumnNumber(script,
-                                         function_info->get_start_position());
-  }
-
-  profile->nodes().push_back(v8::AllocationProfile::Node(
-      {ToApiHandle<v8::String>(isolate_->factory()->InternalizeUtf8String(
-           function_info->get_name())),
-       ToApiHandle<v8::String>(isolate_->factory()->InternalizeUtf8String(
-           function_info->get_script_name())),
-       function_info->get_script_id(), function_info->get_start_position(),
-       line, column, std::vector<v8::AllocationProfile::Node*>(),
-       std::vector<v8::AllocationProfile::Allocation>()}));
-
-  return &profile->nodes().back();
-}
-
-v8::AllocationProfile::Node* SamplingHeapProfiler::FindOrAddChildNode(
-    AllocationProfile* profile, const std::map<int, Script*>& scripts,
-    v8::AllocationProfile::Node* parent, FunctionInfo* function_info) {
-  for (v8::AllocationProfile::Node* child : parent->children) {
-    if (child->script_id == function_info->get_script_id() &&
-        child->start_position == function_info->get_start_position())
-      return child;
-  }
-  v8::AllocationProfile::Node* child =
-      AllocateNode(profile, scripts, function_info);
-  parent->children.push_back(child);
-  return child;
-}
-
-v8::AllocationProfile::Node* SamplingHeapProfiler::AddStack(
-    AllocationProfile* profile, const std::map<int, Script*>& scripts,
-    const std::vector<FunctionInfo*>& stack) {
-  v8::AllocationProfile::Node* node = profile->GetRootNode();
 
   // We need to process the stack in reverse order as the top of the stack is
   // the first element in the list.
   for (auto it = stack.rbegin(); it != stack.rend(); ++it) {
-    FunctionInfo* function_info = *it;
-    node = FindOrAddChildNode(profile, scripts, node, function_info);
+    SharedFunctionInfo* shared = *it;
+    const char* name = this->names()->GetFunctionName(shared->DebugName());
+    int script_id = v8::UnboundScript::kNoScriptId;
+    if (shared->script()->IsScript()) {
+      Script* script = Script::cast(shared->script());
+      script_id = script->id();
+    }
+    node = FindOrAddChildNode(node, name, script_id, shared->start_position());
   }
   return node;
 }
 
+v8::AllocationProfile::Node* SamplingHeapProfiler::TranslateAllocationNode(
+    AllocationProfile* profile, SamplingHeapProfiler::AllocationNode* node,
+    const std::map<int, Script*>& scripts) {
+  Local<v8::String> script_name =
+      ToApiHandle<v8::String>(isolate_->factory()->InternalizeUtf8String(""));
+  int line = v8::AllocationProfile::kNoLineNumberInfo;
+  int column = v8::AllocationProfile::kNoColumnNumberInfo;
+  std::vector<v8::AllocationProfile::Allocation> allocations;
+  if (node->script_id_ != v8::UnboundScript::kNoScriptId) {
+    // Cannot use std::map<T>::at because it is not available on android.
+    auto non_const_scripts = const_cast<std::map<int, Script*>&>(scripts);
+    Script* script = non_const_scripts[node->script_id_];
+    if (script->name()->IsName()) {
+      Name* name = Name::cast(script->name());
+      script_name = ToApiHandle<v8::String>(
+          isolate_->factory()->InternalizeUtf8String(names_->GetName(name)));
+    }
+    Handle<Script> script_handle(script);
+
+    line = 1 + Script::GetLineNumber(script_handle, node->script_position_);
+    column = 1 + Script::GetColumnNumber(script_handle, node->script_position_);
+    for (auto alloc : node->allocations_) {
+      allocations.push_back({alloc.first, alloc.second});
+    }
+  }
+
+  profile->nodes().push_back(v8::AllocationProfile::Node(
+      {ToApiHandle<v8::String>(
+           isolate_->factory()->InternalizeUtf8String(node->name_)),
+       script_name, node->script_id_, node->script_position_, line, column,
+       std::vector<v8::AllocationProfile::Node*>(), allocations}));
+  v8::AllocationProfile::Node* current = &profile->nodes().back();
+  for (auto child : node->children_) {
+    current->children.push_back(
+        TranslateAllocationNode(profile, child, scripts));
+  }
+  return current;
+}
 
 v8::AllocationProfile* SamplingHeapProfiler::GetAllocationProfile() {
   // To resolve positions to line/column numbers, we will need to look up
   // scripts. Build a map to allow fast mapping from script id to script.
   std::map<int, Script*> scripts;
   {
     Script::Iterator iterator(isolate_);
     Script* script;
     while ((script = iterator.Next())) {
       scripts[script->id()] = script;
     }
   }
 
   auto profile = new v8::internal::AllocationProfile();
 
-  // Create the root node.
-  FunctionInfo function_info("(root)");
-  AllocateNode(profile, scripts, &function_info);
-
-  for (SampledAllocation* allocation : samples_) {
-    v8::AllocationProfile::Node* node =
-        AddStack(profile, scripts, allocation->get_stack());
-    node->allocations.push_back({allocation->get_size(), 1});
-  }
+  TranslateAllocationNode(profile, &profile_root_, scripts);
 
   return profile;
 }
 
 
 }  // namespace internal
 }  // namespace v8