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

src,lib: add performance.uvMetricsInfo #54413

Merged
merged 9 commits into from
Aug 30, 2024

Conversation

RafaelGSS
Copy link
Member

This commit exposes a new API to the perf_hooks.performance module. This wraps uv_metrics_info into
performance.uvMetricsInfo() function.

cc: @trevnorris

This commit exposes a new API to the perf_hooks.performance
module. This wraps uv_metrics_info into
performance.uvMetricsInfo() function.
@RafaelGSS RafaelGSS added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 16, 2024
@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 Aug 16, 2024
Copy link
Contributor

@trevnorris trevnorris left a comment

Choose a reason for hiding this comment

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

I really appreciate you doing this. Thanks very much.

doc/api/perf_hooks.md Outdated Show resolved Hide resolved
doc/api/perf_hooks.md Outdated Show resolved Hide resolved
@RedYetiDev RedYetiDev added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label Aug 16, 2024
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

doc/api/perf_hooks.md Outdated Show resolved Hide resolved
doc/api/perf_hooks.md Outdated Show resolved Hide resolved
doc/api/perf_hooks.md Outdated Show resolved Hide resolved
doc/api/perf_hooks.md Outdated Show resolved Hide resolved
Co-authored-by: Tobias Nießen <tniessen@tnie.de>
@RafaelGSS RafaelGSS added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link

codecov bot commented Aug 17, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.31%. Comparing base (123693c) to head (0e9cf0e).
Report is 155 commits behind head on main.

Files with missing lines Patch % Lines
src/node_perf.cc 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54413      +/-   ##
==========================================
+ Coverage   87.10%   87.31%   +0.21%     
==========================================
  Files         648      649       +1     
  Lines      182215   182783     +568     
  Branches    34955    35046      +91     
==========================================
+ Hits       158712   159606     +894     
+ Misses      16789    16465     -324     
+ Partials     6714     6712       -2     
Files with missing lines Coverage Δ
lib/internal/perf/nodetiming.js 94.64% <100.00%> (+0.26%) ⬆️
src/node_perf.cc 86.83% <95.00%> (+0.73%) ⬆️

... and 117 files with indirect coverage changes

Copy link
Member

@H4ad H4ad left a comment

Choose a reason for hiding this comment

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

LGTM with some non-blocking comments.

lib/internal/perf/uv_metrics.js Outdated Show resolved Hide resolved
Comment on lines +271 to +283
Local<Object> obj = Object::New(env->isolate());
obj->Set(env->context(),
env->loop_count(),
Integer::NewFromUnsigned(env->isolate(), metrics.loop_count))
.Check();
obj->Set(env->context(),
env->events(),
Integer::NewFromUnsigned(env->isolate(), metrics.events))
.Check();
obj->Set(env->context(),
env->events_waiting(),
Integer::NewFromUnsigned(env->isolate(), metrics.events_waiting))
.Check();
Copy link
Member

Choose a reason for hiding this comment

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

Just a note for the future optimization: maybe we should use a buffer to store these values to avoid creating and returning js objects from c++.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I have plans to return a typed array and build the object in JS land. I'll do it in a follow up PR.

test/parallel/test-performance-uvmetricsinfo.js Outdated Show resolved Hide resolved
@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 20, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 20, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor

@trevnorris trevnorris left a comment

Choose a reason for hiding this comment

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

Excellent. Thanks for doing this.

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Aug 30, 2024
@RafaelGSS RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 30, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 30, 2024
@nodejs-github-bot nodejs-github-bot merged commit 9a275e1 into nodejs:main Aug 30, 2024
58 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 9a275e1

RafaelGSS added a commit that referenced this pull request Aug 30, 2024
This commit exposes a new API to the perf_hooks.performance
module. This wraps uv_metrics_info into
performance.uvMetricsInfo() function.

PR-URL: #54413
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS added a commit that referenced this pull request Aug 30, 2024
This commit exposes a new API to the perf_hooks.performance
module. This wraps uv_metrics_info into
performance.uvMetricsInfo() function.

PR-URL: #54413
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS added a commit that referenced this pull request Aug 30, 2024
This commit exposes a new API to the perf_hooks.performance
module. This wraps uv_metrics_info into
performance.uvMetricsInfo() function.

PR-URL: #54413
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS added a commit that referenced this pull request Aug 30, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
RafaelGSS added a commit that referenced this pull request Aug 30, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
@RafaelGSS RafaelGSS mentioned this pull request Aug 30, 2024
RafaelGSS added a commit that referenced this pull request Aug 31, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
RafaelGSS added a commit that referenced this pull request Aug 31, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
sendoru pushed a commit to sendoru/node that referenced this pull request Sep 1, 2024
This commit exposes a new API to the perf_hooks.performance
module. This wraps uv_metrics_info into
performance.uvMetricsInfo() function.

PR-URL: nodejs#54413
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS added a commit that referenced this pull request Sep 1, 2024
This commit exposes a new API to the perf_hooks.performance
module. This wraps uv_metrics_info into
performance.uvMetricsInfo() function.

PR-URL: #54413
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS added a commit that referenced this pull request Sep 1, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
RafaelGSS added a commit that referenced this pull request Sep 2, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
RafaelGSS added a commit that referenced this pull request Sep 3, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
RafaelGSS added a commit that referenced this pull request Sep 3, 2024
Notable changes:

net:
  * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264
src:
  * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501
src,lib:
  * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413
test_runner:
  * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429
  * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927
  * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927
vm:
  * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394

PR-URL: #54560
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. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. perf_hooks Issues and PRs related to the implementation of the Performance Timing API. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.