From: loislo Date: Thu, 12 Feb 2015 14:36:21 +0000 (-0800) Subject: Revert of CPUProfiler: Push deopt reason further to ProfileNode. (patchset #1 id... X-Git-Tag: upstream/4.7.83~4424 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cb6ea146dc8813f7dc8fa7445d6d0f542509f317;p=platform%2Fupstream%2Fv8.git Revert of CPUProfiler: Push deopt reason further to ProfileNode. (patchset #1 id:1 of https://codereview.chromium.org/919953002/) Reason for revert: static initializers broke the build Original issue's description: > 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 > 5 test 29 #3 > 3 opt_function 29 #4 > deopted at 52 with reason 'not a heap number' > deopted at 71 with reason 'division by zero' > > BUG=452067 > LOG=n > > Committed: https://crrev.com/ce8701b247d3c6604f24f17a90c02d17b4417f54 > Cr-Commit-Position: refs/heads/master@{#26615} TBR=jarin@chromium.org,svenpanne@chromium.org,yurys@chromium.org,alph@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=452067 Review URL: https://codereview.chromium.org/915173005 Cr-Commit-Position: refs/heads/master@{#26616} --- diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc index 080b526..fbe8e99 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 5b6ed2c..8388999 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -907,8 +907,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 d3a4fd2..78cbdff 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 ef01c91..b1358cc 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -1071,8 +1071,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 0320ed5..b1adc1b 100644 --- a/src/cpu-profiler-inl.h +++ b/src/cpu-profiler-inl.h @@ -38,7 +38,10 @@ void CodeDisableOptEventRecord::UpdateCodeMap(CodeMap* code_map) { void CodeDeoptEventRecord::UpdateCodeMap(CodeMap* code_map) { CodeEntry* entry = code_map->FindEntry(start); - if (entry != NULL) entry->set_deopt_info(deopt_reason, raw_position); + if (entry != NULL) { + entry->set_deopt_reason(deopt_reason); + entry->set_deopt_location(raw_position); + } } diff --git a/src/deoptimizer.h b/src/deoptimizer.h index e48848a..35c3cbe 100644 --- a/src/deoptimizer.h +++ b/src/deoptimizer.h @@ -188,6 +188,14 @@ class Deoptimizer : public Malloced { 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; @@ -206,7 +214,8 @@ class Deoptimizer : public Malloced { bool IsEquivalentTo(const JumpTableEntry& other) const { return address == other.address && bailout_type == other.bailout_type && - needs_frame == other.needs_frame; + needs_frame == other.needs_frame && + (!FLAG_trace_deopt || deopt_info == other.deopt_info); } Label label; diff --git a/src/ia32/assembler-ia32.cc b/src/ia32/assembler-ia32.cc index 1d7e619..719dabc 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 d750cb8..40a0ca3 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -872,8 +872,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 84946cc..96cccbb 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 0dea629..46b5471 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -870,8 +870,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 8f24e43..5ba16b5 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 ae2e792..49c4650 100644 --- a/src/mips64/lithium-codegen-mips64.cc +++ b/src/mips64/lithium-codegen-mips64.cc @@ -820,8 +820,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 38c8f01..322f74a 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 4d17189..665c14e 100644 --- a/src/ppc/lithium-codegen-ppc.cc +++ b/src/ppc/lithium-codegen-ppc.cc @@ -825,8 +825,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 d9f55dd..3e82046 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_(kNoDeoptReason), + deopt_reason_(kEmptyBailoutReason), deopt_location_(0), line_info_(line_info), instruction_start_(instruction_start) {} diff --git a/src/profile-generator.cc b/src/profile-generator.cc index 490cb83..777be3f 100644 --- a/src/profile-generator.cc +++ b/src/profile-generator.cc @@ -8,7 +8,6 @@ #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,9 +158,7 @@ int JITLineInfoTable::GetSourceLineNumber(int pc_offset) const { const char* const CodeEntry::kEmptyNamePrefix = ""; const char* const CodeEntry::kEmptyResourceName = ""; -const char* const CodeEntry::kEmptyBailoutReason = - GetBailoutReason(BailoutReason::kNoReason); -const char* const CodeEntry::kNoDeoptReason = ""; +const char* const CodeEntry::kEmptyBailoutReason = ""; CodeEntry::~CodeEntry() { @@ -215,12 +212,6 @@ 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); @@ -232,15 +223,13 @@ ProfileNode* ProfileNode::FindChild(CodeEntry* entry) { ProfileNode* ProfileNode::FindOrAddChild(CodeEntry* entry) { HashMap::Entry* map_entry = children_.Lookup(entry, CodeEntryHash(entry), true); - ProfileNode* node = reinterpret_cast(map_entry->value); - if (node == NULL) { + if (map_entry->value == NULL) { // New node added. - node = new ProfileNode(tree_, entry); - map_entry->value = node; - children_list_.Add(node); + ProfileNode* new_node = new ProfileNode(tree_, entry); + map_entry->value = new_node; + children_list_.Add(new_node); } - if (entry->has_deopt_info()) node->CollectDeoptInfo(entry); - return node; + return reinterpret_cast(map_entry->value); } @@ -279,21 +268,12 @@ bool ProfileNode::GetLineTicks(v8::CpuProfileNode::LineTick* entries, void ProfileNode::Print(int indent) { - base::OS::Print("%5u %*s %s%s %d #%d", self_ticks_, indent, "", + base::OS::Print("%5u %*s %s%s %d #%d %s", self_ticks_, indent, "", entry_->name_prefix(), entry_->name(), entry_->script_id(), - id()); + id(), entry_->bailout_reason()); 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)) { - 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)) { diff --git a/src/profile-generator.h b/src/profile-generator.h index c5d778e..679e929 100644 --- a/src/profile-generator.h +++ b/src/profile-generator.h @@ -90,21 +90,11 @@ class CodeEntry { void set_bailout_reason(const char* bailout_reason) { bailout_reason_ = bailout_reason; } - const char* bailout_reason() const { return bailout_reason_; } - - void set_deopt_info(const char* deopt_reason, int location) { - DCHECK(deopt_reason_ == kNoDeoptReason); - DCHECK(!deopt_location_); + void set_deopt_reason(const char* deopt_reason) { 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_reason_ != kNoDeoptReason; } - 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); @@ -128,7 +118,6 @@ 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 {}; @@ -157,17 +146,6 @@ 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); @@ -184,8 +162,6 @@ 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); @@ -210,7 +186,6 @@ 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 cd9b7ff..710a9c9 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 f4d7577..ca5c04c 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -782,8 +782,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 2fec0cd..9176b57 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 || isolate()->cpu_profiler()->is_profiling()) { + if (FLAG_trace_deopt) { 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 07be757..13614b0 100644 --- a/src/x87/lithium-codegen-x87.cc +++ b/src/x87/lithium-codegen-x87.cc @@ -1154,8 +1154,7 @@ 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 (FLAG_trace_deopt || isolate()->cpu_profiler()->is_profiling() || - jump_table_.is_empty() || + if (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 1ed0805..1f76879 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -1720,31 +1720,23 @@ TEST(DontStopOnFinishedProfileDelete) { static const char* collect_deopt_events_test_source = - "function opt_function(left, right) {\n" - " var k = left / 10;\n" - " var r = 10 / right;\n" - " return k + r;" + "function opt_function(value) {\n" + " return value / 10;\n" "}\n" "\n" - "function test(left, right) {\n" - " return opt_function(left, right);\n" + "function test(value) {\n" + " return opt_function(value);\n" "}\n" "\n" "startProfiling();\n" "\n" - "test(10, 10);\n" + "for (var i = 0; i < 10; ++i) test(10);\n" "\n" "%OptimizeFunctionOnNextCall(opt_function)\n" "\n" - "test(10, 10);\n" + "for (var i = 0; i < 10; ++i) test(10);\n" "\n" - "test(undefined, 10);\n" - "\n" - "%OptimizeFunctionOnNextCall(opt_function)\n" - "\n" - "test(10, 10);\n" - "\n" - "test(10, 0);\n" + "for (var i = 0; i < 10; ++i) test(undefined);\n" "\n" "stopProfiling();\n" "\n"; @@ -1768,8 +1760,5 @@ TEST(CollectDeoptEvents) { 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()); iprofiler->DeleteProfile(iprofile); }