-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
perf_hooks: performance milestone time origin timestamp improvement #51713
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
perf_hooks: performance milestone time origin timestamp improvement #51713
Conversation
5d0062f to
69e1fe6
Compare
H4ad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is always nice to see more usage of Fast APIs, thanks for the PR!
|
Not blocking but just a reminder - it would be faster if we just pass |
69e1fe6 to
ad89f83
Compare
|
Thanks for the suggestion @joyeecheung ! I've changed the code to pass I've updated the commit message and PR title since it's no more a Fast api implementation, could you please review it again 🙏 |
joyeecheung
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % comments. The timestamp is already a double and there is no need to cast it to uint64_t on the way.
ad89f83 to
5cd317b
Compare
5cd317b to
f5e6847
Compare
f5e6847 to
f098676
Compare
|
Could someone merge this ? 👀 |
Commit Queue failed- Loading data for nodejs/node/pull/51713 ✔ Done loading data for nodejs/node/pull/51713 ----------------------------------- PR info ------------------------------------ Title perf_hooks: performance milestone time origin timestamp improvement (#51713) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch IlyasShabi:fast-time-origin-timestamp -> nodejs:main Labels c++, needs-ci, needs-benchmark-ci Commits 1 - perf_hooks: performance milestone time origin timestamp improvement Committers 1 - Ilyas Shabi PR-URL: https://github.com/nodejs/node/pull/51713 Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Yagiz Nizipli Reviewed-By: Marco Ippolito Reviewed-By: Minwoo Jung Reviewed-By: Benjamin Gruenbaum Reviewed-By: Joyee Cheung ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/51713 Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Yagiz Nizipli Reviewed-By: Marco Ippolito Reviewed-By: Minwoo Jung Reviewed-By: Benjamin Gruenbaum Reviewed-By: Joyee Cheung -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - perf_hooks: performance milestone time origin timestamp improvement ℹ This PR was created on Fri, 09 Feb 2024 19:39:28 GMT ✔ Approvals: 6 ✔ - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/51713#pullrequestreview-1873159855 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/51713#pullrequestreview-1873194489 ✔ - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/51713#pullrequestreview-1873580594 ✔ - Minwoo Jung (@JungMinu): https://github.com/nodejs/node/pull/51713#pullrequestreview-1874371237 ✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/51713#pullrequestreview-1874747501 ✔ - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/51713#pullrequestreview-1889138330 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-02-27T08:38:20Z: https://ci.nodejs.org/job/node-test-pull-request/57449/ - Querying data for job/node-test-pull-request/57449/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/8063423832 |
|
Landed in f4af4b1 |
PR-URL: #51713 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #51713 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #51713 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #51713 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#51713 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
local benchmark: