src: refactor profile initialization

- Process and store --cpu-prof-dir and --cpu-prof-name during Environment creation - Start profilers in one `profiler::StartProfilers()` PR-URL: #27475 Refs: #27421 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
nodejs · May 2, 2019 · 30cceae · 30cceae
1 parent 6f02f15
commit 30cceae
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 74 deletions.
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -673,16 +673,16 @@ Environment::cpu_profiler_connection() {
   return cpu_profiler_connection_.get();
 }
 
-inline void Environment::set_cpu_profile_path(const std::string& path) {
-  cpu_profile_path_ = path;
+inline void Environment::set_cpu_prof_name(const std::string& name) {
+  cpu_prof_name_ = name;
 }
 
-inline const std::string& Environment::cpu_profile_path() const {
-  return cpu_profile_path_;
+inline const std::string& Environment::cpu_prof_name() const {
+  return cpu_prof_name_;
 }
 
-inline void Environment::set_cpu_prof_dir(const std::string& path) {
-  cpu_prof_dir_ = path;
+inline void Environment::set_cpu_prof_dir(const std::string& dir) {
+  cpu_prof_dir_ = dir;
 }
 
 inline const std::string& Environment::cpu_prof_dir() const {

diff --git a/src/env.cc b/src/env.cc
@@ -907,20 +907,6 @@ 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,

diff --git a/src/env.h b/src/env.h
@@ -1140,13 +1140,11 @@ class Environment : public MemoryRetainer {
       std::unique_ptr<profiler::V8CpuProfilerConnection> connection);
   profiler::V8CpuProfilerConnection* cpu_profiler_connection();
 
-  inline void set_cpu_profile_path(const std::string& path);
-  inline const std::string& cpu_profile_path() const;
+  inline void set_cpu_prof_name(const std::string& name);
+  inline const std::string& cpu_prof_name() const;
 
-  inline void set_cpu_prof_dir(const std::string& path);
+  inline void set_cpu_prof_dir(const std::string& dir);
   inline const std::string& cpu_prof_dir() const;
-
-  void InitializeCPUProfDir(const std::string& dir);
 #endif  // HAVE_INSPECTOR
 
  private:
@@ -1184,7 +1182,7 @@ class Environment : public MemoryRetainer {
   std::unique_ptr<profiler::V8CpuProfilerConnection> cpu_profiler_connection_;
   std::string coverage_directory_;
   std::string cpu_prof_dir_;
-  std::string cpu_profile_path_;
+  std::string cpu_prof_name_;
 #endif  // HAVE_INSPECTOR
 
   std::shared_ptr<EnvironmentOptions> options_;

diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc
@@ -209,26 +209,26 @@ void V8CpuProfilerConnection::OnMessage(
 }
 
 void V8CpuProfilerConnection::WriteCpuProfile(Local<String> message) {
-  const std::string& path = env()->cpu_profile_path();
-  CHECK(!path.empty());
-  std::string directory = path.substr(0, path.find_last_of(kPathSeparator));
-  if (directory != path) {
-    uv_fs_t req;
-    int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr);
-    uv_fs_req_cleanup(&req);
-    if (ret < 0 && ret != UV_EEXIST) {
-      char err_buf[128];
-      uv_err_name_r(ret, err_buf, sizeof(err_buf));
-      fprintf(stderr,
-              "%s: Failed to create cpu profile directory %s\n",
-              err_buf,
-              directory.c_str());
-      return;
-    }
+  const std::string& filename = env()->cpu_prof_name();
+  const std::string& directory = env()->cpu_prof_dir();
+  CHECK(!filename.empty());
+  CHECK(!directory.empty());
+  uv_fs_t req;
+  int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr);
+  uv_fs_req_cleanup(&req);
+  if (ret < 0 && ret != UV_EEXIST) {
+    char err_buf[128];
+    uv_err_name_r(ret, err_buf, sizeof(err_buf));
+    fprintf(stderr,
+            "%s: Failed to create cpu profile directory %s\n",
+            err_buf,
+            directory.c_str());
+    return;
   }
   MaybeLocal<String> result = GetResult(message);
+  std::string target = directory + kPathSeparator + filename;
   if (!result.IsEmpty()) {
-    WriteResult(path.c_str(), result.ToLocalChecked());
+    WriteResult(target.c_str(), result.ToLocalChecked());
   }
 }
 
@@ -312,24 +312,42 @@ void EndStartedProfilers(Environment* env) {
   }
 }
 
-void StartCoverageCollection(Environment* env) {
-  CHECK_NULL(env->coverage_connection());
-  env->set_coverage_connection(std::make_unique<V8CoverageConnection>(env));
-  env->coverage_connection()->Start();
+std::string GetCwd() {
+  char cwd[CWD_BUFSIZE];
+  size_t size = CWD_BUFSIZE;
+  int err = uv_cwd(cwd, &size);
+  // This can fail if the cwd is deleted.
+  // TODO(joyeecheung): store this in the Environment during Environment
+  // creation and fallback to exec_path and argv0, then we no longer need
+  // SetCoverageDirectory().
+  CHECK_EQ(err, 0);
+  CHECK_GT(size, 0);
+  return cwd;
 }
 
-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 += *filename;
-  } else {
-    path += profile_name;
+void StartProfilers(Environment* env) {
+  Isolate* isolate = env->isolate();
+  Local<String> coverage_str = env->env_vars()->Get(
+      isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE"));
+  if (!coverage_str.IsEmpty() && coverage_str->Length() > 0) {
+    CHECK_NULL(env->coverage_connection());
+    env->set_coverage_connection(std::make_unique<V8CoverageConnection>(env));
+    env->coverage_connection()->Start();
+  }
+  if (env->options()->cpu_prof) {
+    const std::string& dir = env->options()->cpu_prof_dir;
+    env->set_cpu_prof_dir(dir.empty() ? GetCwd() : dir);
+    if (env->options()->cpu_prof_name.empty()) {
+      DiagnosticFilename filename(env, "CPU", "cpuprofile");
+      env->set_cpu_prof_name(*filename);
+    } else {
+      env->set_cpu_prof_name(env->options()->cpu_prof_name);
+    }
+    CHECK_NULL(env->cpu_profiler_connection());
+    env->set_cpu_profiler_connection(
+        std::make_unique<V8CpuProfilerConnection>(env));
+    env->cpu_profiler_connection()->Start();
   }
-  env->set_cpu_profile_path(std::move(path));
-  env->set_cpu_profiler_connection(
-      std::make_unique<V8CpuProfilerConnection>(env));
-  env->cpu_profiler_connection()->Start();
 }
 
 static void SetCoverageDirectory(const FunctionCallbackInfo<Value>& args) {

diff --git a/src/node.cc b/src/node.cc
@@ -227,21 +227,8 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
   Isolate* isolate = env->isolate();
   Local<Context> context = env->context();
 
-  Local<String> coverage_str = env->env_vars()->Get(
-      isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE"));
-  if (!coverage_str.IsEmpty() && coverage_str->Length() > 0) {
 #if HAVE_INSPECTOR
-    profiler::StartCoverageCollection(env);
-#else
-    fprintf(stderr, "NODE_V8_COVERAGE cannot be used without inspector\n");
-#endif  // HAVE_INSPECTOR
-  }
-
-#if HAVE_INSPECTOR
-  if (env->options()->cpu_prof) {
-    env->InitializeCPUProfDir(env->options()->cpu_prof_dir);
-    profiler::StartCpuProfiling(env, env->options()->cpu_prof_name);
-  }
+  profiler::StartProfilers(env);
 #endif  // HAVE_INSPECTOR
 
   // Add a reference to the global object

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -324,8 +324,7 @@ void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params);
 
 #if HAVE_INSPECTOR
 namespace profiler {
-void StartCoverageCollection(Environment* env);
-void StartCpuProfiling(Environment* env, const std::string& profile_name);
+void StartProfilers(Environment* env);
 void EndStartedProfilers(Environment* env);
 }
 #endif  // HAVE_INSPECTOR