tools: change tick processor install path#4021
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Author
matthewloring commented Nov 25, 2015
@bnoordhuis Does $PREFIX/share seem like a more appropriate place? I'm not too familiar with the standard installation structure. |
Contributor
Nibbler999 commented Nov 25, 2015
$PREFIX/bin would be the logical place. |
Contributor
evanlucas commented Nov 25, 2015
I agree that putting it in $PREFIX/bin would be logical. Although, if we do that, it could cause conflicts with naming it |
Member
bnoordhuis commented Nov 25, 2015
$PREFIX/bin gets my vote as well. It's unfortunate there's an existing |
Contributor
evanlucas commented Nov 25, 2015
any reason for it to not be just |
Member
jbergstroem commented Nov 26, 2015
+1 to |
Author
matthewloring commented Dec 1, 2015
There didn't seem to be consensus on the name so I left it as |
Contributor
evanlucas commented Dec 1, 2015
LGTM |
tools/v8-prof/polyfill.js Outdated
Member
bnoordhuis commented Dec 1, 2015
LGTM with a nit / suggestion. I'm tagging this ctc-agenda to resolve the naming issue. |
Author
matthewloring commented Dec 1, 2015
Sounds good! |
Member
bnoordhuis commented Dec 2, 2015
Resolution from today's call is that it would be best to rename it to something that doesn't conflict with an existing package. An alternative solution is to bake the scripts into the node binary and put them behind a command line flag. |
Author
matthewloring commented Dec 2, 2015
The original name |
Member
bnoordhuis commented Dec 2, 2015
I don't really like either but I guess nodejs-tick-processor is the least objectionable. |
Author
matthewloring commented Dec 2, 2015
I've changed the name to |
Member
rvagg commented Dec 2, 2015
The most attractive option that came up during the call today was to bake it in and make it work something like IMO "tick processor" is a weird naming that doesn't really point to what it does and it'd be good to move from it if possible. It's kind of like the misnaming of GL "shaders". |
Author
matthewloring commented Dec 2, 2015
I am happy to pursue this route but I'm not sure how to get started or where the change would live. |
Member
rvagg commented Dec 2, 2015
@matthewloring maybe the best entry-point would be in src/node.js, have a look at the section with |
Author
matthewloring commented Dec 3, 2015
I've got a prototype exposing the functionality behind the |
Member
bnoordhuis commented Dec 3, 2015
You can bake them into the binary with this patch: diff --git a/node.gyp b/node.gyp index a970c76..c67e369 100644 --- a/node.gyp+++ b/node.gyp@@ -79,6 +79,19 @@ 'lib/internal/socket_list.js', 'lib/internal/util.js', 'lib/internal/streams/lazy_transform.js', + 'deps/v8/tools/SourceMap.js',+ 'deps/v8/tools/codemap.js',+ 'deps/v8/tools/consarray.js',+ 'deps/v8/tools/consarray.js',+ 'deps/v8/tools/csvparser.js',+ 'deps/v8/tools/csvparser.js',+ 'deps/v8/tools/logreader.js',+ 'deps/v8/tools/profile.js',+ 'deps/v8/tools/profile_view.js',+ 'deps/v8/tools/splaytree.js',+ 'deps/v8/tools/tickprocessor-driver.js',+ 'deps/v8/tools/tickprocessor.js',+ 'tools/v8-prof/polyfill.js', ], }, diff --git a/tools/js2c.py b/tools/js2c.py index 0fc0ae0..ec9705e 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,And then they're accessible through |
Author
matthewloring commented Dec 3, 2015
@bnoordhuis@rvagg I've got the flag working and tests passing with the latest revision. |
Contributor
Fishrock123 commented Dec 4, 2015
@matthewloring I'll try to look at this tomorrow |
Author
matthewloring commented Dec 4, 2015
@Fishrock123 Thanks! |
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.
Member
There was a problem hiding this comment.
Is it possible to do this without writing temp files and forking a new process? I imagine you can just concatenate the files and then eval() or vm.runInNewContext() them.
(EDIT: It's perhaps a bit of a pain to send the output through c++filt. Shouldn't be impossible, though.)
Author
There was a problem hiding this comment.
This would definitely be a big improvement. The current state was the easiest way to get things working but I'm hoping to get rid of the temp files/forking in a future PR.
Member
rvagg commented Jan 12, 2016
No strong opinion, happy for this to be on lts-agenda though cause it's another interesting topic for us to cover in general for helping form policy. |
Member
jasnell commented Jan 12, 2016
I'm generally ok with this. There shouldn't be any real impact on LTS deployments. |
Contributor
MylesBorins commented Feb 17, 2016
@nodejs/lts can people chime in on this one? We have a good window right now with v4.4.0 to get this change in |
Member
jasnell commented Feb 17, 2016
LGTM for v4.4 |
Contributor
MylesBorins commented Feb 22, 2016
@nodejs/lts can I please get another lgtm :D |
Member
rvagg commented Feb 23, 2016
lgtm for v4.4, I wouldn't +1 it if it was the only reason to bump semver-minor on LTS though |
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Mar 1, 2016
This change cleans up outstanding comments on nodejs#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: nodejs#4021 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 1, 2016
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]>
Merged
MylesBorins pushed a commit that referenced this pull request Mar 1, 2016
In December we announced that we would be doing a minor release in order to get a number of voted on SEMVER-MINOR changes into LTS. Our ability to release this was delayed due to the unforeseen security release v4.3. We are quickly bumping to v4.4 in order to bring you the features that we had committed to releasing. This release also includes security updates to openssl. More information can be found [on nodejs.org](https://nodejs.org/en/blog/vulnerability/openssl-march-2016/) This release also includes over 70 fixes to our docs and over 50 fixes to tests. The SEMVER-MINOR changes include: * deps: - An update to v8 that introduces a new flag --perf_basic_prof_only_functions (Ali Ijaz Sheikh) #3609 * http: - A new feature in http(s) agent that catches errors on *keep alived* connections (José F. Romaniello) #4482 * src: - Better support for Big-Endian systems (Bryon Leung) #3410 * tls: - A new feature that allows you to pass common SSL options to `tls.createSecurePair` (Коренберг Марк) #2441 * tools - a new flag `--prof-process` which will execute the tick processor on the provided isolate files (Matt Loring) #4021 Notable semver patch changes include: * buld: - Support python path that includes spaces. This should be of particular interest to our Windows users who may have python living in `c:/Program Files` (Felix Becker) #4841 * https: - A potential fix for #3692 HTTP/HTTPS client requests throwing EPROTO (Fedor Indutny) #4982 * installer: - More readable profiling information from isolate tick logs (Matt Loring) #3032 * *npm: - upgrade to npm 2.14.20 (Kat Marchán) #5510 * *openssl: - upgrade openssl to 1.0.2g (Ben Noordhuis) #5507 * process: - Add support for symbols in event emitters. Symbols didn't exist when it was written ¯\_(ツ)_/¯ (cjihrig) #4798 * querystring: - querystring.parse() is now 13-22% faster! (Brian White) #4675 * streams: - performance improvements for moving small buffers that shows a 5% throughput gain. IoT projects have been seen to be as much as 10% faster with this change! (Matteo Collina) #4354 * tools: - eslint has been updated to version 2.1.0 (Rich Trott) #5214 PR-URL: #5301
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Mar 1, 2016
In December we announced that we would be doing a minor release in order to get a number of voted on SEMVER-MINOR changes into LTS. Our ability to release this was delayed due to the unforeseen security release v4.3. We are quickly bumping to v4.4 in order to bring you the features that we had committed to releasing. This release also includes security updates to openssl. More information can be found [on nodejs.org](https://nodejs.org/en/blog/vulnerability/openssl-march-2016/) This release also includes over 70 fixes to our docs and over 50 fixes to tests. The SEMVER-MINOR changes include: * deps: - An update to v8 that introduces a new flag --perf_basic_prof_only_functions (Ali Ijaz Sheikh) nodejs#3609 * http: - A new feature in http(s) agent that catches errors on *keep alived* connections (José F. Romaniello) nodejs#4482 * src: - Better support for Big-Endian systems (Bryon Leung) nodejs#3410 * tls: - A new feature that allows you to pass common SSL options to `tls.createSecurePair` (Коренберг Марк) nodejs#2441 * tools - a new flag `--prof-process` which will execute the tick processor on the provided isolate files (Matt Loring) nodejs#4021 Notable semver patch changes include: * buld: - Support python path that includes spaces. This should be of particular interest to our Windows users who may have python living in `c:/Program Files` (Felix Becker) nodejs#4841 * https: - A potential fix for nodejs#3692 HTTP/HTTPS client requests throwing EPROTO (Fedor Indutny) nodejs#4982 * installer: - More readable profiling information from isolate tick logs (Matt Loring) nodejs#3032 * *npm: - upgrade to npm 2.14.20 (Kat Marchán) nodejs#5510 * *openssl: - upgrade openssl to 1.0.2g (Ben Noordhuis) nodejs#5507 * process: - Add support for symbols in event emitters. Symbols didn't exist when it was written ¯\_(ツ)_/¯ (cjihrig) nodejs#4798 * querystring: - querystring.parse() is now 13-22% faster! (Brian White) nodejs#4675 * streams: - performance improvements for moving small buffers that shows a 5% throughput gain. IoT projects have been seen to be as much as 10% faster with this change! (Matteo Collina) nodejs#4354 * tools: - eslint has been updated to version 2.1.0 (Rich Trott) nodejs#5214 PR-URL: nodejs#5301
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
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]>
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
In December we announced that we would be doing a minor release in order to get a number of voted on SEMVER-MINOR changes into LTS. Our ability to release this was delayed due to the unforeseen security release v4.3. We are quickly bumping to v4.4 in order to bring you the features that we had committed to releasing. This release also includes over 70 fixes to our docs and over 50 fixes to tests. The SEMVER-MINOR changes include: * deps: - An update to v8 that introduces a new flag --perf_basic_prof_only_functions (Ali Ijaz Sheikh) #3609 * http: - A new feature in http(s) agent that catches errors on *keep alived* connections (José F. Romaniello) #4482 * src: - Better support for Big-Endian systems (Bryon Leung) #3410 * tls: - A new feature that allows you to pass common SSL options to `tls.createSecurePair` (Коренберг Марк) #2441 * tools - a new flag `--prof-process` which will execute the tick processor on the provided isolate files (Matt Loring) #4021 Notable semver patch changes include: * buld: - Support python path that includes spaces. This should be of particular interest to our Windows users who may have python living in `c:/Program Files` (Felix Becker) #4841 * https: - A potential fix for #3692 HTTP/HTTPS client requests throwing EPROTO (Fedor Indutny) #4982 * installer: - More readable profiling information from isolate tick logs (Matt Loring) #3032 * *npm: - upgrade to npm 2.14.20 (Kat Marchán) #5510 * process: - Add support for symbols in event emitters. Symbols didn't exist when it was written ¯\_(ツ)_/¯ (cjihrig) #4798 * querystring: - querystring.parse() is now 13-22% faster! (Brian White) #4675 * streams: - performance improvements for moving small buffers that shows a 5% throughput gain. IoT projects have been seen to be as much as 10% faster with this change! (Matteo Collina) #4354 * tools: - eslint has been updated to version 2.1.0 (Rich Trott) #5214 PR-URL: #5301
MylesBorins pushed a commit that referenced this pull request Mar 3, 2016
In December we announced that we would be doing a minor release in order to get a number of voted on SEMVER-MINOR changes into LTS. Our ability to release this was delayed due to the unforeseen security release v4.3. We are quickly bumping to v4.4 in order to bring you the features that we had committed to releasing. This release also includes over 70 fixes to our docs and over 50 fixes to tests. The SEMVER-MINOR changes include: * deps: - An update to v8 that introduces a new flag --perf_basic_prof_only_functions (Ali Ijaz Sheikh) #3609 * http: - A new feature in http(s) agent that catches errors on *keep alived* connections (José F. Romaniello) #4482 * src: - Better support for Big-Endian systems (Bryon Leung) #3410 * tls: - A new feature that allows you to pass common SSL options to `tls.createSecurePair` (Коренберг Марк) #2441 * tools - a new flag `--prof-process` which will execute the tick processor on the provided isolate files (Matt Loring) #4021 Notable semver patch changes include: * buld: - Support python path that includes spaces. This should be of particular interest to our Windows users who may have python living in `c:/Program Files` (Felix Becker) #4841 * https: - A potential fix for #3692 HTTP/HTTPS client requests throwing EPROTO (Fedor Indutny) #4982 * installer: - More readable profiling information from isolate tick logs (Matt Loring) #3032 * *npm: - upgrade to npm 2.14.20 (Kat Marchán) #5510 * process: - Add support for symbols in event emitters. Symbols didn't exist when it was written ¯\_(ツ)_/¯ (cjihrig) #4798 * querystring: - querystring.parse() is now 13-22% faster! (Brian White) #4675 * streams: - performance improvements for moving small buffers that shows a 5% throughput gain. IoT projects have been seen to be as much as 10% faster with this change! (Matteo Collina) #4354 * tools: - eslint has been updated to version 2.1.0 (Rich Trott) #5214 PR-URL: #5301
MylesBorins pushed a commit that referenced this pull request Mar 8, 2016
In December we announced that we would be doing a minor release in order to get a number of voted on SEMVER-MINOR changes into LTS. Our ability to release this was delayed due to the unforeseen security release v4.3. We are quickly bumping to v4.4 in order to bring you the features that we had committed to releasing. This release also includes over 70 fixes to our docs and over 50 fixes to tests. The SEMVER-MINOR changes include: * deps: - An update to v8 that introduces a new flag --perf_basic_prof_only_functions (Ali Ijaz Sheikh) #3609 * http: - A new feature in http(s) agent that catches errors on *keep alived* connections (José F. Romaniello) #4482 * src: - Better support for Big-Endian systems (Bryon Leung) #3410 * tls: - A new feature that allows you to pass common SSL options to `tls.createSecurePair` (Коренберг Марк) #2441 * tools - a new flag `--prof-process` which will execute the tick processor on the provided isolate files (Matt Loring) #4021 Notable semver patch changes include: * buld: - Support python path that includes spaces. This should be of particular interest to our Windows users who may have python living in `c:/Program Files` (Felix Becker) #4841 * https: - A potential fix for #3692 HTTP/HTTPS client requests throwing EPROTO (Fedor Indutny) #4982 * installer: - More readable profiling information from isolate tick logs (Matt Loring) #3032 * *npm: - upgrade to npm 2.14.20 (Kat Marchán) #5510 * process: - Add support for symbols in event emitters. Symbols didn't exist when it was written ¯\_(ツ)_/¯ (cjihrig) #4798 * querystring: - querystring.parse() is now 13-22% faster! (Brian White) #4675 * streams: - performance improvements for moving small buffers that shows a 5% throughput gain. IoT projects have been seen to be as much as 10% faster with this change! (Matteo Collina) #4354 * tools: - eslint has been updated to version 2.1.0 (Rich Trott) #5214 PR-URL: #5301
MylesBorins pushed a commit that referenced this pull request Mar 9, 2016
In December we announced that we would be doing a minor release in order to get a number of voted on SEMVER-MINOR changes into LTS. Our ability to release this was delayed due to the unforeseen security release v4.3. We are quickly bumping to v4.4 in order to bring you the features that we had committed to releasing. This release also includes over 70 fixes to our docs and over 50 fixes to tests. The SEMVER-MINOR changes include: * deps: - An update to v8 that introduces a new flag --perf_basic_prof_only_functions (Ali Ijaz Sheikh) #3609 * http: - A new feature in http(s) agent that catches errors on *keep alived* connections (José F. Romaniello) #4482 * src: - Better support for Big-Endian systems (Bryon Leung) #3410 * tls: - A new feature that allows you to pass common SSL options to `tls.createSecurePair` (Коренберг Марк) #2441 * tools - a new flag `--prof-process` which will execute the tick processor on the provided isolate files (Matt Loring) #4021 Notable semver patch changes include: * buld: - Support python path that includes spaces. This should be of particular interest to our Windows users who may have python living in `c:/Program Files` (Felix Becker) #4841 * https: - A potential fix for #3692 HTTP/HTTPS client requests throwing EPROTO (Fedor Indutny) #4982 * installer: - More readable profiling information from isolate tick logs (Matt Loring) #3032 * *npm: - upgrade to npm 2.14.20 (Kat Marchán) #5510 * process: - Add support for symbols in event emitters. Symbols didn't exist when it was written ¯\_(ツ)_/¯ (cjihrig) #4798 * querystring: - querystring.parse() is now 13-22% faster! (Brian White) #4675 * streams: - performance improvements for moving small buffers that shows a 5% throughput gain. IoT projects have been seen to be as much as 10% faster with this change! (Matteo Collina) #4354 * tools: - eslint has been updated to version 2.1.0 (Rich Trott) #5214 PR-URL: #5301
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
This change cleans up outstanding comments on nodejs#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: nodejs#4021 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Notable changes: * build: - Add support for Intel's VTune JIT profiling when compiled with --enable-vtune-profiling. For more information about VTune, see https://software.intel.com/en-us/node/544211. (Chunyang Dai) nodejs#3785. - Properly enable V8 snapshots by default. Due to a configuration error, snapshots have been kept off by default when the intention is for the feature to be enabled. (Fedor Indutny) nodejs#3962. * crypto: - Simplify use of ECDH (Elliptic Curve Diffie-Hellman) objects (created via crypto.createECDH(curve_name)) with private keys that are not dynamically generated via generateKeys(). The public key is now computed when explicitly setting a private key. Added validity checks to reduce the possibility of computing weak or invalid shared secrets. Also, deprecated the setPublicKey() method for ECDH objects as its usage is unnecessary and can lead to inconsistent state. (Michael Ruddy) nodejs#3511. - Update root certificates from the current list stored maintained by Mozilla NSS. (Ben Noordhuis) nodejs#3951. - Multiple CA certificates can now be passed with the ca option to TLS methods as an array of strings or in a single new-line separated string. (Ben Noordhuis) nodejs#4099 * tools: Include a tick processor in core, exposed via the --prof-process command-line argument which can be used to process V8 profiling output files generated when using the --prof command-line argument. (Matt Loring) nodejs#4021. PR-URL: nodejs#4181
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change cleans up outstanding comments on #3032. It improves error
handling when no isolate file is provided and changes the installation
location of the processor to $PREFIX/share/node-tick-processor.