CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h

We accessed to cpu_profiler for tracking SharedFunctionInfo objects movements and used their addresses for generating function_id. Actually we could replace the manually generated shared_id by the pair script_id + position. In this case we can drop SharedFunctionInfo events support from cpu_profiler and remove the dependency.

BTW GetCallUid was used as an unique identifier of the function on the front-end side. Actually it is a hash which might not be unique. So I renamed GetCallUid with GetHash and implemented GetFunctionId method.

BUG=452067
LOG=n

Review URL: https://codereview.chromium.org/941973002

Cr-Commit-Position: refs/heads/master@{#26775}

src/api.cc

@@ -7191,7 +7191,7 @@ unsigned CpuProfileNode::GetHitCount() const {
 
 
 unsigned CpuProfileNode::GetCallUid() const {
-  return reinterpret_cast<const i::ProfileNode*>(this)->entry()->GetCallUid();
+  return reinterpret_cast<const i::ProfileNode*>(this)->function_id();
 }
 
 

src/arm/lithium-codegen-arm.cc

@@ -9,6 +9,7 @@
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
+#include "src/cpu-profiler.h"
 #include "src/hydrogen-osr.h"
 #include "src/ic/ic.h"
 #include "src/ic/stub-cache.h"

src/arm64/lithium-codegen-arm64.cc

@@ -9,6 +9,7 @@
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
+#include "src/cpu-profiler.h"
 #include "src/hydrogen-osr.h"
 #include "src/ic/ic.h"
 #include "src/ic/stub-cache.h"

src/cpu-profiler-inl.h

@@ -17,9 +17,6 @@ namespace internal {
 
 void CodeCreateEventRecord::UpdateCodeMap(CodeMap* code_map) {
   code_map->AddCode(start, entry, size);
-  if (shared != NULL) {
-    entry->set_shared_id(code_map->GetSharedId(shared));
-  }
 }
 
 
@@ -42,11 +39,6 @@ void CodeDeoptEventRecord::UpdateCodeMap(CodeMap* code_map) {
 }
 
 
-void SharedFunctionInfoMoveEventRecord::UpdateCodeMap(CodeMap* code_map) {
-  code_map->MoveCode(from, to);
-}
-
-
 void ReportBuiltinEventRecord::UpdateCodeMap(CodeMap* code_map) {
   CodeEntry* entry = code_map->FindEntry(start);
   if (!entry) {

src/cpu-profiler.cc

@@ -201,7 +201,6 @@ void CpuProfiler::CallbackEvent(Name* name, Address entry_point) {
       Logger::CALLBACK_TAG,
       profiles_->GetName(name));
   rec->size = 1;
-  rec->shared = NULL;
   processor_->Enqueue(evt_rec);
 }
 
@@ -218,7 +217,6 @@ void CpuProfiler::CodeCreateEvent(Logger::LogEventsAndTags tag,
       CodeEntry::kEmptyResourceName, CpuProfileNode::kNoLineNumberInfo,
       CpuProfileNode::kNoColumnNumberInfo, NULL, code->instruction_start());
   rec->size = code->ExecutableSize();
-  rec->shared = NULL;
   processor_->Enqueue(evt_rec);
 }
 
@@ -235,7 +233,6 @@ void CpuProfiler::CodeCreateEvent(Logger::LogEventsAndTags tag,
       CodeEntry::kEmptyResourceName, CpuProfileNode::kNoLineNumberInfo,
       CpuProfileNode::kNoColumnNumberInfo, NULL, code->instruction_start());
   rec->size = code->ExecutableSize();
-  rec->shared = NULL;
   processor_->Enqueue(evt_rec);
 }
 
@@ -255,15 +252,8 @@ void CpuProfiler::CodeCreateEvent(Logger::LogEventsAndTags tag, Code* code,
   if (info) {
     rec->entry->set_no_frame_ranges(info->ReleaseNoFrameRanges());
   }
-  if (shared->script()->IsScript()) {
-    DCHECK(Script::cast(shared->script()));
-    Script* script = Script::cast(shared->script());
-    rec->entry->set_script_id(script->id()->value());
-    rec->entry->set_bailout_reason(
-        GetBailoutReason(shared->disable_optimization_reason()));
-  }
+  rec->entry->FillFunctionInfo(shared);
   rec->size = code->ExecutableSize();
-  rec->shared = shared->address();
   processor_->Enqueue(evt_rec);
 }
 
@@ -299,11 +289,8 @@ void CpuProfiler::CodeCreateEvent(Logger::LogEventsAndTags tag, Code* code,
   if (info) {
     rec->entry->set_no_frame_ranges(info->ReleaseNoFrameRanges());
   }
-  rec->entry->set_script_id(script->id()->value());
+  rec->entry->FillFunctionInfo(shared);
   rec->size = code->ExecutableSize();
-  rec->shared = shared->address();
-  rec->entry->set_bailout_reason(
-      GetBailoutReason(shared->disable_optimization_reason()));
   processor_->Enqueue(evt_rec);
 }
 
@@ -320,7 +307,6 @@ void CpuProfiler::CodeCreateEvent(Logger::LogEventsAndTags tag,
       CodeEntry::kEmptyResourceName, CpuProfileNode::kNoLineNumberInfo,
       CpuProfileNode::kNoColumnNumberInfo, NULL, code->instruction_start());
   rec->size = code->ExecutableSize();
-  rec->shared = NULL;
   processor_->Enqueue(evt_rec);
 }
 
@@ -360,16 +346,6 @@ void CpuProfiler::CodeDeleteEvent(Address from) {
 }
 
 
-void CpuProfiler::SharedFunctionInfoMoveEvent(Address from, Address to) {
-  CodeEventsContainer evt_rec(CodeEventRecord::SHARED_FUNC_MOVE);
-  SharedFunctionInfoMoveEventRecord* rec =
-      &evt_rec.SharedFunctionInfoMoveEventRecord_;
-  rec->from = from;
-  rec->to = to;
-  processor_->Enqueue(evt_rec);
-}
-
-
 void CpuProfiler::GetterCallbackEvent(Name* name, Address entry_point) {
   if (FilterOutCodeCreateEvent(Logger::CALLBACK_TAG)) return;
   CodeEventsContainer evt_rec(CodeEventRecord::CODE_CREATION);
@@ -380,7 +356,6 @@ void CpuProfiler::GetterCallbackEvent(Name* name, Address entry_point) {
       profiles_->GetName(name),
       "get ");
   rec->size = 1;
-  rec->shared = NULL;
   processor_->Enqueue(evt_rec);
 }
 
@@ -409,7 +384,6 @@ void CpuProfiler::SetterCallbackEvent(Name* name, Address entry_point) {
       profiles_->GetName(name),
       "set ");
   rec->size = 1;
-  rec->shared = NULL;
   processor_->Enqueue(evt_rec);
 }
 

