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.
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.
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: