From af0428aca994fccfaddb2c7d723db13a2453f46b Mon Sep 17 00:00:00 2001 From: Peter Marshall Date: Wed, 2 Jan 2019 13:19:06 +0100 Subject: [PATCH] [cpu-profiler] Add source positions for inlined function calls Currently in both kCallerLineNumbers and kLeafNodeLineNumbers modes, we correctly capture inline stacks. In leaf number mode, this is simple as we simply add the path onto the existing tree. For caller line numbers mode this is more complex, because each path through various inlined function should be represented in the tree, even when there are multiple callsites to the same function inlined. Currently we don't correctly show line numbers for inlined functions. We do actually have this information though, which is generated by turbofan and stored in the source_position_table data structure on the code object. This also changes the behavior of the SourcePositionTable class. A problem we uncovered is that the PC that the sampler provides for every frame except the leaf is the return address of the calling frame. This address is *after* the call has already happened. It can be attributed to the next line of the function, rather than the calling line, which is wrong. We fix that here by using lower_bound in GetSourceLineNumber. The same problem happens in GetInlineStack - the PC of the caller is actually the instruction after the call. The information turbofan generates assumes that the instruction after the call is not part of the call (fair enough). To fix this we do the same thing as above - use lower_bound and then iterate back by one. TBR=alph@chromium.org Bug: v8:8575, v8:8606 Change-Id: Idc4bd4bdc8fb70b70ecc1a77a1e3744a86f83483 Reviewed-on: https://chromium-review.googlesource.com/c/1374290 Commit-Queue: Peter Marshall Reviewed-by: Tobias Tebbi Reviewed-by: Yang Guo Cr-Commit-Position: refs/heads/master@{#58545} --- src/profiler/profile-generator.cc | 37 ++++++---- src/profiler/profile-generator.h | 20 ++++-- src/profiler/profiler-listener.cc | 85 +++++++++++----------- src/source-position.cc | 1 + src/source-position.h | 1 + test/cctest/test-cpu-profiler.cc | 114 +++++++++++++++++++++++++++++- 6 files changed, 195 insertions(+), 63 deletions(-) diff --git a/src/profiler/profile-generator.cc b/src/profiler/profile-generator.cc index 5c0e7c4a37..3f3f40c19e 100644 --- a/src/profiler/profile-generator.cc +++ b/src/profiler/profile-generator.cc @@ -36,7 +36,7 @@ int SourcePositionTable::GetSourceLineNumber(int pc_offset) const { return v8::CpuProfileNode::kNoLineNumberInfo; } auto it = - std::upper_bound(pc_offsets_to_lines_.begin(), pc_offsets_to_lines_.end(), + std::lower_bound(pc_offsets_to_lines_.begin(), pc_offsets_to_lines_.end(), PCOffsetAndLineNumber{pc_offset, 0}); if (it != pc_offsets_to_lines_.begin()) --it; return it->line_number; @@ -119,17 +119,17 @@ int CodeEntry::GetSourceLine(int pc_offset) const { return v8::CpuProfileNode::kNoLineNumberInfo; } -void CodeEntry::AddInlineStack( - int pc_offset, std::vector> inline_stack) { +void CodeEntry::AddInlineStack(int pc_offset, + std::vector inline_stack) { EnsureRareData()->inline_locations_.insert( std::make_pair(pc_offset, std::move(inline_stack))); } -const std::vector>* CodeEntry::GetInlineStack( - int pc_offset) const { - if (!rare_data_) return nullptr; - auto it = rare_data_->inline_locations_.find(pc_offset); - return it != rare_data_->inline_locations_.end() ? &it->second : nullptr; +const std::vector* CodeEntry::GetInlineStack(int pc_offset) const { + if (!rare_data_ || rare_data_->inline_locations_.empty()) return nullptr; + auto it = rare_data_->inline_locations_.lower_bound(pc_offset); + if (it != rare_data_->inline_locations_.begin()) it--; + return it->second.empty() ? nullptr : &it->second; } void CodeEntry::set_deopt_info( @@ -759,15 +759,16 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) { int pc_offset = static_cast(stack_pos - entry->instruction_start()); // TODO(petermarshall): pc_offset can still be negative in some cases. - const std::vector>* inline_stack = + const std::vector* inline_stack = entry->GetInlineStack(pc_offset); if (inline_stack) { - std::transform( - inline_stack->rbegin(), inline_stack->rend(), - std::back_inserter(stack_trace), - [=](const std::unique_ptr& ptr) { - return CodeEntryAndLineNumber{ptr.get(), no_line_info}; - }); + std::transform(inline_stack->begin(), inline_stack->end(), + std::back_inserter(stack_trace), + [](const InlineEntry& inline_entry) { + return CodeEntryAndLineNumber{ + inline_entry.code_entry.get(), + inline_entry.call_line_number}; + }); } // Skip unresolved frames (e.g. internal frame) and get source line of // the first JS caller. @@ -779,6 +780,12 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) { src_line_not_found = false; } line_number = entry->GetSourceLine(pc_offset); + + // The inline stack contains the top-level function i.e. the same + // function as entry. We don't want to add it twice. The one from the + // inline stack has the correct line number for this particular inlining + // so we use it instead of pushing entry to stack_trace. + if (inline_stack) continue; } stack_trace.push_back({entry, line_number}); } diff --git a/src/profiler/profile-generator.h b/src/profiler/profile-generator.h index 516a28af18..4dd3fea8a1 100644 --- a/src/profiler/profile-generator.h +++ b/src/profiler/profile-generator.h @@ -49,6 +49,8 @@ class SourcePositionTable : public Malloced { DISALLOW_COPY_AND_ASSIGN(SourcePositionTable); }; +struct InlineEntry; + class CodeEntry { public: // CodeEntry doesn't own name strings, just references them. @@ -103,10 +105,8 @@ class CodeEntry { int GetSourceLine(int pc_offset) const; - void AddInlineStack(int pc_offset, - std::vector> inline_stack); - const std::vector>* GetInlineStack( - int pc_offset) const; + void AddInlineStack(int pc_offset, std::vector inline_stack); + const std::vector* GetInlineStack(int pc_offset) const; void set_instruction_start(Address start) { instruction_start_ = start; } Address instruction_start() const { return instruction_start_; } @@ -141,8 +141,7 @@ class CodeEntry { const char* deopt_reason_ = kNoDeoptReason; const char* bailout_reason_ = kEmptyBailoutReason; int deopt_id_ = kNoDeoptimizationId; - std::unordered_map>> - inline_locations_; + std::map> inline_locations_; std::vector deopt_inlined_frames_; }; @@ -188,6 +187,15 @@ class CodeEntry { DISALLOW_COPY_AND_ASSIGN(CodeEntry); }; +// Used to store information about inline call stacks - call_line_number is the +// line number within the function represented by code_entry. Inlining +// inherently happens at callsites, so this line number will always be the call +// to the next inline/noninline frame. +struct InlineEntry { + std::unique_ptr code_entry; + int call_line_number; +}; + struct CodeEntryAndLineNumber { CodeEntry* code_entry; int line_number; diff --git a/src/profiler/profiler-listener.cc b/src/profiler/profiler-listener.cc index 3a9bac7773..217e8a6f49 100644 --- a/src/profiler/profiler-listener.cc +++ b/src/profiler/profiler-listener.cc @@ -201,52 +201,57 @@ void ProfilerListener::RecordInliningInfo(CodeEntry* entry, if (!abstract_code->IsCode()) return; Code code = abstract_code->GetCode(); if (code->kind() != Code::OPTIMIZED_FUNCTION) return; - DeoptimizationData deopt_input_data = - DeoptimizationData::cast(code->deoptimization_data()); - int deopt_count = deopt_input_data->DeoptCount(); - for (int i = 0; i < deopt_count; i++) { - int pc_offset = deopt_input_data->Pc(i)->value(); - if (pc_offset == -1) continue; - int translation_index = deopt_input_data->TranslationIndex(i)->value(); - TranslationIterator it(deopt_input_data->TranslationByteArray(), - translation_index); - Translation::Opcode opcode = static_cast(it.Next()); - DCHECK_EQ(Translation::BEGIN, opcode); - it.Skip(Translation::NumberOfOperandsFor(opcode)); - int depth = 0; - std::vector> inline_stack; - while (it.HasNext() && - Translation::BEGIN != - (opcode = static_cast(it.Next()))) { - if (opcode != Translation::INTERPRETED_FRAME) { - it.Skip(Translation::NumberOfOperandsFor(opcode)); - continue; - } - it.Next(); // Skip ast_id - int shared_info_id = it.Next(); - it.Next(); // Skip height - it.Next(); // Skip return value offset - it.Next(); // Skip return value count - SharedFunctionInfo shared_info = SharedFunctionInfo::cast( - deopt_input_data->LiteralArray()->get(shared_info_id)); - if (!depth++) continue; // Skip the current function itself. + + // Needed for InliningStack(). + HandleScope scope(isolate_); + int last_inlining_id = -2; + for (SourcePositionTableIterator it(abstract_code->source_position_table()); + !it.done(); it.Advance()) { + int code_offset = it.code_offset(); + + // Save space by not duplicating repeated entries that map to the same + // inlining ID. We might get multiple source positions per inlining ID, but + // they all map to the same line. This automatically collapses adjacent + // inlining stacks (or empty stacks) that are exactly the same. + if (it.source_position().InliningId() == last_inlining_id) continue; + last_inlining_id = it.source_position().InliningId(); + + // Only look at positions for inlined calls. + if (it.source_position().InliningId() == SourcePosition::kNotInlined) { + entry->AddInlineStack(code_offset, std::vector()); + continue; + } + + std::vector stack = + it.source_position().InliningStack(handle(code, isolate_)); + std::vector inline_stack; + for (SourcePositionInfo& pos_info : stack) { + if (pos_info.position.ScriptOffset() == kNoSourcePosition) continue; + if (pos_info.script.is_null()) continue; + + int line_number = + pos_info.script->GetLineNumber(pos_info.position.ScriptOffset()) + 1; const char* resource_name = - (shared_info->script()->IsScript() && - Script::cast(shared_info->script())->name()->IsName()) - ? GetName(Name::cast(Script::cast(shared_info->script())->name())) + (pos_info.script->name()->IsName()) + ? GetName(Name::cast(pos_info.script->name())) : CodeEntry::kEmptyResourceName; - CodeEntry* inline_entry = - new CodeEntry(entry->tag(), GetName(shared_info->DebugName()), - resource_name, CpuProfileNode::kNoLineNumberInfo, - CpuProfileNode::kNoColumnNumberInfo, nullptr, - code->InstructionStart()); - inline_entry->FillFunctionInfo(shared_info); - inline_stack.emplace_back(inline_entry); + // We need the start line number and column number of the function for + // kLeafNodeLineNumbers mode. Creating a SourcePositionInfo is a handy way + // of getting both easily. + SourcePositionInfo start_pos_info( + SourcePosition(pos_info.shared->StartPosition()), pos_info.shared); + + std::unique_ptr inline_entry = base::make_unique( + entry->tag(), GetName(pos_info.shared->DebugName()), resource_name, + start_pos_info.line + 1, start_pos_info.column + 1, nullptr, + code->InstructionStart()); + inline_entry->FillFunctionInfo(*pos_info.shared); + inline_stack.push_back(InlineEntry{std::move(inline_entry), line_number}); } if (!inline_stack.empty()) { - entry->AddInlineStack(pc_offset, std::move(inline_stack)); + entry->AddInlineStack(code_offset, std::move(inline_stack)); } } } diff --git a/src/source-position.cc b/src/source-position.cc index c2ba4fa88b..b90c4d38cc 100644 --- a/src/source-position.cc +++ b/src/source-position.cc @@ -124,6 +124,7 @@ void SourcePosition::Print(std::ostream& out, Code code) const { SourcePositionInfo::SourcePositionInfo(SourcePosition pos, Handle f) : position(pos), + shared(f), script(f.is_null() || !f->script()->IsScript() ? Handle