From 7cfcf8069bb91240d25f83fa5ab5b7af2da49260 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 2 May 2019 21:10:10 +0800
Subject: [PATCH] inspector: implement --cpu-prof-interval

This patch implements --cpu-prof-interval to specify the sampling
interval of the CPU profiler started by --cpu-prof from the command
line. Also adjust the interval to 100 in test-cpu-prof.js to make
the test less flaky - it would fail if the time taken to finish
the workload is smaller than the sampling interval, which was
more likely on powerful machines when the interval was 1000.

PR-URL: https://github.com/nodejs/node/pull/27535
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 doc/api/cli.md                                | 10 +++
 doc/node.1                                    |  6 ++
 src/env-inl.h                                 |  8 +++
 src/env.h                                     |  4 ++
 src/inspector_profiler.cc                     |  6 +-
 src/node_options.cc                           |  9 +++
 src/node_options.h                            |  2 +
 .../workload/fibonacci-worker-argv.js         |  6 +-
 test/sequential/test-cpu-prof.js              | 65 ++++++++++++++++++-
 9 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index e819451ee132b4..ee766a28582074 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -107,6 +107,16 @@ added: v12.0.0
 Specify the directory where the CPU profiles generated by `--cpu-prof` will
 be placed.
 
+### `--cpu-prof-interval`
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+Specify the sampling interval in microseconds for the CPU profiles generated
+by `--cpu-prof`. The default is 1000 microseconds.
+
 ### `--cpu-prof-name`
 <!-- YAML
 added: v12.0.0
diff --git a/doc/node.1 b/doc/node.1
index 0b094c62e89e29..ab715c0c1bfca2 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -90,6 +90,12 @@ The directory where the CPU profiles generated by
 .Fl -cpu-prof
 will be placed.
 .
+.It Fl -cpu-prof-interval
+The sampling interval in microseconds for the CPU profiles generated by
+.Fl -cpu-prof .
+The default is
+.Sy 1000 .
+.
 .It Fl -cpu-prof-name
 File name of the V8 CPU profile generated with
 .Fl -cpu-prof
diff --git a/src/env-inl.h b/src/env-inl.h
index 5761eb71536b08..992e51354e6cf2 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -673,6 +673,14 @@ Environment::cpu_profiler_connection() {
   return cpu_profiler_connection_.get();
 }
 
+inline void Environment::set_cpu_prof_interval(uint64_t interval) {
+  cpu_prof_interval_ = interval;
+}
+
+inline uint64_t Environment::cpu_prof_interval() const {
+  return cpu_prof_interval_;
+}
+
 inline void Environment::set_cpu_prof_name(const std::string& name) {
   cpu_prof_name_ = name;
 }
