node
node copied to clipboard
nodejs
perf_hooks: Implement performance.timeOrigin and performance.toJSON() with fast API calls
local benchmark:
confidence improvement accuracy (*) (**) (***)
perf_hooks/time-origin.js method='timeOrigin' n=1000000 *** 75.84 % ±1.07% ±1.44% ±1.91%
perf_hooks/time-origin.js method='toJSON' n=1000000 *** 27.41 % ±1.24% ±1.66% ±2.16%
Not blocking but just a reminder - it would be faster if we just pass env->time_origin_timestamp() the same way we pass env->time_origin() to JS land via a typed array, and don't do a native call at all, because this is conceptually a constant (I am a bit surprised that it's not done already since we've been doing this for env->time_origin(), though with a closer look it seems to be only used by performance.nodeTiming.
Thanks for the suggestion @joyeecheung ! I've changed the code to pass env->time_origin_timestamp() the same way we do with env->time_origin(). It's now faster. Here is the benchmarks to show the improvement.
confidence improvement accuracy (*) (**) (***)
perf_hooks/time-origin.js method='timeOrigin' n=1000000 *** 120.57 % ±2.46% ±3.30% ±4.35%
perf_hooks/time-origin.js method='toJSON' n=1000000 *** 32.10 % ±1.64% ±2.18% ±2.86%
I've updated the commit message and PR title since it's no more a Fast api implementation, could you please review it again 🙏
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 Shabihttps://github.com/nodejs/node/actions/runs/8063423832PR-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/.ncu