From 49440b7ea2496dbb7b343cf1872d96b92be470db Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Wed, 25 Nov 2015 06:08:58 -0800 Subject: [PATCH] tools: add --prof-process flag to node binary 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: https://github.com/nodejs/node/pull/4021 Reviewed-By: Ben Noordhuis Reviewed-By: Evan Lucas --- .eslintignore | 1 + doc/node.1 | 2 + .../internal/v8_prof_polyfill.js | 9 +++- lib/internal/v8_prof_processor.js | 44 ++++++++++++++++ node.gyp | 13 +++++ src/node.cc | 14 ++++- src/node.js | 3 ++ test/parallel/test-tick-processor.js | 8 ++- tools/install.py | 44 ---------------- tools/js2c.py | 2 +- tools/rpm/node.spec | 4 -- tools/v8-prof/tick-processor.js | 51 ------------------- 12 files changed, 88 insertions(+), 107 deletions(-) rename tools/v8-prof/polyfill.js => lib/internal/v8_prof_polyfill.js (93%) create mode 100644 lib/internal/v8_prof_processor.js delete mode 100644 tools/v8-prof/tick-processor.js diff --git a/.eslintignore b/.eslintignore index 6f1caaa873..507305bc8f 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,3 +1,4 @@ +lib/internal/v8_prof_polyfill.js lib/punycode.js test/addons/doc-*/ test/fixtures diff --git a/doc/node.1 b/doc/node.1 index 4926dcccf3..9af846ec1f 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -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 diff --git a/tools/v8-prof/polyfill.js b/lib/internal/v8_prof_polyfill.js similarity index 93% rename from tools/v8-prof/polyfill.js rename to lib/internal/v8_prof_polyfill.js index 24661c7002..1beae0e4e4 100644 --- a/tools/v8-prof/polyfill.js +++ b/lib/internal/v8_prof_polyfill.js @@ -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 = ''; diff --git a/lib/internal/v8_prof_processor.js b/lib/internal/v8_prof_processor.js new file mode 100644 index 0000000000..543f977d4a --- /dev/null +++ b/lib/internal/v8_prof_processor.js @@ -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' }); diff --git a/node.gyp b/node.gyp index a970c761d1..4f096f580a 100644 --- a/node.gyp +++ b/node.gyp @@ -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', ], }, diff --git a/src/node.cc b/src/node.cc index bf544c470d..819693559a 100644 --- a/src/node.cc +++ b/src/node.cc @@ -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; @@ -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())); @@ -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" @@ -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; @@ -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; diff --git a/src/node.js b/src/node.js index c8a4fcba45..059c65f7df 100644 --- a/src/node.js +++ b/src/node.js @@ -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 diff --git a/test/parallel/test-tick-processor.js b/test/parallel/test-tick-processor.js index 65da7362d9..796f6ee0aa 100644 --- a/test/parallel/test-tick-processor.js +++ b/test/parallel/test-tick-processor.js @@ -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/, @@ -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); } diff --git a/tools/install.py b/tools/install.py index 99cbbf249f..ead93966f0 100755 --- a/tools/install.py +++ b/tools/install.py @@ -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' @@ -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: diff --git a/tools/js2c.py b/tools/js2c.py index 0fc0ae0ee7..ec9705ec6a 100755 --- a/tools/js2c.py +++ b/tools/js2c.py @@ -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, diff --git a/tools/rpm/node.spec b/tools/rpm/node.spec index a86cdf74db..39b98ec554 100644 --- a/tools/rpm/node.spec +++ b/tools/rpm/node.spec @@ -94,7 +94,6 @@ 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}/ @@ -102,9 +101,6 @@ done %changelog -* Tue Sep 22 2015 Matt Loring -- Added tick processor. - * Tue Jul 7 2015 Ali Ijaz Sheikh - Added gdbinit. diff --git a/tools/v8-prof/tick-processor.js b/tools/v8-prof/tick-processor.js deleted file mode 100644 index b25bcb75a4..0000000000 --- a/tools/v8-prof/tick-processor.js +++ /dev/null @@ -1,51 +0,0 @@ -'use strict'; -var cp = require('child_process'); -var fs = require('fs'); -var path = require('path'); - -var toolsPath = path.join(__dirname, '..', '..', 'deps', 'v8', 'tools'); -var scriptFiles = [ - path.join(__dirname, 'polyfill.js'), - path.join(toolsPath, 'splaytree.js'), - path.join(toolsPath, 'codemap.js'), - path.join(toolsPath, 'csvparser.js'), - path.join(toolsPath, 'consarray.js'), - path.join(toolsPath, 'csvparser.js'), - path.join(toolsPath, 'consarray.js'), - path.join(toolsPath, 'profile.js'), - path.join(toolsPath, 'profile_view.js'), - path.join(toolsPath, 'logreader.js'), - path.join(toolsPath, 'tickprocessor.js'), - path.join(toolsPath, 'SourceMap.js'), - path.join(toolsPath, 'tickprocessor-driver.js')]; -var tempScript = path.join(__dirname, 'tick-processor-tmp-' + process.pid); - -process.on('exit', function() { - try { fs.unlinkSync(tempScript); } catch (e) {} -}); -process.on('uncaughtException', function(err) { - try { fs.unlinkSync(tempScript); } catch (e) {} - throw err; -}); - -var inStreams = scriptFiles.map(function(f) { - return fs.createReadStream(f); -}); -var outStream = fs.createWriteStream(tempScript); -inStreams.reduce(function(prev, curr, i) { - prev.on('end', function() { - curr.pipe(outStream, { end: i === inStreams.length - 1}); - }); - return curr; -}); -inStreams[0].pipe(outStream, { end: false }); -outStream.on('close', function() { - var tickArguments = [tempScript]; - if (process.platform === 'darwin') { - tickArguments.push('--mac', '--nm=' + path.join(toolsPath, 'mac-nm')); - } else if (process.platform === 'win32') { - tickArguments.push('--windows'); - } - tickArguments.push.apply(tickArguments, process.argv.slice(2)); - var processTicks = cp.spawn(process.execPath, tickArguments, { stdio: 'inherit' }); -});