diff --git a/src/env.h b/src/env.h
index 8fed10514bbb20..a3c6ec3cc9092f 100644
--- a/src/env.h
+++ b/src/env.h
@@ -1143,6 +1143,9 @@ class Environment : public MemoryRetainer {
   inline void set_cpu_prof_name(const std::string& name);
   inline const std::string& cpu_prof_name() const;
 
+  inline void set_cpu_prof_interval(uint64_t interval);
+  inline uint64_t cpu_prof_interval() const;
+
   inline void set_cpu_prof_dir(const std::string& dir);
   inline const std::string& cpu_prof_dir() const;
 #endif  // HAVE_INSPECTOR
@@ -1183,6 +1186,7 @@ class Environment : public MemoryRetainer {
   std::string coverage_directory_;
   std::string cpu_prof_dir_;
   std::string cpu_prof_name_;
+  uint64_t cpu_prof_interval_;
 #endif  // HAVE_INSPECTOR
 
   std::shared_ptr<EnvironmentOptions> options_;
diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc
index a3739d52b21de7..2d3f061e987d35 100644
--- a/src/inspector_profiler.cc
+++ b/src/inspector_profiler.cc
@@ -21,7 +21,6 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
-using v8_inspector::StringBuffer;
 using v8_inspector::StringView;
 
 #ifdef _WIN32
@@ -254,6 +253,10 @@ MaybeLocal<Object> V8CpuProfilerConnection::GetProfile(Local<Object> result) {
 void V8CpuProfilerConnection::Start() {
   DispatchMessage("Profiler.enable");
   DispatchMessage("Profiler.start");
+  std::string params = R"({ "interval": )";
+  params += std::to_string(env()->cpu_prof_interval());
+  params += " }";
+  DispatchMessage("Profiler.setSamplingInterval", params.c_str());
 }
 
 void V8CpuProfilerConnection::End() {
@@ -304,6 +307,7 @@ void StartProfilers(Environment* env) {
   }
   if (env->options()->cpu_prof) {
     const std::string& dir = env->options()->cpu_prof_dir;
+    env->set_cpu_prof_interval(env->options()->cpu_prof_interval);
     env->set_cpu_prof_dir(dir.empty() ? GetCwd() : dir);
     if (env->options()->cpu_prof_name.empty()) {
       DiagnosticFilename filename(env, "CPU", "cpuprofile");
diff --git a/src/node_options.cc b/src/node_options.cc
index a6d7e41e8d2899..a36666c3e0f452 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -161,6 +161,11 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
     if (!cpu_prof_dir.empty()) {
       errors->push_back("--cpu-prof-dir must be used with --cpu-prof");
     }
+    // We can't catch the case where the value passed is the default value,
+    // then the option just becomes a noop which is fine.
+    if (cpu_prof_interval != kDefaultCpuProfInterval) {
+      errors->push_back("--cpu-prof-interval must be used with --cpu-prof");
+    }
   }
 
   debug_options_.CheckOptions(errors);
@@ -356,6 +361,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "specified file name of the V8 CPU profile generated with "
             "--cpu-prof",
             &EnvironmentOptions::cpu_prof_name);
+  AddOption("--cpu-prof-interval",
+            "specified sampling interval in microseconds for the V8 CPU "
+            "profile generated with --cpu-prof. (default: 1000)",
+            &EnvironmentOptions::cpu_prof_interval);
   AddOption("--cpu-prof-dir",
             "Directory where the V8 profiles generated by --cpu-prof will be "
             "placed. Does not affect --prof.",
diff --git a/src/node_options.h b/src/node_options.h
index b640b14d93639a..db564ddb3d3e6d 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -111,6 +111,8 @@ class EnvironmentOptions : public Options {
   bool prof_process = false;
 #if HAVE_INSPECTOR
   std::string cpu_prof_dir;
+  static const uint64_t kDefaultCpuProfInterval = 1000;
+  uint64_t cpu_prof_interval = kDefaultCpuProfInterval;
   std::string cpu_prof_name;
   bool cpu_prof = false;
 #endif  // HAVE_INSPECTOR
diff --git a/test/fixtures/workload/fibonacci-worker-argv.js b/test/fixtures/workload/fibonacci-worker-argv.js
index 9c968bfeec4ae2..69a78ec4e175ef 100644
--- a/test/fixtures/workload/fibonacci-worker-argv.js
+++ b/test/fixtures/workload/fibonacci-worker-argv.js
@@ -3,5 +3,9 @@
 const { Worker } = require('worker_threads');
 const path = require('path');
 new Worker(path.join(__dirname, 'fibonacci.js'), {
-  execArgv: ['--cpu-prof']
+  execArgv: [
+    '--cpu-prof',
+    '--cpu-prof-interval',
+    process.env.CPU_PROF_INTERVAL || '100'
+  ]
 });
diff --git a/test/sequential/test-cpu-prof.js b/test/sequential/test-cpu-prof.js
index 19c8afd92cd5b9..38be6211e4d23f 100644
--- a/test/sequential/test-cpu-prof.js
+++ b/test/sequential/test-cpu-prof.js
@@ -50,18 +50,43 @@ if (common.isWindows) {
   FIB = 40;
 }
 
+// We need to set --cpu-interval to a smaller value to make sure we can
+// find our workload in the samples. 50us should be a small enough sampling
+// interval for this.
+const kCpuProfInterval = 50;
 const env = {
   ...process.env,
   FIB,
   NODE_DEBUG_NATIVE: 'INSPECTOR_PROFILER'
 };
 
+// Test --cpu-prof without --cpu-prof-interval. Here we just verify that
+// we manage to generate a profile.
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--cpu-prof',
+    fixtures.path('workload', 'fibonacci.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 0);
+  const profiles = getCpuProfiles(tmpdir.path);
+  assert.strictEqual(profiles.length, 1);
+}
+
 // Outputs CPU profile when event loop is drained.
 // TODO(joyeecheung): share the fixutres with v8 coverage tests
 {
   tmpdir.refresh();
   const output = spawnSync(process.execPath, [
     '--cpu-prof',
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     fixtures.path('workload', 'fibonacci.js'),
   ], {
     cwd: tmpdir.path,
@@ -81,6 +106,8 @@ const env = {
   tmpdir.refresh();
   const output = spawnSync(process.execPath, [
     '--cpu-prof',
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     fixtures.path('workload', 'fibonacci-exit.js'),
   ], {
     cwd: tmpdir.path,
@@ -100,6 +127,8 @@ const env = {
   tmpdir.refresh();
   const output = spawnSync(process.execPath, [
     '--cpu-prof',
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     fixtures.path('workload', 'fibonacci-sigint.js'),
   ], {
     cwd: tmpdir.path,
@@ -123,7 +152,10 @@ const env = {
     fixtures.path('workload', 'fibonacci-worker-argv.js'),
   ], {
     cwd: tmpdir.path,
-    env
+    env: {
+      ...process.env,
+      CPU_PROF_INTERVAL: kCpuProfInterval
+    }
   });
   if (output.status !== 0) {
     console.log(output.stderr.toString());
@@ -176,12 +208,35 @@ const env = {
     `${process.execPath}: --cpu-prof-dir must be used with --cpu-prof`);
 }
 
+// --cpu-prof-interval without --cpu-prof
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--cpu-prof-interval',
+    kCpuProfInterval,
+    fixtures.path('workload', 'fibonacci.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  const stderr = output.stderr.toString().trim();
+  if (output.status !== 9) {
+    console.log(stderr);
+  }
+  assert.strictEqual(output.status, 9);
+  assert.strictEqual(
+    stderr,
+    `${process.execPath}: --cpu-prof-interval must be used with --cpu-prof`);
+}
+
 // --cpu-prof-name
 {
   tmpdir.refresh();
   const file = path.join(tmpdir.path, 'test.cpuprofile');
   const output = spawnSync(process.execPath, [
     '--cpu-prof',
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     '--cpu-prof-name',
     'test.cpuprofile',
     fixtures.path('workload', 'fibonacci.js'),
@@ -203,6 +258,8 @@ const env = {
   tmpdir.refresh();
   const output = spawnSync(process.execPath, [
     '--cpu-prof',
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     '--cpu-prof-dir',
     'prof',
     fixtures.path('workload', 'fibonacci.js'),
@@ -227,6 +284,8 @@ const env = {
   const dir = path.join(tmpdir.path, 'prof');
   const output = spawnSync(process.execPath, [
     '--cpu-prof',
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     '--cpu-prof-dir',
     dir,
     fixtures.path('workload', 'fibonacci.js'),
@@ -251,6 +310,8 @@ const env = {
   const file = path.join(dir, 'test.cpuprofile');
   const output = spawnSync(process.execPath, [
     '--cpu-prof',
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     '--cpu-prof-name',
     'test.cpuprofile',
     '--cpu-prof-dir',
@@ -274,6 +335,8 @@ const env = {
 {
   tmpdir.refresh();
   const output = spawnSync(process.execPath, [
+    '--cpu-prof-interval',
+    kCpuProfInterval,
     '--cpu-prof-dir',
     'prof',
     '--cpu-prof',