Skip to content

Commit

Permalink
deps: V8: backport 76c3ac5 from upstream
Browse files Browse the repository at this point in the history
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 <jarin@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#50339}

Refs: v8/v8@76c3ac5
PR-URL: #18298
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
ofrobots authored and MylesBorins committed Feb 21, 2018
1 parent b02f4e1 commit 2d98b58
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 6 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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.19',
'v8_embedder_string': '-node.20',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
17 changes: 12 additions & 5 deletions deps/v8/src/profiler/profiler-listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,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);
}
Expand Down
79 changes: 79 additions & 0 deletions deps/v8/test/cctest/test-cpu-profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1745,6 +1745,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());
Expand Down

0 comments on commit 2d98b58

Please sign in to comment.