Skip to content

Commit

Permalink
tools: add --prof-process flag to node binary
Browse files Browse the repository at this point in the history
This change cleans up outstanding comments on #3032. It improves error
handling when no isolate file is provided and adds the --prof-process
flag to the node binary which executes the tick processor on the
provided isolate file.

PR-URL: #4021
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
  • Loading branch information
Matt Loring authored and bnoordhuis committed Dec 8, 2015
1 parent a04721d commit 49440b7
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 107 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
lib/internal/v8_prof_polyfill.js
lib/punycode.js
test/addons/doc-*/
test/fixtures
Expand Down
2 changes: 2 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ and servers.

--track-heap-objects track heap object allocations for heap snapshots

--prof-process process v8 profiler output generated using --prof

--v8-options print v8 command line options

--tls-cipher-list=list use an alternative default TLS cipher list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,14 @@ arguments = process.argv.slice(2);
var quit = process.exit;

// Polyfill "readline()".
var fd = fs.openSync(arguments[arguments.length - 1], 'r');
var logFile = arguments[arguments.length - 1];
try {
fs.accessSync(logFile);
} catch(e) {
console.error('Please provide a valid isolate file as the final argument.');
process.exit(1);
}
var fd = fs.openSync(logFile, 'r');
var buf = new Buffer(4096);
var dec = new (require('string_decoder').StringDecoder)('utf-8');
var line = '';
Expand Down
44 changes: 44 additions & 0 deletions lib/internal/v8_prof_processor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict';
var cp = require('child_process');
var fs = require('fs');
var path = require('path');

var scriptFiles = [
'internal/v8_prof_polyfill',
'v8/tools/splaytree',
'v8/tools/codemap',
'v8/tools/csvparser',
'v8/tools/consarray',
'v8/tools/profile',
'v8/tools/profile_view',
'v8/tools/logreader',
'v8/tools/tickprocessor',
'v8/tools/SourceMap',
'v8/tools/tickprocessor-driver'
];
var tempScript = 'tick-processor-tmp-' + process.pid;
var tempNm = 'mac-nm-' + process.pid;

process.on('exit', function() {
try { fs.unlinkSync(tempScript); } catch (e) {}
try { fs.unlinkSync(tempNm); } catch (e) {}
});
process.on('uncaughtException', function(err) {
try { fs.unlinkSync(tempScript); } catch (e) {}
try { fs.unlinkSync(tempNm); } catch (e) {}
throw err;
});

scriptFiles.forEach(function(script) {
fs.appendFileSync(tempScript, process.binding('natives')[script]);
});
var tickArguments = [tempScript];
if (process.platform === 'darwin') {
fs.writeFileSync(tempNm, process.binding('natives')['v8/tools/mac-nm'],
{ mode: 0o555 });
tickArguments.push('--mac', '--nm=' + path.join(process.cwd(), tempNm));
} else if (process.platform === 'win32') {
tickArguments.push('--windows');
}
tickArguments.push.apply(tickArguments, process.argv.slice(1));
cp.spawn(process.execPath, tickArguments, { stdio: 'inherit' });
13 changes: 13 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,20 @@
'lib/internal/repl.js',
'lib/internal/socket_list.js',
'lib/internal/util.js',
'lib/internal/v8_prof_polyfill.js',
'lib/internal/v8_prof_processor.js',
'lib/internal/streams/lazy_transform.js',
'deps/v8/tools/splaytree.js',
'deps/v8/tools/codemap.js',
'deps/v8/tools/consarray.js',
'deps/v8/tools/csvparser.js',
'deps/v8/tools/profile.js',
'deps/v8/tools/profile_view.js',
'deps/v8/tools/logreader.js',
'deps/v8/tools/tickprocessor.js',
'deps/v8/tools/SourceMap.js',
'deps/v8/tools/tickprocessor-driver.js',
'deps/v8/tools/mac-nm',
],
},

Expand Down
14 changes: 13 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ static const char** preload_modules = nullptr;
static bool use_debug_agent = false;
static bool debug_wait_connect = false;
static int debug_port = 5858;
static bool prof_process = false;
static bool v8_is_profiling = false;
static bool node_is_initialized = false;
static node_module* modpending;
Expand Down Expand Up @@ -2958,6 +2959,11 @@ void SetupProcessObject(Environment* env,
READONLY_PROPERTY(process, "throwDeprecation", True(env->isolate()));
}

// --prof-process
if (prof_process) {
READONLY_PROPERTY(process, "profProcess", True(env->isolate()));
}

