Skip to content

Commit

Permalink
inspector: split --cpu-prof-path to --cpu-prof-dir and --cpu-prof-name
Browse files Browse the repository at this point in the history
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: #27306
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
joyeecheung committed Apr 22, 2019
1 parent a3d1922 commit 49d3d11
Show file tree
Hide file tree
Showing 12 changed files with 231 additions and 54 deletions.
26 changes: 18 additions & 8 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,28 +83,38 @@ added: REPLACEME
> Stability: 1 - Experimental
Starts the V8 CPU profiler on start up, and writes the CPU profile to disk
before exit. If `--cpu-prof-path` is not specified, the profile will be
written to `${cwd}/CPU.${yyyymmdd}.${hhmmss}.${pid}.${tid}.${seq}.cpuprofile`.
before exit.

If `--cpu-prof-dir` is not specified, the generated profile will be placed
in the current working directory.

If `--cpu-prof-name` is not specified, the generated profile will be
named `CPU.${yyyymmdd}.${hhmmss}.${pid}.${tid}.${seq}.cpuprofile`.

```console
$ node --cpu-prof index.js
$ ls *.cpuprofile
CPU.20190409.202950.15293.0.0.cpuprofile
```

### `--cpu-prof-path`
### `--cpu-prof-dir`
<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental
Location where the CPU profile generated by `--cpu-prof`
should be written to. When used alone, it implies `--cpu-prof`.
Specify the directory where the CPU profiles generated by `--cpu-prof` will
be placed.

```console
$ node --cpu-prof-path /tmp/test.cpuprofile index.js
```
### `--cpu-prof-name`
<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental
Specify the file name of the CPU profile generated by `--cpu-prof`.

### `--enable-fips`
<!-- YAML
Expand Down
14 changes: 9 additions & 5 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,17 @@ Print source-able bash completion script for Node.js.
.It Fl -cpu-prof
Start the V8 CPU profiler on start up, and write the CPU profile to disk
before exit. If
.Fl -cpu-prof-path
is not specified, the profile will be written to the current working directory.
.Fl -cpu-prof-dir
is not specified, the profile will be written to the current working directory
with a generated file name.
.
.It Fl -cpu-prof-path
Path the V8 CPU profile generated with
.It Fl -cpu-prof-dir
The directory where the CPU profiles generated by
.Fl -cpu-prof
will be written to. When used alone, it implies
will be placed.
.
.It Fl -cpu-prof-name
File name of the V8 CPU profile generated with
.Fl -cpu-prof
.
.It Fl -enable-fips
Expand Down
17 changes: 17 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down
18 changes: 18 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand Down
16 changes: 5 additions & 11 deletions src/inspector_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 18 additions & 6 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/workload/fibonacci-worker-argv.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

const { Worker } = require('worker_threads');
const path = require('path');
new Worker(path.join(__dirname, 'fibonacci.js'), {
execArgv: ['--cpu-prof']
});
4 changes: 1 addition & 3 deletions test/fixtures/workload/fibonacci-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,4 @@

const { Worker } = require('worker_threads');
const path = require('path');
new Worker(path.join(__dirname, 'fibonacci.js'), {
execArgv: ['--cpu-prof']
});
new Worker(path.join(__dirname, 'fibonacci.js'));
Loading

0 comments on commit 49d3d11

Please sign in to comment.