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.

Sign up for GitHub

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

Jump to bottom

cli: allow --cpu-prof* in NODE_OPTIONS #57018

Merged
merged 1 commit into from
Feb 20, 2025
Merged

cli: allow --cpu-prof* in NODE_OPTIONS #57018

merged 1 commit into from
Feb 20, 2025

Conversation

Ceres6
Copy link
Contributor

@Ceres6 Ceres6 commented Feb 12, 2025

Added --cpu-prof* flags to NODE_OPTIONS envvar and docs

Fixes: #56944

@Ceres6
cli: allow --cpu-prof* in NODE_OPTIONS
Loading
Loading status checks…
e5083e3 
Fixes: nodejs#56944
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 12, 2025
@Ceres6
Copy link
Contributor Author

Ceres6 commented Feb 12, 2025

cc @dmichon-msft @joyeecheung @Qard

joyeecheung
joyeecheung approved these changes Feb 12, 2025
View reviewed changes
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 12, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 12, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65157/

@codecov Codecov
Copy link

codecov bot commented Feb 12, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.12%. Comparing base (1671921) to head (e5083e3).
Report is 450 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #57018      +/-   ##
==========================================
- Coverage   89.15%   89.12%   -0.04%     
==========================================
  Files         665      665              
  Lines      192798   193193     +395     
  Branches    37130    37223      +93     
==========================================
+ Hits       171886   172177     +291     
- Misses      13673    13757      +84     
- Partials     7239     7259      +20
Files with missing lines Coverage Δ
src/node_options.cc 87.97% <ø> (+0.01%) ⬆️

... and 65 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Armanidashh
Armanidashh reviewed Feb 13, 2025
View reviewed changes
Copy link

@Armanidashh Armanidashh left a comment

Choose a reason for hiding this comment

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

Merge

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65183/

@Qard Qard added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 20, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 20, 2025
@nodejs-github-bot nodejs-github-bot merged commit 15fec13 into nodejs:main Feb 20, 2025
66 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 15fec13

acidiney pushed a commit to acidiney/node that referenced this pull request Feb 23, 2025
@Ceres6 @acidiney
cli: allow --cpu-prof* in NODE_OPTIONS

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
a0156a4 
Fixes: nodejs#56944
PR-URL: nodejs#57018
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit that referenced this pull request Feb 24, 2025
@Ceres6 @targos
cli: allow --cpu-prof* in NODE_OPTIONS
891a239 
Fixes: #56944
PR-URL: #57018
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@github-actions github-actions bot mentioned this pull request Feb 25, 2025
Merged
targos pushed a commit that referenced this pull request Feb 25, 2025
@Ceres6 @targos
cli: allow --cpu-prof* in NODE_OPTIONS
e26d484 
Fixes: #56944
PR-URL: #57018
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@dmichon-msft
Copy link

Any chance of this getting backported to 22.x?

aduh95 pushed a commit that referenced this pull request Apr 2, 2025
@Ceres6 @aduh95
cli: allow --cpu-prof* in NODE_OPTIONS
41ad504 
Fixes: #56944
PR-URL: #57018
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Apr 3, 2025
@Ceres6 @aduh95
cli: allow --cpu-prof* in NODE_OPTIONS
84ac1db 
Fixes: #56944
PR-URL: #57018
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@kettanaito
Copy link

@dmichon-msft, this has been backported to v22 from what I can see on v22.x-staging:

node/src/node_options.cc

Lines 624 to 629 in 7aca254

AddOption("--cpu-prof",
"Start the V8 CPU profiler on start up, and write the CPU profile "
"to disk before exit. If --cpu-prof-dir is not specified, write "
"the profile to the current working directory.",
&EnvironmentOptions::cpu_prof,
kAllowedInEnvvar);

but it doesn't seem to be released yet:

node/src/node_options.cc

Lines 624 to 628 in be79f4a

AddOption("--cpu-prof",
"Start the V8 CPU profiler on start up, and write the CPU profile "
"to disk before exit. If --cpu-prof-dir is not specified, write "
"the profile to the current working directory.",
&EnvironmentOptions::cpu_prof);

Would be great to issue a new v22 release and have it out for everyone!

@aduh95 aduh95 added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 8, 2025
@marco-ippolito marco-ippolito added the lts-watch-v22.x PRs that may need to be released in v22.x label Apr 8, 2025
RafaelGSS added a commit that referenced this pull request Apr 11, 2025
@RafaelGSS
2025-04-15, Version 22.15.0 'Jod' (LTS)
5f08eb5 
Notable changes:

assert:
  * (SEMVER-MINOR) implement partial error comparison (Ruben Bridgewater) #57370
  * (SEMVER-MINOR) improve partialDeepStrictEqual (Ruben Bridgewater) #57370
assert,util:
  * (SEMVER-MINOR) improve performance (Ruben Bridgewater) #57370
benchmark:
  * (SEMVER-MINOR) adjust assert runtimes (Ruben Bridgewater) #57370
  * (SEMVER-MINOR) skip running some assert benchmarks by default (Ruben Bridgewater) #57370
  * (SEMVER-MINOR) add assert partialDeepStrictEqual benchmark (Ruben Bridgewater) #57370
cli:
  * (SEMVER-MINOR) allow --cpu-prof* in NODE_OPTIONS (Carlos Espa) #57018
crypto:
  * update root certificates to NSS 3.108 (Node.js GitHub Bot) #57381
  * (SEMVER-MINOR) support --use-system-ca on Windows (Joyee Cheung) #56833
  * (SEMVER-MINOR) added support for reading certificates from macOS system store (Tim Jacomb) #56599
