From 00687fb5044d9ab3361e445b7bf8949bf99994e2 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Mon, 22 Jan 2018 12:19:05 -0800 Subject: [PATCH] deps: V8: backport 76c3ac5 from upstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 https://github.com/GoogleCloudPlatform/cloud-profiler-nodejs/issues/89 Bug: v8:7203, v8:7241 Change-Id: I8ea31943741770e6611275a9c93375922b934547 Reviewed-on: https://chromium-review.googlesource.com/848093 Reviewed-by: Jaroslav Sevcik Commit-Queue: Franziska Hinkelmann Cr-Commit-Position: refs/heads/master@{#50339} Refs: https://github.com/v8/v8/commit/76c3ac58b00e08e3e2fecc3f7169bd64d91c8f75 PR-URL: https://github.com/nodejs/node/pull/18298 Reviewed-By: Michaël Zasso Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig --- common.gypi | 2 +- deps/v8/src/profiler/profiler-listener.cc | 17 +++-- deps/v8/test/cctest/test-cpu-profiler.cc | 79 +++++++++++++++++++++++ 3 files changed, 92 insertions(+), 6 deletions(-) diff --git a/common.gypi b/common.gypi index 7cceef87596891..9979b5bfedc4b0 100644 --- a/common.gypi +++ b/common.gypi @@ -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, diff --git a/deps/v8/src/profiler/profiler-listener.cc b/deps/v8/src/profiler/profiler-listener.cc index 169b12da0770c1..07a10afb5d3035 100644 --- a/deps/v8/src/profiler/profiler-listener.cc +++ b/deps/v8/src/profiler/profiler-listener.cc @@ -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); } diff --git a/deps/v8/test/cctest/test-cpu-profiler.cc b/deps/v8/test/cctest/test-cpu-profiler.cc index 0a297d9f0c9cfe..aa2ae26da4ec84 100644 --- a/deps/v8/test/cctest/test-cpu-profiler.cc +++ b/deps/v8/test/cctest/test-cpu-profiler.cc @@ -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 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 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 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(const_cast(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());