[cpu-profiler] Fix bugs and add tests for JITLineInfoTable

Looking up line numbers with the JITLineInfoTable would sometimes give
wrong answers. Fix these bugs and add a cctest for this data structure.

Also do some cleanup while we're here like inlining the (empty)
constructor and destructor and removing the empty() method which is
only used unnecessarily anyway, to make the contract of
GetSourceLineNumber a bit clearer.

Also rename the data structure to SourcePositionTable, because it
doesn't just provide info for JIT code, but also bytecode, and 'Info'
is pretty ambiguous.

Bug: v8:7018
Change-Id: I126581c844d85df6b2b3f80f2f5acbce01c16ba1
Reviewed-on: https://chromium-review.googlesource.com/1006795
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52571}
This commit is contained in:
Peter Marshall 2018-04-11 15:09:15 +02:00 committed by Commit Bot
parent c6e3e1c0a8
commit 4feb5ce7fd
6 changed files with 77 additions and 35 deletions

View File

@ -13,7 +13,7 @@ namespace internal {
CodeEntry::CodeEntry(CodeEventListener::LogEventsAndTags tag, const char* name,
const char* name_prefix, const char* resource_name,
int line_number, int column_number,
std::unique_ptr<JITLineInfoTable> line_info,
std::unique_ptr<SourcePositionTable> line_info,
Address instruction_start)
: bit_field_(TagField::encode(tag) |
BuiltinIdField::encode(Builtins::builtin_count)),

View File

@ -18,27 +18,23 @@
namespace v8 {
namespace internal {
JITLineInfoTable::JITLineInfoTable() {}
JITLineInfoTable::~JITLineInfoTable() {}
void JITLineInfoTable::SetPosition(int pc_offset, int line) {
void SourcePositionTable::SetPosition(int pc_offset, int line) {
DCHECK_GE(pc_offset, 0);
DCHECK_GT(line, 0); // The 1-based number of the source line.
if (GetSourceLineNumber(pc_offset) != line) {
pc_offset_map_.insert(std::make_pair(pc_offset, line));
auto result = pc_offset_map_.insert(std::make_pair(pc_offset, line));
// Check that a new element was inserted.
USE(result);
DCHECK(result.second);
}
}
int SourcePositionTable::GetSourceLineNumber(int pc_offset) const {
if (pc_offset_map_.empty()) return v8::CpuProfileNode::kNoLineNumberInfo;
int JITLineInfoTable::GetSourceLineNumber(int pc_offset) const {
PcOffsetMap::const_iterator it = pc_offset_map_.lower_bound(pc_offset);
if (it == pc_offset_map_.end()) {
if (pc_offset_map_.empty()) return v8::CpuProfileNode::kNoLineNumberInfo;
return (--pc_offset_map_.end())->second;
PcOffsetMap::const_iterator it = pc_offset_map_.upper_bound(pc_offset);
if (it != pc_offset_map_.begin()) {
return (--it)->second;
}
return it->second;
}
@ -121,9 +117,7 @@ void CodeEntry::SetBuiltinId(Builtins::Name id) {
int CodeEntry::GetSourceLine(int pc_offset) const {
if (line_info_ && !line_info_->empty()) {
return line_info_->GetSourceLineNumber(pc_offset);
}
if (line_info_) return line_info_->GetSourceLineNumber(pc_offset);
return v8::CpuProfileNode::kNoLineNumberInfo;
}

View File

@ -20,23 +20,21 @@ namespace internal {
struct TickSample;
// Provides a mapping from the offsets within generated code to
// the source line.
class JITLineInfoTable : public Malloced {
// Provides a mapping from the offsets within generated code or a bytecode array
// to the source line.
class SourcePositionTable : public Malloced {
public:
JITLineInfoTable();
~JITLineInfoTable();
SourcePositionTable() {}
~SourcePositionTable() {}
void SetPosition(int pc_offset, int line);
int GetSourceLineNumber(int pc_offset) const;
bool empty() const { return pc_offset_map_.empty(); }
private:
// pc_offset -> source line
typedef std::map<int, int> PcOffsetMap;
PcOffsetMap pc_offset_map_;
DISALLOW_COPY_AND_ASSIGN(JITLineInfoTable);
DISALLOW_COPY_AND_ASSIGN(SourcePositionTable);
};
@ -48,7 +46,7 @@ class CodeEntry {
const char* resource_name = CodeEntry::kEmptyResourceName,
int line_number = v8::CpuProfileNode::kNoLineNumberInfo,
int column_number = v8::CpuProfileNode::kNoColumnNumberInfo,
std::unique_ptr<JITLineInfoTable> line_info = nullptr,
std::unique_ptr<SourcePositionTable> line_info = nullptr,
Address instruction_start = nullptr);
const char* name_prefix() const { return name_prefix_; }
@ -57,7 +55,7 @@ class CodeEntry {
const char* resource_name() const { return resource_name_; }
int line_number() const { return line_number_; }
int column_number() const { return column_number_; }
const JITLineInfoTable* line_info() const { return line_info_.get(); }
const SourcePositionTable* line_info() const { return line_info_.get(); }
int script_id() const { return script_id_; }
void set_script_id(int script_id) { script_id_ = script_id; }
int position() const { return position_; }
@ -162,7 +160,7 @@ class CodeEntry {
const char* bailout_reason_;
const char* deopt_reason_;
int deopt_id_;
std::unique_ptr<JITLineInfoTable> line_info_;
std::unique_ptr<SourcePositionTable> line_info_;
Address instruction_start_;
// Should be an unordered_map, but it doesn't currently work on Win & MacOS.
std::map<int, std::vector<std::unique_ptr<CodeEntry>>> inline_locations_;

View File

@ -83,10 +83,10 @@ void ProfilerListener::CodeCreateEvent(CodeEventListener::LogEventsAndTags tag,
CodeEventsContainer evt_rec(CodeEventRecord::CODE_CREATION);
CodeCreateEventRecord* rec = &evt_rec.CodeCreateEventRecord_;
rec->start = abstract_code->address();
std::unique_ptr<JITLineInfoTable> line_table;
std::unique_ptr<SourcePositionTable> line_table;
if (shared->script()->IsScript()) {
Script* script = Script::cast(shared->script());
line_table.reset(new JITLineInfoTable());
line_table.reset(new SourcePositionTable());
int offset = abstract_code->IsCode() ? Code::kHeaderSize
: BytecodeArray::kHeaderSize;
for (SourcePositionTableIterator it(abstract_code->source_position_table());
@ -296,7 +296,7 @@ void ProfilerListener::RecordDeoptInlinedFrames(CodeEntry* entry,
CodeEntry* ProfilerListener::NewCodeEntry(
CodeEventListener::LogEventsAndTags tag, const char* name,
const char* name_prefix, const char* resource_name, int line_number,
int column_number, std::unique_ptr<JITLineInfoTable> line_info,
int column_number, std::unique_ptr<SourcePositionTable> line_info,
Address instruction_start) {
std::unique_ptr<CodeEntry> code_entry = base::make_unique<CodeEntry>(
tag, name, name_prefix, resource_name, line_number, column_number,

View File

@ -58,7 +58,7 @@ class ProfilerListener : public CodeEventListener {
const char* resource_name = CodeEntry::kEmptyResourceName,
int line_number = v8::CpuProfileNode::kNoLineNumberInfo,
int column_number = v8::CpuProfileNode::kNoColumnNumberInfo,
std::unique_ptr<JITLineInfoTable> line_info = nullptr,
std::unique_ptr<SourcePositionTable> line_info = nullptr,
Address instruction_start = nullptr);
void AddObserver(CodeEventObserver* observer);

View File

@ -27,6 +27,9 @@
//
// Tests of profiles generator and utilities.
#include <limits>
#include <memory>
#include "src/v8.h"
#include "include/v8-profiler.h"
@ -1108,9 +1111,10 @@ static void TickLines(bool optimize) {
CodeEntry* func_entry = generator->code_map()->FindEntry(code_address);
CHECK(func_entry);
CHECK_EQ(0, strcmp(func_name, func_entry->name()));
const i::JITLineInfoTable* line_info = func_entry->line_info();
const i::SourcePositionTable* line_info = func_entry->line_info();
CHECK(line_info);
CHECK(!line_info->empty());
CHECK_NE(v8::CpuProfileNode::kNoLineNumberInfo,
line_info->GetSourceLineNumber(100));
// Check the hit source lines using V8 Public APIs.
const i::ProfileTree* tree = profile->top_down();
@ -2433,6 +2437,52 @@ TEST(NativeFrameStackTrace) {
profile->Delete();
}
TEST(SourcePositionTable) {
std::unique_ptr<i::SourcePositionTable> info(new SourcePositionTable());
// Newly created tables should return NoLineNumberInfo for any lookup.
int no_info = v8::CpuProfileNode::kNoLineNumberInfo;
CHECK_EQ(no_info, info->GetSourceLineNumber(std::numeric_limits<int>::min()));
CHECK_EQ(no_info, info->GetSourceLineNumber(0));
CHECK_EQ(no_info, info->GetSourceLineNumber(1));
CHECK_EQ(no_info, info->GetSourceLineNumber(9));
CHECK_EQ(no_info, info->GetSourceLineNumber(10));
CHECK_EQ(no_info, info->GetSourceLineNumber(11));
CHECK_EQ(no_info, info->GetSourceLineNumber(19));
CHECK_EQ(no_info, info->GetSourceLineNumber(20));
CHECK_EQ(no_info, info->GetSourceLineNumber(21));
CHECK_EQ(no_info, info->GetSourceLineNumber(100));
CHECK_EQ(no_info, info->GetSourceLineNumber(std::numeric_limits<int>::max()));
info->SetPosition(10, 1);
info->SetPosition(20, 2);
// The only valid return values are 1 or 2 - every pc maps to a line number.
CHECK_EQ(1, info->GetSourceLineNumber(std::numeric_limits<int>::min()));
CHECK_EQ(1, info->GetSourceLineNumber(0));
CHECK_EQ(1, info->GetSourceLineNumber(1));
CHECK_EQ(1, info->GetSourceLineNumber(9));
CHECK_EQ(1, info->GetSourceLineNumber(10));
CHECK_EQ(1, info->GetSourceLineNumber(11));
CHECK_EQ(1, info->GetSourceLineNumber(19));
CHECK_EQ(2, info->GetSourceLineNumber(20));
CHECK_EQ(2, info->GetSourceLineNumber(21));
CHECK_EQ(2, info->GetSourceLineNumber(100));
CHECK_EQ(2, info->GetSourceLineNumber(std::numeric_limits<int>::max()));
// Test SetPosition behavior.
// Setting a position to the same value has no effect.
info->SetPosition(10, 1);
CHECK_EQ(1, info->GetSourceLineNumber(9));
CHECK_EQ(1, info->GetSourceLineNumber(10));
CHECK_EQ(1, info->GetSourceLineNumber(11));
info->SetPosition(25, 3);
CHECK_EQ(2, info->GetSourceLineNumber(21));
CHECK_EQ(3, info->GetSourceLineNumber(100));
CHECK_EQ(3, info->GetSourceLineNumber(std::numeric_limits<int>::max()));
}
} // namespace test_cpu_profiler
} // namespace internal
} // namespace v8