Skip to content

Commit

Permalink
[cpu-profiler] Remove instruction_start field from CodeEntry
Browse files Browse the repository at this point in the history 
This data is duplicated across the code map, and not actually required
for some esoteric types of CodeEntry objects (e.g. inline stacks). Unify
sourcing of this data from the code map instead.

Change-Id: I75fddc03221d1d6b7dab77d16fa05ad6eb3dd2a9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2026416
Reviewed-by: Peter Marshall <[email protected]>
Commit-Queue: Andrew Comminos <[email protected]>
Cr-Commit-Position: refs/heads/master@{#66033}
  • Loading branch information
@acomminos
acomminos authored and Commit Bot committed Jan 29, 2020
1 parent e883264 commit 8580537
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 44 deletions.
10 changes: 5 additions & 5 deletions src/profiler/profile-generator-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ CodeEntry::CodeEntry(CodeEventListener::LogEventsAndTags tag, const char* name,
const char* resource_name, int line_number,
int column_number,
std::unique_ptr<SourcePositionTable> line_info,
Address instruction_start, bool is_shared_cross_origin)
bool is_shared_cross_origin)
: bit_field_(TagField::encode(tag) |
BuiltinIdField::encode(Builtins::builtin_count) |
SharedCrossOriginField::encode(is_shared_cross_origin)),
Expand All @@ -26,11 +26,11 @@ CodeEntry::CodeEntry(CodeEventListener::LogEventsAndTags tag, const char* name,
column_number_(column_number),
script_id_(v8::UnboundScript::kNoScriptId),
position_(0),
line_info_(std::move(line_info)),
instruction_start_(instruction_start) {}
line_info_(std::move(line_info)) {}

inline CodeEntry* ProfileGenerator::FindEntry(Address address) {
CodeEntry* entry = code_map_->FindEntry(address);
inline CodeEntry* ProfileGenerator::FindEntry(Address address,
Address* out_instruction_start) {
CodeEntry* entry = code_map_->FindEntry(address, out_instruction_start);
if (entry) entry->mark_used();
return entry;
}
Expand Down
29 changes: 11 additions & 18 deletions src/profiler/profile-generator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,6 @@ void CodeEntry::print() const {
base::OS::Print(" - column_number: %d\n", column_number_);
base::OS::Print(" - script_id: %d\n", script_id_);
base::OS::Print(" - position: %d\n", position_);
base::OS::Print(" - instruction_start: %p\n",
reinterpret_cast<void*>(instruction_start_));

if (line_info_) {
line_info_->print();
Expand Down Expand Up @@ -664,8 +662,6 @@ void CodeMap::AddCode(Address addr, CodeEntry* entry, unsigned size) {
ClearCodesInRange(addr, addr + size);
unsigned index = AddCodeEntry(addr, entry);
code_map_.emplace(addr, CodeEntryMapInfo{index, size});
DCHECK(entry->instruction_start() == kNullAddress ||
addr == entry->instruction_start());
}

void CodeMap::ClearCodesInRange(Address start, Address end) {
Expand All @@ -683,17 +679,15 @@ void CodeMap::ClearCodesInRange(Address start, Address end) {
code_map_.erase(left, right);
}

CodeEntry* CodeMap::FindEntry(Address addr) {
CodeEntry* CodeMap::FindEntry(Address addr, Address* out_instruction_start) {
auto it = code_map_.upper_bound(addr);
if (it == code_map_.begin()) return nullptr;
--it;
Address start_address = it->first;
Address end_address = start_address + it->second.size;
CodeEntry* ret = addr < end_address ? entry(it->second.index) : nullptr;
if (ret && ret->instruction_start() != kNullAddress) {
DCHECK_EQ(start_address, ret->instruction_start());
DCHECK(addr >= start_address && addr < end_address);
}
DCHECK(!ret || (addr >= start_address && addr < end_address));
if (ret && out_instruction_start) *out_instruction_start = start_address;
return ret;
}

Expand All @@ -706,9 +700,6 @@ void CodeMap::MoveCode(Address from, Address to) {
DCHECK(from + info.size <= to || to + info.size <= from);
ClearCodesInRange(to, to + info.size);
code_map_.emplace(to, info);

CodeEntry* entry = code_entries_[info.index].entry;
entry->set_instruction_start(to);
}

unsigned CodeMap::AddCodeEntry(Address start, CodeEntry* entry) {
Expand Down Expand Up @@ -886,21 +877,23 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
true});
} else {
Address attributed_pc = reinterpret_cast<Address>(sample.pc);
CodeEntry* pc_entry = FindEntry(attributed_pc);
Address pc_entry_instruction_start = kNullAddress;
CodeEntry* pc_entry =
FindEntry(attributed_pc, &pc_entry_instruction_start);
// If there is no pc_entry, we're likely in native code. Find out if the
// top of the stack (the return address) was pointing inside a JS
// function, meaning that we have encountered a frameless invocation.
if (!pc_entry && !sample.has_external_callback) {
attributed_pc = reinterpret_cast<Address>(sample.tos);
pc_entry = FindEntry(attributed_pc);
pc_entry = FindEntry(attributed_pc, &pc_entry_instruction_start);
}
// If pc is in the function code before it set up stack frame or after the
// frame was destroyed, SafeStackFrameIterator incorrectly thinks that
// ebp contains the return address of the current function and skips the
// caller's frame. Check for this case and just skip such samples.
if (pc_entry) {
int pc_offset =
static_cast<int>(attributed_pc - pc_entry->instruction_start());
static_cast<int>(attributed_pc - pc_entry_instruction_start);
// TODO(petermarshall): pc_offset can still be negative in some cases.
src_line = pc_entry->GetSourceLine(pc_offset);
if (src_line == v8::CpuProfileNode::kNoLineNumberInfo) {
Expand Down Expand Up @@ -932,12 +925,12 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
for (unsigned i = 0; i < sample.frames_count; ++i) {
Address stack_pos = reinterpret_cast<Address>(sample.stack[i]);
Address native_context = reinterpret_cast<Address>(sample.contexts[i]);
CodeEntry* entry = FindEntry(stack_pos);
Address instruction_start = kNullAddress;
CodeEntry* entry = FindEntry(stack_pos, &instruction_start);
int line_number = no_line_info;
if (entry) {
// Find out if the entry has an inlining stack associated.
int pc_offset =
static_cast<int>(stack_pos - entry->instruction_start());
int pc_offset = static_cast<int>(stack_pos - instruction_start);
// TODO(petermarshall): pc_offset can still be negative in some cases.
const std::vector<CodeEntryAndLineNumber>* inline_stack =
entry->GetInlineStack(pc_offset);
Expand Down
10 changes: 3 additions & 7 deletions src/profiler/profile-generator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class CodeEntry {
int line_number = v8::CpuProfileNode::kNoLineNumberInfo,
int column_number = v8::CpuProfileNode::kNoColumnNumberInfo,
std::unique_ptr<SourcePositionTable> line_info = nullptr,
Address instruction_start = kNullAddress,
bool is_shared_cross_origin = false);

const char* name() const { return name_; }
Expand Down Expand Up @@ -136,9 +135,6 @@ class CodeEntry {
const std::vector<CodeEntryAndLineNumber>* GetInlineStack(
int pc_offset) const;

void set_instruction_start(Address start) { instruction_start_ = start; }
Address instruction_start() const { return instruction_start_; }

CodeEventListener::LogEventsAndTags tag() const {
return TagField::decode(bit_field_);
}
Expand Down Expand Up @@ -223,7 +219,6 @@ class CodeEntry {
int script_id_;
int position_;
std::unique_ptr<SourcePositionTable> line_info_;
Address instruction_start_;
std::unique_ptr<RareData> rare_data_;

DISALLOW_COPY_AND_ASSIGN(CodeEntry);
Expand Down Expand Up @@ -449,7 +444,7 @@ class V8_EXPORT_PRIVATE CodeMap {

void AddCode(Address addr, CodeEntry* entry, unsigned size);
void MoveCode(Address from, Address to);
CodeEntry* FindEntry(Address addr);
CodeEntry* FindEntry(Address addr, Address* out_instruction_start = nullptr);
void Print();

private:
Expand Down Expand Up @@ -533,7 +528,8 @@ class V8_EXPORT_PRIVATE ProfileGenerator {
CodeMap* code_map() { return code_map_; }

private:
CodeEntry* FindEntry(Address address);
CodeEntry* FindEntry(Address address,
Address* out_instruction_start = nullptr);
CodeEntry* EntryForVMState(StateTag tag);

CpuProfilesCollection* profiles_;
Expand Down
24 changes: 10 additions & 14 deletions src/profiler/profiler-listener.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ void ProfilerListener::CodeCreateEvent(LogEventsAndTags tag,
rec->instruction_start = code->InstructionStart();
rec->entry = new CodeEntry(tag, GetName(name), CodeEntry::kEmptyResourceName,
CpuProfileNode::kNoLineNumberInfo,
CpuProfileNode::kNoColumnNumberInfo, nullptr,
code->InstructionStart());
CpuProfileNode::kNoColumnNumberInfo, nullptr);
rec->instruction_size = code->InstructionSize();
DispatchCodeEvent(evt_rec);
}
Expand All @@ -52,8 +51,7 @@ void ProfilerListener::CodeCreateEvent(LogEventsAndTags tag,
rec->instruction_start = code->InstructionStart();
rec->entry = new CodeEntry(tag, GetName(*name), CodeEntry::kEmptyResourceName,
CpuProfileNode::kNoLineNumberInfo,
CpuProfileNode::kNoColumnNumberInfo, nullptr,
code->InstructionStart());
CpuProfileNode::kNoColumnNumberInfo, nullptr);
rec->instruction_size = code->InstructionSize();
DispatchCodeEvent(evt_rec);
}
Expand All @@ -68,8 +66,7 @@ void ProfilerListener::CodeCreateEvent(LogEventsAndTags tag,
rec->entry = new CodeEntry(tag, GetName(shared->DebugName()),
GetName(InferScriptName(*script_name, *shared)),
CpuProfileNode::kNoLineNumberInfo,
CpuProfileNode::kNoColumnNumberInfo, nullptr,
code->InstructionStart());
CpuProfileNode::kNoColumnNumberInfo, nullptr);
DCHECK(!code->IsCode());
rec->entry->FillFunctionInfo(*shared);
rec->instruction_size = code->InstructionSize();
Expand Down Expand Up @@ -164,7 +161,7 @@ void ProfilerListener::CodeCreateEvent(LogEventsAndTags tag,
std::unique_ptr<CodeEntry> inline_entry = std::make_unique<CodeEntry>(
tag, GetFunctionName(*pos_info.shared), resource_name,
start_pos_info.line + 1, start_pos_info.column + 1, nullptr,
code->InstructionStart(), inline_is_shared_cross_origin);
inline_is_shared_cross_origin);
inline_entry->FillFunctionInfo(*pos_info.shared);

// Create a canonical CodeEntry for each inlined frame and then re-use
Expand All @@ -182,8 +179,7 @@ void ProfilerListener::CodeCreateEvent(LogEventsAndTags tag,
rec->entry =
new CodeEntry(tag, GetFunctionName(*shared),
GetName(InferScriptName(*script_name, *shared)), line,
column, std::move(line_table),
abstract_code->InstructionStart(), is_shared_cross_origin);
column, std::move(line_table), is_shared_cross_origin);
if (!inline_stacks.empty()) {
rec->entry->SetInlineStacks(std::move(cached_inline_entries),
std::move(inline_stacks));
Expand All @@ -200,10 +196,10 @@ void ProfilerListener::CodeCreateEvent(LogEventsAndTags tag,
CodeEventsContainer evt_rec(CodeEventRecord::CODE_CREATION);
CodeCreateEventRecord* rec = &evt_rec.CodeCreateEventRecord_;
rec->instruction_start = code->instruction_start();
rec->entry = new CodeEntry(
tag, GetName(name), CodeEntry::kWasmResourceNamePrefix,
CpuProfileNode::kNoLineNumberInfo, CpuProfileNode::kNoColumnNumberInfo,
nullptr, code->instruction_start(), true);
rec->entry =
new CodeEntry(tag, GetName(name), CodeEntry::kWasmResourceNamePrefix,
CpuProfileNode::kNoLineNumberInfo,
CpuProfileNode::kNoColumnNumberInfo, nullptr, true);
rec->instruction_size = code->instructions().length();
DispatchCodeEvent(evt_rec);
}
Expand Down Expand Up @@ -247,7 +243,7 @@ void ProfilerListener::RegExpCodeCreateEvent(Handle<AbstractCode> code,
rec->entry = new CodeEntry(
CodeEventListener::REG_EXP_TAG, GetConsName("RegExp: ", *source),
CodeEntry::kEmptyResourceName, CpuProfileNode::kNoLineNumberInfo,
CpuProfileNode::kNoColumnNumberInfo, nullptr, code->InstructionStart());
CpuProfileNode::kNoColumnNumberInfo, nullptr);
rec->instruction_size = code->InstructionSize();
DispatchCodeEvent(evt_rec);
}
Expand Down

0 comments on commit 8580537

Please sign in to comment.