From: loislo Date: Thu, 12 Feb 2015 19:51:26 +0000 (-0800) Subject: CPUProfiler: Push deopt reason further to ProfileNode. X-Git-Tag: upstream/4.7.83~4410 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d23ab23b05365b94c516845c55764334ba07fe26;p=platform%2Fupstream%2Fv8.git CPUProfiler: Push deopt reason further to ProfileNode. 1) create beefy RelocInfo table when cpu profiler is active, so if a function was optimized when profiler was active RelocInfo would get separate DeoptInfo for the each deopt case. 2) push DeoptInfo from CodeEntry to ProfileNode. When deopt happens we put the info collected on #1 into CodeEntry and record stack sample. On the sampling thread we grab the deopt data and append it to the corresponding ProfileNode deopts list. Sample profile dump. [Top down]: 0 (root) 0 #1 1 29 #2 1 test 29 #3 2 opt_function 29 #4 2 opt_function 29 #5 deopted at 118 with reason 'not a heap number' deopted at 137 with reason 'division by zero' BUG=452067 LOG=n Committed: https://crrev.com/ce8701b247d3c6604f24f17a90c02d17b4417f54 Cr-Commit-Position: refs/heads/master@{#26615} Review URL: https://codereview.chromium.org/919953002 Cr-Commit-Position: refs/heads/master@{#26630} --- diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc index fbe8e9959..080b526f1 100644 --- a/src/arm/assembler-arm.cc +++ b/src/arm/assembler-arm.cc @@ -3392,7 +3392,7 @@ void Assembler::RecordComment(const char* msg) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 8388999dc..5b6ed2caf 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -907,7 +907,8 @@ void LCodeGen::DeoptimizeIf(Condition condition, LInstruction* instr, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry.IsEquivalentTo(jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/src/arm64/assembler-arm64.cc b/src/arm64/assembler-arm64.cc index 78cbdff2b..d3a4fd2ef 100644 --- a/src/arm64/assembler-arm64.cc +++ b/src/arm64/assembler-arm64.cc @@ -3081,7 +3081,7 @@ void Assembler::RecordComment(const char* msg) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index b1358ccf6..ef01c91d4 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -1071,7 +1071,8 @@ void LCodeGen::DeoptimizeBranch( entry, deopt_info, bailout_type, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry->IsEquivalentTo(*jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/src/cpu-profiler-inl.h b/src/cpu-profiler-inl.h index b1adc1bc8..0320ed5a7 100644 --- a/src/cpu-profiler-inl.h +++ b/src/cpu-profiler-inl.h @@ -38,10 +38,7 @@ void CodeDisableOptEventRecord::UpdateCodeMap(CodeMap* code_map) { void CodeDeoptEventRecord::UpdateCodeMap(CodeMap* code_map) { CodeEntry* entry = code_map->FindEntry(start); - if (entry != NULL) { - entry->set_deopt_reason(deopt_reason); - entry->set_deopt_location(raw_position); - } + if (entry != NULL) entry->set_deopt_info(deopt_reason, raw_position); } diff --git a/src/deoptimizer.h b/src/deoptimizer.h index 35c3cbe66..91b6fca29 100644 --- a/src/deoptimizer.h +++ b/src/deoptimizer.h @@ -181,21 +181,12 @@ class Deoptimizer : public Malloced { DEOPT_MESSAGES_LIST(DEOPT_MESSAGES_CONSTANTS) kLastDeoptReason }; #undef DEOPT_MESSAGES_CONSTANTS - static const char* GetDeoptReason(DeoptReason deopt_reason); struct DeoptInfo { DeoptInfo(int r, const char* m, DeoptReason d) : raw_position(r), mnemonic(m), deopt_reason(d) {} - bool operator==(const DeoptInfo& other) const { - return raw_position == other.raw_position && - CStringEquals(mnemonic, other.mnemonic) && - deopt_reason == other.deopt_reason; - } - - bool operator!=(const DeoptInfo& other) const { return !(*this == other); } - int raw_position; const char* mnemonic; DeoptReason deopt_reason; @@ -214,8 +205,7 @@ class Deoptimizer : public Malloced { bool IsEquivalentTo(const JumpTableEntry& other) const { return address == other.address && bailout_type == other.bailout_type && - needs_frame == other.needs_frame && - (!FLAG_trace_deopt || deopt_info == other.deopt_info); + needs_frame == other.needs_frame; } Label label; diff --git a/src/ia32/assembler-ia32.cc b/src/ia32/assembler-ia32.cc index 719dabc3f..1d7e619ab 100644 --- a/src/ia32/assembler-ia32.cc +++ b/src/ia32/assembler-ia32.cc @@ -2652,7 +2652,7 @@ void Assembler::RecordComment(const char* msg, bool force) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 40a0ca3c2..d750cb87d 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -872,7 +872,8 @@ void LCodeGen::DeoptimizeIf(Condition cc, LInstruction* instr, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry.IsEquivalentTo(jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/src/mips/assembler-mips.cc b/src/mips/assembler-mips.cc index 96cccbb87..84946cc49 100644 --- a/src/mips/assembler-mips.cc +++ b/src/mips/assembler-mips.cc @@ -2355,7 +2355,7 @@ void Assembler::RecordComment(const char* msg) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index 46b5471f6..0dea629d3 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -870,7 +870,8 @@ void LCodeGen::DeoptimizeIf(Condition condition, LInstruction* instr, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry.IsEquivalentTo(jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/src/mips64/assembler-mips64.cc b/src/mips64/assembler-mips64.cc index 5ba16b5de..8f24e4381 100644 --- a/src/mips64/assembler-mips64.cc +++ b/src/mips64/assembler-mips64.cc @@ -2582,7 +2582,7 @@ void Assembler::RecordComment(const char* msg) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/mips64/lithium-codegen-mips64.cc b/src/mips64/lithium-codegen-mips64.cc index 49c465086..ae2e792f4 100644 --- a/src/mips64/lithium-codegen-mips64.cc +++ b/src/mips64/lithium-codegen-mips64.cc @@ -820,7 +820,8 @@ void LCodeGen::DeoptimizeIf(Condition condition, LInstruction* instr, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry.IsEquivalentTo(jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/src/ppc/assembler-ppc.cc b/src/ppc/assembler-ppc.cc index 322f74a98..38c8f0143 100644 --- a/src/ppc/assembler-ppc.cc +++ b/src/ppc/assembler-ppc.cc @@ -2196,7 +2196,7 @@ void Assembler::RecordComment(const char* msg) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/ppc/lithium-codegen-ppc.cc b/src/ppc/lithium-codegen-ppc.cc index 665c14e2e..4d17189c8 100644 --- a/src/ppc/lithium-codegen-ppc.cc +++ b/src/ppc/lithium-codegen-ppc.cc @@ -825,7 +825,8 @@ void LCodeGen::DeoptimizeIf(Condition cond, LInstruction* instr, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry.IsEquivalentTo(jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/src/profile-generator-inl.h b/src/profile-generator-inl.h index 3e8204618..d9f55dd44 100644 --- a/src/profile-generator-inl.h +++ b/src/profile-generator-inl.h @@ -25,7 +25,7 @@ CodeEntry::CodeEntry(Logger::LogEventsAndTags tag, const char* name, script_id_(v8::UnboundScript::kNoScriptId), no_frame_ranges_(NULL), bailout_reason_(kEmptyBailoutReason), - deopt_reason_(kEmptyBailoutReason), + deopt_reason_(kNoDeoptReason), deopt_location_(0), line_info_(line_info), instruction_start_(instruction_start) {} diff --git a/src/profile-generator.cc b/src/profile-generator.cc index 777be3fbd..292e3325d 100644 --- a/src/profile-generator.cc +++ b/src/profile-generator.cc @@ -8,6 +8,7 @@ #include "src/compiler.h" #include "src/debug.h" +#include "src/deoptimizer.h" #include "src/global-handles.h" #include "src/sampler.h" #include "src/scopeinfo.h" @@ -159,6 +160,7 @@ int JITLineInfoTable::GetSourceLineNumber(int pc_offset) const { const char* const CodeEntry::kEmptyNamePrefix = ""; const char* const CodeEntry::kEmptyResourceName = ""; const char* const CodeEntry::kEmptyBailoutReason = ""; +const char* const CodeEntry::kNoDeoptReason = ""; CodeEntry::~CodeEntry() { @@ -212,6 +214,12 @@ int CodeEntry::GetSourceLine(int pc_offset) const { } +void ProfileNode::CollectDeoptInfo(CodeEntry* entry) { + deopt_infos_.Add(DeoptInfo(entry->deopt_reason(), entry->deopt_location())); + entry->clear_deopt_info(); +} + + ProfileNode* ProfileNode::FindChild(CodeEntry* entry) { HashMap::Entry* map_entry = children_.Lookup(entry, CodeEntryHash(entry), false); @@ -223,13 +231,14 @@ ProfileNode* ProfileNode::FindChild(CodeEntry* entry) { ProfileNode* ProfileNode::FindOrAddChild(CodeEntry* entry) { HashMap::Entry* map_entry = children_.Lookup(entry, CodeEntryHash(entry), true); - if (map_entry->value == NULL) { + ProfileNode* node = reinterpret_cast(map_entry->value); + if (node == NULL) { // New node added. - ProfileNode* new_node = new ProfileNode(tree_, entry); - map_entry->value = new_node; - children_list_.Add(new_node); + node = new ProfileNode(tree_, entry); + map_entry->value = node; + children_list_.Add(node); } - return reinterpret_cast(map_entry->value); + return node; } @@ -268,12 +277,22 @@ bool ProfileNode::GetLineTicks(v8::CpuProfileNode::LineTick* entries, void ProfileNode::Print(int indent) { - base::OS::Print("%5u %*s %s%s %d #%d %s", self_ticks_, indent, "", + base::OS::Print("%5u %*s %s%s %d #%d", self_ticks_, indent, "", entry_->name_prefix(), entry_->name(), entry_->script_id(), - id(), entry_->bailout_reason()); + id()); if (entry_->resource_name()[0] != '\0') base::OS::Print(" %s:%d", entry_->resource_name(), entry_->line_number()); base::OS::Print("\n"); + for (auto info : deopt_infos_) { + base::OS::Print("%*s deopted at %d with reason '%s'\n", indent + 10, "", + info.deopt_location, info.deopt_reason); + } + const char* bailout_reason = entry_->bailout_reason(); + if (bailout_reason != GetBailoutReason(BailoutReason::kNoReason) && + bailout_reason != CodeEntry::kEmptyBailoutReason) { + base::OS::Print("%*s bailed out due to '%s'\n", indent + 10, "", + bailout_reason); + } for (HashMap::Entry* p = children_.Start(); p != NULL; p = children_.Next(p)) { @@ -310,13 +329,18 @@ ProfileTree::~ProfileTree() { ProfileNode* ProfileTree::AddPathFromEnd(const Vector& path, int src_line) { ProfileNode* node = root_; + CodeEntry* last_entry = NULL; for (CodeEntry** entry = path.start() + path.length() - 1; entry != path.start() - 1; --entry) { if (*entry != NULL) { node = node->FindOrAddChild(*entry); + last_entry = *entry; } } + if (last_entry && last_entry->has_deopt_info()) { + node->CollectDeoptInfo(last_entry); + } node->IncrementSelfTicks(); if (src_line != v8::CpuProfileNode::kNoLineNumberInfo) { node->IncrementLineTicks(src_line); diff --git a/src/profile-generator.h b/src/profile-generator.h index 679e92981..f7176a053 100644 --- a/src/profile-generator.h +++ b/src/profile-generator.h @@ -90,11 +90,20 @@ class CodeEntry { void set_bailout_reason(const char* bailout_reason) { bailout_reason_ = bailout_reason; } - void set_deopt_reason(const char* deopt_reason) { + const char* bailout_reason() const { return bailout_reason_; } + + void set_deopt_info(const char* deopt_reason, int location) { + DCHECK(!deopt_location_); deopt_reason_ = deopt_reason; + deopt_location_ = location; + } + const char* deopt_reason() const { return deopt_reason_; } + int deopt_location() const { return deopt_location_; } + bool has_deopt_info() const { return deopt_location_; } + void clear_deopt_info() { + deopt_reason_ = kNoDeoptReason; + deopt_location_ = 0; } - void set_deopt_location(int location) { deopt_location_ = location; } - const char* bailout_reason() const { return bailout_reason_; } static inline bool is_js_function_tag(Logger::LogEventsAndTags tag); @@ -118,6 +127,7 @@ class CodeEntry { static const char* const kEmptyNamePrefix; static const char* const kEmptyResourceName; static const char* const kEmptyBailoutReason; + static const char* const kNoDeoptReason; private: class TagField : public BitField {}; @@ -146,6 +156,17 @@ class CodeEntry { class ProfileTree; class ProfileNode { + private: + struct DeoptInfo { + DeoptInfo(const char* deopt_reason, int deopt_location) + : deopt_reason(deopt_reason), deopt_location(deopt_location) {} + DeoptInfo(const DeoptInfo& info) + : deopt_reason(info.deopt_reason), + deopt_location(info.deopt_location) {} + const char* deopt_reason; + int deopt_location; + }; + public: inline ProfileNode(ProfileTree* tree, CodeEntry* entry); @@ -162,6 +183,8 @@ class ProfileNode { unsigned int GetHitLineCount() const { return line_ticks_.occupancy(); } bool GetLineTicks(v8::CpuProfileNode::LineTick* entries, unsigned int length) const; + void CollectDeoptInfo(CodeEntry* entry); + const List& deopt_infos() const { return deopt_infos_; } void Print(int indent); @@ -186,6 +209,7 @@ class ProfileNode { unsigned id_; HashMap line_ticks_; + List deopt_infos_; DISALLOW_COPY_AND_ASSIGN(ProfileNode); }; diff --git a/src/x64/assembler-x64.cc b/src/x64/assembler-x64.cc index 710a9c9b0..cd9b7ff43 100644 --- a/src/x64/assembler-x64.cc +++ b/src/x64/assembler-x64.cc @@ -3451,7 +3451,7 @@ void Assembler::RecordComment(const char* msg, bool force) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index ca5c04cc9..f4d75775b 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -782,7 +782,8 @@ void LCodeGen::DeoptimizeIf(Condition cc, LInstruction* instr, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry.IsEquivalentTo(jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/src/x87/assembler-x87.cc b/src/x87/assembler-x87.cc index 9176b57eb..2fec0cd6c 100644 --- a/src/x87/assembler-x87.cc +++ b/src/x87/assembler-x87.cc @@ -1921,7 +1921,7 @@ void Assembler::RecordComment(const char* msg, bool force) { void Assembler::RecordDeoptReason(const int reason, const int raw_position) { - if (FLAG_trace_deopt) { + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling()) { EnsureSpace ensure_space(this); RecordRelocInfo(RelocInfo::POSITION, raw_position); RecordRelocInfo(RelocInfo::DEOPT_REASON, reason); diff --git a/src/x87/lithium-codegen-x87.cc b/src/x87/lithium-codegen-x87.cc index 13614b023..07be757ed 100644 --- a/src/x87/lithium-codegen-x87.cc +++ b/src/x87/lithium-codegen-x87.cc @@ -1154,7 +1154,8 @@ void LCodeGen::DeoptimizeIf(Condition cc, LInstruction* instr, !frame_is_built_); // We often have several deopts to the same entry, reuse the last // jump entry if this is the case. - if (jump_table_.is_empty() || + if (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || + jump_table_.is_empty() || !table_entry.IsEquivalentTo(jump_table_.last())) { jump_table_.Add(table_entry, zone()); } diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc index 1f7687909..d54aefa7a 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -32,6 +32,7 @@ #include "include/v8-profiler.h" #include "src/base/platform/platform.h" #include "src/cpu-profiler-inl.h" +#include "src/deoptimizer.h" #include "src/smart-pointers.h" #include "src/utils.h" #include "test/cctest/cctest.h" @@ -1720,23 +1721,33 @@ TEST(DontStopOnFinishedProfileDelete) { static const char* collect_deopt_events_test_source = - "function opt_function(value) {\n" - " return value / 10;\n" + "function opt_function(left, right, depth) {\n" + " if (depth) return opt_function(left, right, depth - 1);\n" + "\n" + " var k = left / 10;\n" + " var r = 10 / right;\n" + " return k + r;" "}\n" "\n" - "function test(value) {\n" - " return opt_function(value);\n" + "function test(left, right) {\n" + " return opt_function(left, right, 1);\n" "}\n" "\n" "startProfiling();\n" "\n" - "for (var i = 0; i < 10; ++i) test(10);\n" + "test(10, 10);\n" + "\n" + "%OptimizeFunctionOnNextCall(opt_function)\n" + "\n" + "test(10, 10);\n" + "\n" + "test(undefined, 10);\n" "\n" "%OptimizeFunctionOnNextCall(opt_function)\n" "\n" - "for (var i = 0; i < 10; ++i) test(10);\n" + "test(10, 10);\n" "\n" - "for (var i = 0; i < 10; ++i) test(undefined);\n" + "test(10, 0);\n" "\n" "stopProfiling();\n" "\n"; @@ -1756,9 +1767,16 @@ TEST(CollectDeoptEvents) { i::CpuProfile* iprofile = iprofiler->GetProfile(0); iprofile->Print(); v8::CpuProfile* profile = reinterpret_cast(iprofile); - const char* branch[] = {"", "test", "opt_function"}; + const char* branch[] = {"", "test", "opt_function", "opt_function"}; const v8::CpuProfileNode* opt_function = GetSimpleBranch( env->GetIsolate(), profile->GetTopDownRoot(), branch, arraysize(branch)); CHECK(opt_function); + const i::ProfileNode* iopt_function = + reinterpret_cast(opt_function); + CHECK_EQ(2, iopt_function->deopt_infos().length()); + CHECK_EQ(i::Deoptimizer::GetDeoptReason(i::Deoptimizer::kNotAHeapNumber), + iopt_function->deopt_infos()[0].deopt_reason); + CHECK_EQ(i::Deoptimizer::GetDeoptReason(i::Deoptimizer::kDivisionByZero), + iopt_function->deopt_infos()[1].deopt_reason); iprofiler->DeleteProfile(iprofile); }