Skip to content

Commit

Permalink
inspector: implement --cpu-prof-interval
Browse files Browse the repository at this point in the history
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: #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>
  • Loading branch information
joyeecheung authored and targos committed May 6, 2019
1 parent 294d2ea commit de337bb
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 3 deletions.
10 changes: 10 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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_;
Expand Down
6 changes: 5 additions & 1 deletion src/inspector_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ using v8::Object;
using v8::String;
using v8::Value;

using v8_inspector::StringBuffer;
using v8_inspector::StringView;

#ifdef _WIN32
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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");
Expand Down
9 changes: 9 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.",
Expand Down
2 changes: 2 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/workload/fibonacci-worker-argv.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
]
});
65 changes: 64 additions & 1 deletion test/sequential/test-cpu-prof.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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());
Expand Down Expand Up @@ -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'),
Expand All @@ -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'),
Expand All @@ -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'),
Expand All @@ -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',
Expand All @@ -274,6 +335,8 @@ const env = {
{
tmpdir.refresh();
const output = spawnSync(process.execPath, [
'--cpu-prof-interval',
kCpuProfInterval,
'--cpu-prof-dir',
'prof',
'--cpu-prof',
Expand Down

0 comments on commit de337bb

Please sign in to comment.