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: nodejs#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>
Trott · May 5, 2019 · 7cfcf80 · 7cfcf80
1 parent 0171bab
commit 7cfcf80
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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',