Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: V8: backport 76c3ac5 from upstream #18298

Merged
merged 1 commit into from
Jan 24, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
deps: V8: backport 76c3ac5 from upstream
This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

Original commit message:
  [cpu-profiler] Fix script name when recording inlining info

  Use the script name from the shared function info to create an
  inline entry. Otherwise functions are attributed to the wrong file
  in the CpuProfileNode.

  See googleapis/cloud-profiler-nodejs#89

  Bug: v8:7203, v8:7241
  Change-Id: I8ea31943741770e6611275a9c93375922b934547
  Reviewed-on: https://chromium-review.googlesource.com/848093
  Reviewed-by: Jaroslav Sevcik <[email protected]>
  Commit-Queue: Franziska Hinkelmann <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#50339}

Refs: v8/v8@76c3ac5
PR-URL: #18298
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
ofrobots committed Jan 24, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 00687fb5044d9ab3361e445b7bf8949bf99994e2
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.8',
'v8_embedder_string': '-node.9',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
17 changes: 12 additions & 5 deletions deps/v8/src/profiler/profiler-listener.cc
Original file line number Diff line number Diff line change
@@ -230,11 +230,18 @@ void ProfilerListener::RecordInliningInfo(CodeEntry* entry,
SharedFunctionInfo* shared_info = SharedFunctionInfo::cast(
deopt_input_data->LiteralArray()->get(shared_info_id));
if (!depth++) continue; // Skip the current function itself.
CodeEntry* inline_entry = new CodeEntry(
entry->tag(), GetFunctionName(shared_info->DebugName()),
CodeEntry::kEmptyNamePrefix, entry->resource_name(),
CpuProfileNode::kNoLineNumberInfo,
CpuProfileNode::kNoColumnNumberInfo, NULL, code->instruction_start());
const char* resource_name =
(shared_info->script()->IsScript() &&
Script::cast(shared_info->script())->name()->IsName())
? GetName(Name::cast(Script::cast(shared_info->script())->name()))
: CodeEntry::kEmptyResourceName;

CodeEntry* inline_entry =
new CodeEntry(entry->tag(), GetFunctionName(shared_info->DebugName()),
CodeEntry::kEmptyNamePrefix, resource_name,
CpuProfileNode::kNoLineNumberInfo,
CpuProfileNode::kNoColumnNumberInfo, nullptr,
code->instruction_start());
inline_entry->FillFunctionInfo(shared_info);
inline_stack.push_back(inline_entry);
}
79 changes: 79 additions & 0 deletions deps/v8/test/cctest/test-cpu-profiler.cc
Original file line number Diff line number Diff line change
@@ -1738,6 +1738,85 @@ TEST(FunctionDetails) {
script_a->GetUnboundScript()->GetId(), 5, 14);
}

TEST(FunctionDetailsInlining) {
if (!CcTest::i_isolate()->use_optimizer() || i::FLAG_always_opt) return;
i::FLAG_allow_natives_syntax = true;
v8::HandleScope scope(CcTest::isolate());
v8::Local<v8::Context> env = CcTest::NewContext(PROFILER_EXTENSION);
v8::Context::Scope context_scope(env);
ProfilerHelper helper(env);

// alpha is in a_script, beta in b_script. beta is
// inlined in alpha, but it should be attributed to b_script.

v8::Local<v8::Script> script_b = CompileWithOrigin(
"function beta(k) {\n"
" let sum = 2;\n"
" for(let i = 0; i < k; i ++) {\n"
" sum += i;\n"
" sum = sum + 'a';\n"
" }\n"
" return sum;\n"
"}\n"
"\n",
"script_b");

v8::Local<v8::Script> script_a = CompileWithOrigin(
"function alpha(p) {\n"
" let res = beta(p);\n"
" res = res + res;\n"
" return res;\n"
"}\n"
"let p = 2;\n"
"\n"
"\n"
"// Warm up before profiling or the inlining doesn't happen.\n"
"p = alpha(p);\n"
"p = alpha(p);\n"
"%OptimizeFunctionOnNextCall(alpha);\n"
"p = alpha(p);\n"
"\n"
"\n"
"startProfiling();\n"
"for(let i = 0; i < 10000; i++) {\n"
" p = alpha(p);\n"
"}\n"
"stopProfiling();\n"
"\n"
"\n",
"script_a");

script_b->Run(env).ToLocalChecked();
script_a->Run(env).ToLocalChecked();

const v8::CpuProfile* profile = i::ProfilerExtension::last_profile;
const v8::CpuProfileNode* current = profile->GetTopDownRoot();
reinterpret_cast<ProfileNode*>(const_cast<v8::CpuProfileNode*>(current))
->Print(0);
// The tree should look like this:
// 0 (root) 0 #1
// 5 (program) 0 #6
// 2 14 #2 script_a:1
// ;;; deopted at script_id: 14 position: 299 with reason 'Insufficient
// type feedback for call'.
// 1 alpha 14 #4 script_a:1
// 9 beta 13 #5 script_b:0
// 0 startProfiling 0 #3

const v8::CpuProfileNode* root = profile->GetTopDownRoot();
const v8::CpuProfileNode* script = GetChild(env, root, "");
CheckFunctionDetails(env->GetIsolate(), script, "", "script_a",
script_a->GetUnboundScript()->GetId(), 1, 1);
const v8::CpuProfileNode* alpha = FindChild(env, script, "alpha");
// Return early if profiling didn't sample alpha.
if (!alpha) return;
CheckFunctionDetails(env->GetIsolate(), alpha, "alpha", "script_a",
script_a->GetUnboundScript()->GetId(), 1, 15);
const v8::CpuProfileNode* beta = FindChild(env, alpha, "beta");
if (!beta) return;
CheckFunctionDetails(env->GetIsolate(), beta, "beta", "script_b",
script_b->GetUnboundScript()->GetId(), 0, 0);
}

TEST(DontStopOnFinishedProfileDelete) {
v8::HandleScope scope(CcTest::isolate());