Skip to content

Commit

Permalink
CPUProfiler: Push deopt reason further to ProfileNode.
Browse files Browse the repository at this point in the history
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 v8#2
    5      test 29 v8#3
    3        opt_function 29 v8#4
                 deopted at 52 with reason 'not a heap number'
                 deopted at 71 with reason 'division by zero'

BUG=452067
LOG=n

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

Cr-Commit-Position: refs/heads/master@{#26615}
  • Loading branch information
loislo authored and Commit bot committed Feb 12, 2015
1 parent b79b985 commit ce8701b
Show file tree
Hide file tree
Showing 22 changed files with 101 additions and 49 deletions.
2 changes: 1 addition & 1 deletion src/arm/assembler-arm.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/arm/lithium-codegen-arm.cc
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/arm64/assembler-arm64.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/arm64/lithium-codegen-arm64.cc
Expand Up @@ -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());
}
Expand Down
5 changes: 1 addition & 4 deletions src/cpu-profiler-inl.h
Expand Up @@ -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);
}


Expand Down
11 changes: 1 addition & 10 deletions src/deoptimizer.h
Expand Up @@ -188,14 +188,6 @@ 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;
Expand All @@ -214,8 +206,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;
Expand Down
2 changes: 1 addition & 1 deletion src/ia32/assembler-ia32.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/ia32/lithium-codegen-ia32.cc
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/mips/assembler-mips.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/mips/lithium-codegen-mips.cc
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/mips64/assembler-mips64.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/mips64/lithium-codegen-mips64.cc
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/ppc/assembler-ppc.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/ppc/lithium-codegen-ppc.cc
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/profile-generator-inl.h
Expand Up @@ -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) {}
Expand Down
36 changes: 28 additions & 8 deletions src/profile-generator.cc
Expand Up @@ -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"
Expand Down Expand Up @@ -158,7 +159,9 @@ 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::kEmptyBailoutReason =
GetBailoutReason(BailoutReason::kNoReason);
const char* const CodeEntry::kNoDeoptReason = "";


CodeEntry::~CodeEntry() {
Expand Down Expand Up @@ -212,6 +215,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);
Expand All @@ -223,13 +232,15 @@ 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<ProfileNode*>(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<ProfileNode*>(map_entry->value);
if (entry->has_deopt_info()) node->CollectDeoptInfo(entry);
return node;
}


Expand Down Expand Up @@ -268,12 +279,21 @@ 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)) {
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)) {
Expand Down
31 changes: 28 additions & 3 deletions src/profile-generator.h
Expand Up @@ -90,11 +90,21 @@ 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_reason_ == kNoDeoptReason);
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_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);

Expand All @@ -118,6 +128,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<Logger::LogEventsAndTags, 0, 8> {};
Expand Down Expand Up @@ -146,6 +157,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);

Expand All @@ -162,6 +184,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<DeoptInfo>& deopt_infos() const { return deopt_infos_; }

void Print(int indent);

Expand All @@ -186,6 +210,7 @@ class ProfileNode {
unsigned id_;
HashMap line_ticks_;

List<DeoptInfo> deopt_infos_;
DISALLOW_COPY_AND_ASSIGN(ProfileNode);
};

Expand Down
2 changes: 1 addition & 1 deletion src/x64/assembler-x64.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/x64/lithium-codegen-x64.cc
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/x87/assembler-x87.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/x87/lithium-codegen-x87.cc
Expand Up @@ -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());
}
Expand Down
25 changes: 18 additions & 7 deletions test/cctest/test-cpu-profiler.cc
Expand Up @@ -1720,23 +1720,31 @@ TEST(DontStopOnFinishedProfileDelete) {


static const char* collect_deopt_events_test_source =
"function opt_function(value) {\n"
" return value / 10;\n"
"function opt_function(left, right) {\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);\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"
"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(undefined, 10);\n"
"\n"
"%OptimizeFunctionOnNextCall(opt_function)\n"
"\n"
"test(10, 10);\n"
"\n"
"test(10, 0);\n"
"\n"
"stopProfiling();\n"
"\n";
Expand All @@ -1760,5 +1768,8 @@ 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<const i::ProfileNode*>(opt_function);
CHECK_EQ(2, iopt_function->deopt_infos().length());
iprofiler->DeleteProfile(iprofile);
}

0 comments on commit ce8701b

Please sign in to comment.