deps:
  * update timezone to 2025a (Node.js GitHub Bot) #56876
  * (SEMVER-MINOR) update ada to v3.0.1 (Yagiz Nizipli) #56452
deps,tools:
  * (SEMVER-MINOR) add zstd 1.5.6 (Jan Martin) #52100
dns:
  * (SEMVER-MINOR) add TLSA record query and parsing (Rithvik Vibhu) #52983
doc:
  * add @geeksilva97 to collaborators (Edy Silva) #57241
module:
  * (SEMVER-MINOR) use synchronous hooks for preparsing in import(cjs) (Joyee Cheung) #55698
  * (SEMVER-MINOR) implement module.registerHooks() (Joyee Cheung) #55698
process:
  * (SEMVER-MINOR) add execve (Paolo Insogna) #56496
  * (SEMVER-MINOR) add threadCpuUsage (Paolo Insogna) #56467
sqlite:
  * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490
  * (SEMVER-MINOR) allow returning `ArrayBufferView`s from user-defined functions (René) #56790
src:
  * set signal inspector io thread name (RafaelGSS) #56416
  * set thread name for main thread and v8 worker (RafaelGSS) #56416
  * set worker thread name using worker.name (RafaelGSS) #56416
  * use a default thread name for inspector (RafaelGSS) #56416
test:
  * (SEMVER-MINOR) add WPT for URLPattern (Yagiz Nizipli) #56452
tls:
  * (SEMVER-MINOR) implement tls.getCACertificates() (Joyee Cheung) #57107
url:
  * (SEMVER-MINOR) add URLPattern implementation (Yagiz Nizipli) #56452
util:
  * (SEMVER-MINOR) expose diff function used by the assertion errors (Giovanni Bucci) #57462
v8:
  * (SEMVER-MINOR) add v8.getCppHeapStatistics() method (Aditi) #57146
zlib:
  * (SEMVER-MINOR) add zstd support (Jan Martin) #52100

PR-URL: TODO
@RafaelGSS RafaelGSS mentioned this pull request Apr 11, 2025
Open
RafaelGSS added a commit that referenced this pull request Apr 11, 2025
@RafaelGSS
2025-04-15, Version 22.15.0 'Jod' (LTS)
e1372dd 
Notable changes:

assert:
  * (SEMVER-MINOR) implement partial error comparison (Ruben Bridgewater) #57370
  * (SEMVER-MINOR) improve partialDeepStrictEqual (Ruben Bridgewater) #57370
assert,util:
  * (SEMVER-MINOR) improve performance (Ruben Bridgewater) #57370
benchmark:
  * (SEMVER-MINOR) adjust assert runtimes (Ruben Bridgewater) #57370
  * (SEMVER-MINOR) skip running some assert benchmarks by default (Ruben Bridgewater) #57370
  * (SEMVER-MINOR) add assert partialDeepStrictEqual benchmark (Ruben Bridgewater) #57370
cli:
  * (SEMVER-MINOR) allow --cpu-prof* in NODE_OPTIONS (Carlos Espa) #57018
crypto:
  * update root certificates to NSS 3.108 (Node.js GitHub Bot) #57381
  * (SEMVER-MINOR) support --use-system-ca on Windows (Joyee Cheung) #56833
  * (SEMVER-MINOR) added support for reading certificates from macOS system store (Tim Jacomb) #56599
deps:
  * update timezone to 2025a (Node.js GitHub Bot) #56876
  * (SEMVER-MINOR) update ada to v3.0.1 (Yagiz Nizipli) #56452
deps,tools:
  * (SEMVER-MINOR) add zstd 1.5.6 (Jan Martin) #52100
dns:
  * (SEMVER-MINOR) add TLSA record query and parsing (Rithvik Vibhu) #52983
doc:
  * add @geeksilva97 to collaborators (Edy Silva) #57241
module:
  * (SEMVER-MINOR) use synchronous hooks for preparsing in import(cjs) (Joyee Cheung) #55698
  * (SEMVER-MINOR) implement module.registerHooks() (Joyee Cheung) #55698
process:
  * (SEMVER-MINOR) add execve (Paolo Insogna) #56496
  * (SEMVER-MINOR) add threadCpuUsage (Paolo Insogna) #56467
sqlite:
  * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490
  * (SEMVER-MINOR) allow returning `ArrayBufferView`s from user-defined functions (René) #56790
src:
  * set signal inspector io thread name (RafaelGSS) #56416
  * set thread name for main thread and v8 worker (RafaelGSS) #56416
  * set worker thread name using worker.name (RafaelGSS) #56416
  * use a default thread name for inspector (RafaelGSS) #56416
test:
  * (SEMVER-MINOR) add WPT for URLPattern (Yagiz Nizipli) #56452
tls:
  * (SEMVER-MINOR) implement tls.getCACertificates() (Joyee Cheung) #57107
url:
  * (SEMVER-MINOR) add URLPattern implementation (Yagiz Nizipli) #56452
util:
  * (SEMVER-MINOR) expose diff function used by the assertion errors (Giovanni Bucci) #57462
v8:
  * (SEMVER-MINOR) add v8.getCppHeapStatistics() method (Aditi) #57146
zlib:
  * (SEMVER-MINOR) add zstd support (Jan Martin) #52100

PR-URL: #57840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Armanidashh Armanidashh

@Qard Qard

@joyeecheung joyeecheung

@legendecas legendecas

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lts-watch-v22.x PRs that may need to be released in v22.x needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support --cpu-prof in NODE_OPTIONS
10 participants
@Ceres6 @nodejs-github-bot @dmichon-msft @kettanaito @Qard @joyeecheung @legendecas @Armanidashh @aduh95 @marco-ippolito