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: #4021 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
nodejs · Dec 8, 2015 · 1c1c1a0 · 1c1c1a0
1 parent da3137d
commit 1c1c1a0
Show file tree

Hide file tree

Showing 12 changed files with 88 additions and 107 deletions.
diff --git 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
@@ -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 → lib/internal/v8_prof_polyfill.js b/tools/v8-prof/polyfill.js → 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <[email protected]>
-- Added tick processor.
-
 * Tue Jul 7 2015 Ali Ijaz Sheikh <[email protected]>
 - Added gdbinit.
 

diff --git a/tools/v8-prof/tick-processor.js b/tools/v8-prof/tick-processor.js