src/cpu-profiler.h

@@ -28,7 +28,6 @@ class ProfileGenerator;
   V(CODE_MOVE, CodeMoveEventRecord)                      \
   V(CODE_DISABLE_OPT, CodeDisableOptEventRecord)         \
   V(CODE_DEOPT, CodeDeoptEventRecord)                    \
-  V(SHARED_FUNC_MOVE, SharedFunctionInfoMoveEventRecord) \
   V(REPORT_BUILTIN, ReportBuiltinEventRecord)
 
 
@@ -52,7 +51,6 @@ class CodeCreateEventRecord : public CodeEventRecord {
   Address start;
   CodeEntry* entry;
   unsigned size;
-  Address shared;
 
   INLINE(void UpdateCodeMap(CodeMap* code_map));
 };
@@ -86,15 +84,6 @@ class CodeDeoptEventRecord : public CodeEventRecord {
 };
 
 
-class SharedFunctionInfoMoveEventRecord : public CodeEventRecord {
- public:
-  Address from;
-  Address to;
-
-  INLINE(void UpdateCodeMap(CodeMap* code_map));
-};
-
-
 class ReportBuiltinEventRecord : public CodeEventRecord {
  public:
   Address start;
@@ -251,7 +240,7 @@ class CpuProfiler : public CodeEventListener {
   virtual void GetterCallbackEvent(Name* name, Address entry_point);
   virtual void RegExpCodeCreateEvent(Code* code, String* source);
   virtual void SetterCallbackEvent(Name* name, Address entry_point);
-  virtual void SharedFunctionInfoMoveEvent(Address from, Address to);
+  virtual void SharedFunctionInfoMoveEvent(Address from, Address to) {}
 
   INLINE(bool is_profiling() const) { return is_profiling_; }
   bool* is_profiling_address() {

src/deoptimizer.cc

@@ -6,6 +6,7 @@
 
 #include "src/accessors.h"
 #include "src/codegen.h"
+#include "src/cpu-profiler.h"
 #include "src/deoptimizer.h"
 #include "src/disasm.h"
 #include "src/full-codegen.h"

src/heap/heap-inl.h

@@ -8,7 +8,6 @@
 #include <cmath>
 
 #include "src/base/platform/platform.h"
-#include "src/cpu-profiler.h"
 #include "src/heap/heap.h"
 #include "src/heap/store-buffer.h"
 #include "src/heap/store-buffer-inl.h"
@@ -242,13 +241,9 @@ void Heap::OnMoveEvent(HeapObject* target, HeapObject* source,
     heap_profiler->ObjectMoveEvent(source->address(), target->address(),
                                    size_in_bytes);
   }
-
-  if (isolate_->logger()->is_logging_code_events() ||
-      isolate_->cpu_profiler()->is_profiling()) {
-    if (target->IsSharedFunctionInfo()) {
-      PROFILE(isolate_, SharedFunctionInfoMoveEvent(source->address(),
-                                                    target->address()));
-    }
+  if (target->IsSharedFunctionInfo()) {
+    LOG_CODE_EVENT(isolate_, SharedFunctionInfoMoveEvent(source->address(),
+                                                         target->address()));
   }
 
   if (FLAG_verify_predictable) {

src/ia32/lithium-codegen-ia32.cc

@@ -10,6 +10,7 @@
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
 #include "src/codegen.h"
+#include "src/cpu-profiler.h"
 #include "src/deoptimizer.h"
 #include "src/hydrogen-osr.h"
 #include "src/ia32/lithium-codegen-ia32.h"

src/ic/handler-compiler.cc

@@ -4,6 +4,7 @@
 
 #include "src/v8.h"
 
+#include "src/cpu-profiler.h"
 #include "src/ic/call-optimization.h"
 #include "src/ic/handler-compiler.h"
 #include "src/ic/ic.h"

src/ic/ic-compiler.cc

@@ -4,6 +4,7 @@
 
 #include "src/v8.h"
 
+#include "src/cpu-profiler.h"
 #include "src/ic/handler-compiler.h"
 #include "src/ic/ic-inl.h"
 #include "src/ic/ic-compiler.h"

src/log.cc

@@ -1412,8 +1412,6 @@ void Logger::SnapshotPositionEvent(Address addr, int pos) {
 
 
 void Logger::SharedFunctionInfoMoveEvent(Address from, Address to) {
-  PROFILER_LOG(SharedFunctionInfoMoveEvent(from, to));
-
   if (!is_logging_code_events()) return;
   MoveEventInternal(SHARED_FUNC_MOVE_EVENT, from, to);
 }

src/mips/lithium-codegen-mips.cc

@@ -30,6 +30,7 @@
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
+#include "src/cpu-profiler.h"
 #include "src/hydrogen-osr.h"
 #include "src/ic/ic.h"
 #include "src/ic/stub-cache.h"

src/mips64/lithium-codegen-mips64.cc

@@ -6,6 +6,7 @@
 
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
+#include "src/cpu-profiler.h"
 #include "src/hydrogen-osr.h"
 #include "src/ic/ic.h"
 #include "src/ic/stub-cache.h"

src/ppc/lithium-codegen-ppc.cc

@@ -7,6 +7,7 @@
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
+#include "src/cpu-profiler.h"
 #include "src/hydrogen-osr.h"
 #include "src/ic/ic.h"
 #include "src/ic/stub-cache.h"

src/profile-generator-inl.h

@@ -21,8 +21,8 @@ CodeEntry::CodeEntry(Logger::LogEventsAndTags tag, const char* name,
       resource_name_(resource_name),
       line_number_(line_number),
       column_number_(column_number),
-      shared_id_(0),
       script_id_(v8::UnboundScript::kNoScriptId),
+      position_(0),
       no_frame_ranges_(NULL),
       bailout_reason_(kEmptyBailoutReason),
       deopt_reason_(kNoDeoptReason),
@@ -48,6 +48,11 @@ ProfileNode::ProfileNode(ProfileTree* tree, CodeEntry* entry)
       children_(CodeEntriesMatch),
       id_(tree->next_node_id()),
       line_ticks_(LineTickMatch) {}
+
+
+inline unsigned ProfileNode::function_id() const {
+  return tree_->GetFunctionId(this);
+}
 } }  // namespace v8::internal
 
 #endif  // V8_PROFILE_GENERATOR_INL_H_

src/profile-generator.cc

@@ -169,10 +169,12 @@ CodeEntry::~CodeEntry() {
 }
 
 
-uint32_t CodeEntry::GetCallUid() const {
+uint32_t CodeEntry::GetHash() const {
   uint32_t hash = ComputeIntegerHash(tag(), v8::internal::kZeroHashSeed);
-  if (shared_id_ != 0) {
-    hash ^= ComputeIntegerHash(static_cast<uint32_t>(shared_id_),
+  if (script_id_ != v8::UnboundScript::kNoScriptId) {
+    hash ^= ComputeIntegerHash(static_cast<uint32_t>(script_id_),
+                               v8::internal::kZeroHashSeed);
+    hash ^= ComputeIntegerHash(static_cast<uint32_t>(position_),
                                v8::internal::kZeroHashSeed);
   } else {
     hash ^= ComputeIntegerHash(
@@ -190,13 +192,14 @@ uint32_t CodeEntry::GetCallUid() const {
 }
 
 
-bool CodeEntry::IsSameAs(CodeEntry* entry) const {
-  return this == entry ||
-         (tag() == entry->tag() && shared_id_ == entry->shared_id_ &&
-          (shared_id_ != 0 ||
-           (name_prefix_ == entry->name_prefix_ && name_ == entry->name_ &&
-            resource_name_ == entry->resource_name_ &&
-            line_number_ == entry->line_number_)));
+bool CodeEntry::IsSameFunctionAs(CodeEntry* entry) const {
+  if (this == entry) return true;
+  if (script_id_ != v8::UnboundScript::kNoScriptId) {
+    return script_id_ == entry->script_id_ && position_ == entry->position_;
+  }
+  return name_prefix_ == entry->name_prefix_ && name_ == entry->name_ &&
+         resource_name_ == entry->resource_name_ &&
+         line_number_ == entry->line_number_;
 }
 
 
@@ -214,6 +217,15 @@ int CodeEntry::GetSourceLine(int pc_offset) const {
 }
 
 
+void CodeEntry::FillFunctionInfo(SharedFunctionInfo* shared) {
+  if (!shared->script()->IsScript()) return;
+  Script* script = Script::cast(shared->script());
+  set_script_id(script->id()->value());
+  set_position(shared->start_position());
+  set_bailout_reason(GetBailoutReason(shared->disable_optimization_reason()));
+}
+
+
 void ProfileNode::CollectDeoptInfo(CodeEntry* entry) {
   deopt_infos_.Add(DeoptInfo(entry->deopt_reason(), entry->deopt_location()));
   entry->clear_deopt_info();
@@ -316,8 +328,9 @@ class DeleteNodesCallback {
 ProfileTree::ProfileTree()
     : root_entry_(Logger::FUNCTION_TAG, "(root)"),
       next_node_id_(1),
-      root_(new ProfileNode(this, &root_entry_)) {
-}
+      root_(new ProfileNode(this, &root_entry_)),
+      next_function_id_(1),
+      function_ids_(ProfileNode::CodeEntriesMatch) {}
 
 
 ProfileTree::~ProfileTree() {
@@ -326,6 +339,17 @@ ProfileTree::~ProfileTree() {
 }
 
 
+unsigned ProfileTree::GetFunctionId(const ProfileNode* node) {
+  CodeEntry* code_entry = node->entry();
+  HashMap::Entry* entry =
+      function_ids_.Lookup(code_entry, code_entry->GetHash(), true);
+  if (!entry->value) {
+    entry->value = reinterpret_cast<void*>(next_function_id_++);
+  }
+  return static_cast<unsigned>(reinterpret_cast<uintptr_t>(entry->value));
+}
+
+
 ProfileNode* ProfileTree::AddPathFromEnd(const Vector<CodeEntry*>& path,
                                          int src_line) {
   ProfileNode* node = root_;
@@ -427,7 +451,6 @@ void CpuProfile::Print() {
 }
 
 
-CodeEntry* const CodeMap::kSharedFunctionCodeEntry = NULL;
 const CodeMap::CodeTreeConfig::Key CodeMap::CodeTreeConfig::kNoKey = NULL;
 
 
@@ -469,22 +492,6 @@ CodeEntry* CodeMap::FindEntry(Address addr, Address* start) {
 }
 
 
-int CodeMap::GetSharedId(Address addr) {
-  CodeTree::Locator locator;
-  // For shared function entries, 'size' field is used to store their IDs.
-  if (tree_.Find(addr, &locator)) {
-    const CodeEntryInfo& entry = locator.value();
-    DCHECK(entry.entry == kSharedFunctionCodeEntry);
-    return entry.size;
-  } else {
-    tree_.Insert(addr, &locator);
-    int id = next_shared_id_++;
-    locator.set_value(CodeEntryInfo(kSharedFunctionCodeEntry, id));
-    return id;
-  }
-}
-
-
 void CodeMap::MoveCode(Address from, Address to) {
   if (from == to) return;
   CodeTree::Locator locator;
@@ -497,12 +504,7 @@ void CodeMap::MoveCode(Address from, Address to) {
 
 void CodeMap::CodeTreePrinter::Call(
     const Address& key, const CodeMap::CodeEntryInfo& value) {
-  // For shared function entries, 'size' field is used to store their IDs.
-  if (value.entry == kSharedFunctionCodeEntry) {
-    base::OS::Print("%p SharedFunctionInfo %d\n", key, value.size);
-  } else {
-    base::OS::Print("%p %5d %s\n", key, value.size, value.entry->name());
-  }
+  base::OS::Print("%p %5d %s\n", key, value.size, value.entry->name());
 }
 
 

src/profile-generator.h

@@ -84,9 +84,10 @@ class CodeEntry {
   int line_number() const { return line_number_; }
   int column_number() const { return column_number_; }
   const JITLineInfoTable* line_info() const { return line_info_; }
-  void set_shared_id(int shared_id) { shared_id_ = shared_id; }
   int script_id() const { return script_id_; }
   void set_script_id(int script_id) { script_id_ = script_id; }
+  int position() const { return position_; }
+  void set_position(int position) { position_ = position; }
   void set_bailout_reason(const char* bailout_reason) {
     bailout_reason_ = bailout_reason;
   }
@@ -105,6 +106,8 @@ class CodeEntry {
     deopt_location_ = 0;
   }
 
+  void FillFunctionInfo(SharedFunctionInfo* shared);
+
   static inline bool is_js_function_tag(Logger::LogEventsAndTags tag);
 
   List<OffsetRange>* no_frame_ranges() const { return no_frame_ranges_; }
@@ -117,8 +120,8 @@ class CodeEntry {
     return BuiltinIdField::decode(bit_field_);
   }
 
-  uint32_t GetCallUid() const;
-  bool IsSameAs(CodeEntry* entry) const;
+  uint32_t GetHash() const;
+  bool IsSameFunctionAs(CodeEntry* entry) const;
 
   int GetSourceLine(int pc_offset) const;
 
@@ -140,8 +143,8 @@ class CodeEntry {
   const char* resource_name_;
   int line_number_;
   int column_number_;
-  int shared_id_;
   int script_id_;
+  int position_;
   List<OffsetRange>* no_frame_ranges_;
   const char* bailout_reason_;
   const char* deopt_reason_;
@@ -180,6 +183,7 @@ class ProfileNode {
   unsigned self_ticks() const { return self_ticks_; }
   const List<ProfileNode*>* children() const { return &children_list_; }
   unsigned id() const { return id_; }
+  unsigned function_id() const;
   unsigned int GetHitLineCount() const { return line_ticks_.occupancy(); }
   bool GetLineTicks(v8::CpuProfileNode::LineTick* entries,
                     unsigned int length) const;
@@ -188,15 +192,13 @@ class ProfileNode {
 
   void Print(int indent);
 
- private:
   static bool CodeEntriesMatch(void* entry1, void* entry2) {
-    return reinterpret_cast<CodeEntry*>(entry1)->IsSameAs(
-        reinterpret_cast<CodeEntry*>(entry2));
+    return reinterpret_cast<CodeEntry*>(entry1)
+        ->IsSameFunctionAs(reinterpret_cast<CodeEntry*>(entry2));
   }
 
-  static uint32_t CodeEntryHash(CodeEntry* entry) {
-    return entry->GetCallUid();
-  }
+ private:
+  static uint32_t CodeEntryHash(CodeEntry* entry) { return entry->GetHash(); }
 
   static bool LineTickMatch(void* a, void* b) { return a == b; }
 
@@ -224,6 +226,7 @@ class ProfileTree {
       int src_line = v8::CpuProfileNode::kNoLineNumberInfo);
   ProfileNode* root() const { return root_; }
   unsigned next_node_id() { return next_node_id_++; }
+  unsigned GetFunctionId(const ProfileNode* node);
 
   void Print() {
     root_->Print(0);
@@ -237,6 +240,9 @@ class ProfileTree {
   unsigned next_node_id_;
   ProfileNode* root_;
 
+  unsigned next_function_id_;
+  HashMap function_ids_;
+
   DISALLOW_COPY_AND_ASSIGN(ProfileTree);
 };
 
@@ -281,7 +287,7 @@ class CpuProfile {
 
 class CodeMap {
  public:
-  CodeMap() : next_shared_id_(1) { }
+  CodeMap() {}
   void AddCode(Address addr, CodeEntry* entry, unsigned size);
   void MoveCode(Address from, Address to);
   CodeEntry* FindEntry(Address addr, Address* start = NULL);
@@ -315,11 +321,7 @@ class CodeMap {
 
   void DeleteAllCoveredCode(Address start, Address end);
 
-  // Fake CodeEntry pointer to distinguish shared function entries.
-  static CodeEntry* const kSharedFunctionCodeEntry;
-
   CodeTree tree_;
-  int next_shared_id_;
 
   DISALLOW_COPY_AND_ASSIGN(CodeMap);
 };

src/runtime/runtime-function.cc

@@ -7,6 +7,7 @@
 #include "src/accessors.h"
 #include "src/arguments.h"
 #include "src/compiler.h"
+#include "src/cpu-profiler.h"
 #include "src/deoptimizer.h"
 #include "src/frames.h"
 #include "src/runtime/runtime-utils.h"

src/serialize.cc

@@ -10,6 +10,7 @@
 #include "src/bootstrapper.h"
 #include "src/code-stubs.h"
 #include "src/compiler.h"
+#include "src/cpu-profiler.h"
 #include "src/deoptimizer.h"
 #include "src/execution.h"
 #include "src/global-handles.h"

src/x64/lithium-codegen-x64.cc

@@ -9,6 +9,7 @@
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
+#include "src/cpu-profiler.h"
 #include "src/hydrogen-osr.h"
 #include "src/ic/ic.h"
 #include "src/ic/stub-cache.h"

src/x87/lithium-codegen-x87.cc

@@ -10,6 +10,7 @@
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
 #include "src/codegen.h"
+#include "src/cpu-profiler.h"
 #include "src/deoptimizer.h"
 #include "src/hydrogen-osr.h"
 #include "src/ic/ic.h"