// --trace-deprecation
if (trace_deprecation) {
READONLY_PROPERTY(process, "traceDeprecation", True(env->isolate()));
Expand Down Expand Up @@ -3195,6 +3201,8 @@ static void PrintHelp() {
" is detected after the first tick\n"
" --track-heap-objects track heap object allocations for heap "
"snapshots\n"
" --prof-process process v8 profiler output generated\n"
" using --prof\n"
" --v8-options print v8 command line options\n"
#if HAVE_OPENSSL
" --tls-cipher-list=val use an alternative default TLS cipher list\n"
Expand Down Expand Up @@ -3266,7 +3274,8 @@ static void ParseArgs(int* argc,
new_argv[0] = argv[0];

unsigned int index = 1;
while (index < nargs && argv[index][0] == '-') {
bool short_circuit = false;
while (index < nargs && argv[index][0] == '-' && !short_circuit) {
const char* const arg = argv[index];
unsigned int args_consumed = 1;

Expand Down Expand Up @@ -3327,6 +3336,9 @@ static void ParseArgs(int* argc,
track_heap_objects = true;
} else if (strcmp(arg, "--throw-deprecation") == 0) {
throw_deprecation = true;
} else if (strcmp(arg, "--prof-process") == 0) {
prof_process = true;
short_circuit = true;
} else if (strcmp(arg, "--v8-options") == 0) {
new_v8_argv[new_v8_argc] = "--help";
new_v8_argc += 1;
Expand Down
3 changes: 3 additions & 0 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
var d = NativeModule.require('_debug_agent');
d.start();

} else if (process.profProcess) {
NativeModule.require('internal/v8_prof_processor');

} else {
// There is user code to be run

Expand Down
8 changes: 3 additions & 5 deletions test/parallel/test-tick-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ var common = require('../common');

common.refreshTmpDir();
process.chdir(common.tmpDir);
var processor =
path.join(common.testDir, '..', 'tools', 'v8-prof', 'tick-processor.js');
// Unknown checked for to prevent flakiness, if pattern is not found,
// then a large number of unknown ticks should be present
runTest(/LazyCompile.*\[eval\]:1|.*% UNKNOWN/,
Expand Down Expand Up @@ -45,9 +43,9 @@ function runTest(pattern, code) {
assert.fail(null, null, 'There should be a single log file.');
}
var log = matches[0];
var out = cp.execSync(process.execPath + ' ' + processor +
' --call-graph-size=10 ' + log,
var out = cp.execSync(process.execPath +
' --prof-process --call-graph-size=10 ' + log,
{encoding: 'utf8'});
assert(out.match(pattern));
assert(pattern.test(out));
fs.unlinkSync(log);
}
44 changes: 0 additions & 44 deletions tools/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,48 +127,6 @@ def subdir_files(path, dest, action):
for subdir, files in ret.items():
action(files, subdir + '/')

def build_tick_processor(action):
tmp_script = 'out/Release/tick-processor'
if action == install:
# construct script
scripts = [
'tools/v8-prof/polyfill.js',
'deps/v8/tools/splaytree.js',
'deps/v8/tools/codemap.js',
'deps/v8/tools/csvparser.js',
'deps/v8/tools/consarray.js',
'deps/v8/tools/csvparser.js',
'deps/v8/tools/consarray.js',
'deps/v8/tools/profile.js',
'deps/v8/tools/profile_view.js',
'deps/v8/tools/logreader.js',
'deps/v8/tools/tickprocessor.js',
'deps/v8/tools/SourceMap.js',
'deps/v8/tools/tickprocessor-driver.js']
args = []
if sys.platform == 'win32':
args.append('--windows')
elif sys.platform == 'darwin':
args.append('--nm=' + abspath(install_path, 'share/doc/node') + '/mac-nm')
args.append('--mac')
with open(tmp_script, 'w') as out_file:
# Add #! line to run with node
out_file.write('#! ' + abspath(install_path, 'bin/node') + '\n')
# inject arguments
for arg in args:
out_file.write('process.argv.splice(2, 0, \'' + arg + '\');\n')
# cat in source files
for script in scripts:
with open(script) as in_file:
shutil.copyfileobj(in_file, out_file)
# make executable
st = os.stat(tmp_script)
os.chmod(tmp_script, st.st_mode | stat.S_IEXEC)
# perform installations
action([tmp_script], 'share/doc/node/')
if sys.platform == 'darwin':
action(['deps/v8/tools/mac-nm'], 'share/doc/node/')

def files(action):
is_windows = sys.platform == 'win32'

Expand All @@ -183,8 +141,6 @@ def files(action):

action(['deps/v8/tools/gdbinit'], 'share/doc/node/')

build_tick_processor(action)

if 'freebsd' in sys.platform or 'openbsd' in sys.platform:
action(['doc/node.1'], 'man/man1/')
else:
Expand Down
2 changes: 1 addition & 1 deletion tools/js2c.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def JS2C(source, target):
else:
ids.append((id, len(lines)))

escaped_id = id.replace('/', '_')
escaped_id = id.replace('-', '_').replace('/', '_')
source_lines.append(SOURCE_DECLARATION % {
'id': id,
'escaped_id': escaped_id,
Expand Down
4 changes: 0 additions & 4 deletions tools/rpm/node.spec
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,13 @@ done
/usr/include/*
/usr/lib/node_modules/
/usr/share/doc/node/gdbinit
/usr/share/doc/node/tick-processor
/usr/share/man/man1/node.1.gz
/usr/share/systemtap/tapset/node.stp
%{_datadir}/%{name}/
%doc CHANGELOG.md LICENSE README.md


%changelog
* Tue Sep 22 2015 Matt Loring <mattloring@google.com>
- Added tick processor.

* Tue Jul 7 2015 Ali Ijaz Sheikh <ofrobots@google.com>
- Added gdbinit.

Expand Down
51 changes: 0 additions & 51 deletions tools/v8-prof/tick-processor.js

This file was deleted.

3 comments on commit 49440b7

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewloring do you think it would be possible for this to imply --prof and pass that to v8?

@bnoordhuis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've experimented with running the profiler at program exit. It's pretty unreliable / confusing because it ends up profiling the tick processor scripts too and there is no good way to turn off the profiler beforehand.

@matthewloring
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the profiler profile the processing scripts is definitely an issue. It is also possible for multiple tick logs to be generated from the execution of the single script and determining which are relevant after the process exits is difficult.

Please sign in to comment.