inspector: split --cpu-prof-path to --cpu-prof-dir and --cpu-prof-name

To improve the integration of `--cpu-prof` with workers, this patch
splits `--cpu-prof-path` into `--cpu-prof-dir` and `--cpu-prof-name`,
so when a worker is launched from a thread that enables
`--cpu-prof`, if the parent thread sets `--cpu-prof-dir`, then the
profile of both thread would be generated to the specified directory.
If they end up specifying the same `--cpu-prof-name` the behavior
is undefined the last profile will overwritten the first one.

PR-URL: https://github.com/nodejs/node/pull/27306
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

7 files changed, 68 insertions, 19 deletions

diff --git a/src/env-inl.h b/src/env-inl.h
index 3a6caa19f8..ee0503620c 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -38,6 +38,14 @@
 
 #include <utility>
 
+#ifdef _WIN32
+/* MAX_PATH is in characters, not bytes. Make sure we have enough headroom. */
+#define CWD_BUFSIZE (MAX_PATH * 4)
+#else
+#include <climits>  // PATH_MAX on Solaris.
+#define CWD_BUFSIZE (PATH_MAX)
+#endif
+
 namespace node {
 
 inline v8::Isolate* IsolateData::isolate() const {
@@ -678,6 +686,15 @@ inline void Environment::set_cpu_profile_path(const std::string& path) {
 inline const std::string& Environment::cpu_profile_path() const {
   return cpu_profile_path_;
 }
+
+inline void Environment::set_cpu_prof_dir(const std::string& path) {
+  cpu_prof_dir_ = path;
+}
+
+inline const std::string& Environment::cpu_prof_dir() const {
+  return cpu_prof_dir_;
+}
+
 #endif  // HAVE_INSPECTOR
 
 inline std::shared_ptr<HostPort> Environment::inspector_host_port() {
diff --git a/src/env.cc b/src/env.cc
index ccb3c404f9..cbb8ab6fe2 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -845,6 +845,24 @@ void Environment::stop_sub_worker_contexts() {
   }
 }
 
+#if HAVE_INSPECTOR
+
+void Environment::InitializeCPUProfDir(const std::string& dir) {
+  if (!dir.empty()) {
+    cpu_prof_dir_ = dir;
+    return;
+  }
+  char cwd[CWD_BUFSIZE];
+  size_t size = CWD_BUFSIZE;
+  int err = uv_cwd(cwd, &size);
+  // TODO(joyeecheung): fallback to exec path / argv[0]
+  CHECK_EQ(err, 0);
+  CHECK_GT(size, 0);
+  cpu_prof_dir_ = cwd;
+}
+
+#endif  // HAVE_INSPECTOR
+
 void MemoryTracker::TrackField(const char* edge_name,
                                const CleanupHookCallback& value,
                                const char* node_name) {
diff --git a/src/env.h b/src/env.h
index cfef54dcbd..def7cba388 100644
--- a/src/env.h
+++ b/src/env.h
@@ -1137,6 +1137,11 @@ class Environment : public MemoryRetainer {
 
   inline void set_cpu_profile_path(const std::string& path);
   inline const std::string& cpu_profile_path() const;
+
+  inline void set_cpu_prof_dir(const std::string& path);
+  inline const std::string& cpu_prof_dir() const;
+
+  void InitializeCPUProfDir(const std::string& dir);
 #endif  // HAVE_INSPECTOR
 
  private:
@@ -1173,6 +1178,7 @@ class Environment : public MemoryRetainer {
   std::unique_ptr<profiler::V8CoverageConnection> coverage_connection_;
   std::unique_ptr<profiler::V8CpuProfilerConnection> cpu_profiler_connection_;
   std::string coverage_directory_;
+  std::string cpu_prof_dir_;
   std::string cpu_profile_path_;
 #endif  // HAVE_INSPECTOR
 
diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc
index 46d4c4fec8..3507ae4ef2 100644
--- a/src/inspector_profiler.cc
+++ b/src/inspector_profiler.cc
@@ -318,19 +318,13 @@ void StartCoverageCollection(Environment* env) {
   env->coverage_connection()->Start();
 }
 
-void StartCpuProfiling(Environment* env, const std::string& profile_path) {
-  std::string path;
-  if (profile_path.empty()) {
-    char cwd[CWD_BUFSIZE];
-    size_t size = CWD_BUFSIZE;
-    int err = uv_cwd(cwd, &size);
-    // TODO(joyeecheung): fallback to exec path / argv[0]
-    CHECK_EQ(err, 0);
-    CHECK_GT(size, 0);
+void StartCpuProfiling(Environment* env, const std::string& profile_name) {
+  std::string path = env->cpu_prof_dir() + std::string(kPathSeparator);
+  if (profile_name.empty()) {
     DiagnosticFilename filename(env, "CPU", "cpuprofile");
-    path = cwd + std::string(kPathSeparator) + (*filename);
+    path += *filename;
   } else {
-    path = profile_path;
+    path += profile_name;
   }
   env->set_cpu_profile_path(std::move(path));
   env->set_cpu_profiler_connection(
diff --git a/src/node.cc b/src/node.cc
index 8efc20475d..061d6bb424 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -239,7 +239,8 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
 
 #if HAVE_INSPECTOR
   if (env->options()->cpu_prof) {
-    profiler::StartCpuProfiling(env, env->options()->cpu_prof_path);
+    env->InitializeCPUProfDir(env->options()->cpu_prof_dir);
+    profiler::StartCpuProfiling(env, env->options()->cpu_prof_name);
   }
 #endif  // HAVE_INSPECTOR
 
diff --git a/src/node_options.cc b/src/node_options.cc
index 8914acfd60..adc6f61586 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -149,6 +149,15 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
   }
 
 #if HAVE_INSPECTOR
+  if (!cpu_prof) {
+    if (!cpu_prof_name.empty()) {
+      errors->push_back("--cpu-prof-name must be used with --cpu-prof");
+    }
+    if (!cpu_prof_dir.empty()) {
+      errors->push_back("--cpu-prof-dir must be used with --cpu-prof");
+    }
+  }
+
   debug_options_.CheckOptions(errors);
 #endif  // HAVE_INSPECTOR
 }
@@ -335,14 +344,17 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
 #if HAVE_INSPECTOR
   AddOption("--cpu-prof",
             "Start the V8 CPU profiler on start up, and write the CPU profile "
-            "to disk before exit. If --cpu-prof-path is not specified, write "
+            "to disk before exit. If --cpu-prof-dir is not specified, write "
             "the profile to the current working directory.",
             &EnvironmentOptions::cpu_prof);
-  AddOption("--cpu-prof-path",
-            "Path the V8 CPU profile generated with --cpu-prof will be "
-            "written to.",
-            &EnvironmentOptions::cpu_prof_path);
-  Implies("--cpu-prof-path", "--cpu-prof");
+  AddOption("--cpu-prof-name",
+            "specified file name of the V8 CPU profile generated with "
+            "--cpu-prof",
+            &EnvironmentOptions::cpu_prof_name);
+  AddOption("--cpu-prof-dir",
+            "Directory where the V8 profiles generated by --cpu-prof will be "
+            "placed. Does not affect --prof.",
+            &EnvironmentOptions::cpu_prof_dir);
 #endif  // HAVE_INSPECTOR
   AddOption("--redirect-warnings",
             "write warnings to file instead of stderr",
diff --git a/src/node_options.h b/src/node_options.h
index d0d35b162e..f922615690 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -110,7 +110,8 @@ class EnvironmentOptions : public Options {
   bool preserve_symlinks_main = false;
   bool prof_process = false;
 #if HAVE_INSPECTOR
-  std::string cpu_prof_path;
+  std::string cpu_prof_dir;
+  std::string cpu_prof_name;
   bool cpu_prof = false;
 #endif  // HAVE_INSPECTOR
   std::string redirect_warnings;