Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: add profiling APIs to the diagnostics support document #22588

Closed

Conversation

mmarchini
Copy link
Contributor

The first commit updates Linux perf "Regular Testing in Node.js CI" to Yes since we have tests for Linux perf in our CI (as part of the node-test-commit-v8-linux job). The second commit adds V8's CodeEventHandler API and the --interpreted-frames-native-stack flag as "Not yet classified" tools/APIs, since the flag is essential for system profiling tools to correctly sample Node.js processes and the API is useful to avoid using --perf-basic-prof.

Checklist

We have tests for Linux perf under `test/v8-updates/test-linux-perf.js`,
and those tests are run as part of the `node-test-commit-v8-linux` job.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Aug 29, 2018
@mmarchini
Copy link
Contributor Author

cc @nodejs/diagnostics

@AndreasMadsen
Copy link
Member

Do you have some documentation for --interpreted-frames-native-stack? I would like to know more.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Can you please edit only the specific lines? It’s very hard to review otherwise.

Add V8 CodeEventHandler and --interpreted-frames-native-stack to our
diagnostics tiers of support document as Unclassified APIs.
@mmarchini
Copy link
Contributor Author

@mcollina done! (I was trying to keep the table aligned)

@AndreasMadsen from --v8-options:

  --interpreted-frames-native-stack (Show interpreted frames on the native stack (useful for external profilers).)
        type: bool  default: false

There flag is also mentioned in mmarchini/nodejs-production-diagnostic-tools and I'm writing blog posts to document how to use this flag with Linux perf, DTrace and similar tools.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina
Copy link
Member

+1 on keeping the table aligned, but maybe in a separate commit so it's easy to see the difference.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 31, 2018
@addaleax
Copy link
Member

Landed in de67ae6, 21e8ce2

@addaleax addaleax closed this Aug 31, 2018
addaleax pushed a commit that referenced this pull request Aug 31, 2018
We have tests for Linux perf under `test/v8-updates/test-linux-perf.js`,
and those tests are run as part of the `node-test-commit-v8-linux` job.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this pull request Aug 31, 2018
Add V8 CodeEventHandler and --interpreted-frames-native-stack to our
diagnostics tiers of support document as Unclassified APIs.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this pull request Aug 31, 2018
We have tests for Linux perf under `test/v8-updates/test-linux-perf.js`,
and those tests are run as part of the `node-test-commit-v8-linux` job.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this pull request Aug 31, 2018
Add V8 CodeEventHandler and --interpreted-frames-native-stack to our
diagnostics tiers of support document as Unclassified APIs.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Sep 3, 2018
We have tests for Linux perf under `test/v8-updates/test-linux-perf.js`,
and those tests are run as part of the `node-test-commit-v8-linux` job.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Sep 3, 2018
Add V8 CodeEventHandler and --interpreted-frames-native-stack to our
diagnostics tiers of support document as Unclassified APIs.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Sep 6, 2018
We have tests for Linux perf under `test/v8-updates/test-linux-perf.js`,
and those tests are run as part of the `node-test-commit-v8-linux` job.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Sep 6, 2018
Add V8 CodeEventHandler and --interpreted-frames-native-stack to our
diagnostics tiers of support document as Unclassified APIs.

PR-URL: #22588
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants