perf_hooks: implement performance.now() with fast API calls by joyeecheung · Pull Request #50492 · nodejs/node

joyeecheung · 2023-10-31T16:40:17Z

Local benchmark numbers:

                            confidence improvement accuracy (*)   (**)  (***)
perf_hooks/now.js n=1000000        ***      7.04 %       ±1.38% ±1.84% ±2.40%

nodejs-github-bot · 2023-10-31T17:20:28Z

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

nodejs-github-bot · 2023-10-31T22:46:16Z

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

devsnek · 2023-11-01T03:44:57Z

I haven't kept up with fast calls lately but originally the double return type was only supported on some architectures. Do we know that this is actually optimizing the call everywhere?

joyeecheung · 2023-11-01T09:04:56Z

Haven’t really looked into if it’s always optimized but I think fast APIs are not guaranteed to always be hit anyway, that’s why the slow fallback callbacks are required.

src/node_perf.cc

devsnek · 2023-11-01T18:17:24Z

@joyeecheung i think my only concern would be whether this was, in practice, a regression on any of our supported platforms. maybe a benchmark run could double check?

joyeecheung · 2023-11-03T09:40:54Z

We only have benchmark CI on Ubuntu x64 but here is a CI run https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1469/

i think my only concern would be whether this was, in practice, a regression on any of our supported platforms. maybe a benchmark run could double check?

JSCallReducer should just skip generating fast calls for signatures that are not supported on the running platform, that's why the slow callback is required. If a regression can be caused by running a fast call on a platform it cannot optimize, that sounds more like an internal bug of V8...

joyeecheung · 2023-11-03T12:12:03Z

From benchmark CI:

10:04:57                             confidence improvement accuracy (*)    (**)   (***)
10:04:57 perf_hooks/now.js n=1000000         **     12.90 %       ±8.03% ±10.75% ±14.13%

nodejs-github-bot · 2023-11-03T21:47:51Z

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

nodejs-github-bot · 2023-11-04T19:16:40Z

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

nodejs-github-bot · 2023-11-06T09:27:28Z

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

nodejs-github-bot · 2023-11-07T08:57:13Z

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

nodejs-github-bot · 2023-11-14T21:52:24Z

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

nodejs-github-bot · 2023-11-15T14:17:57Z

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

nodejs-github-bot · 2023-11-15T18:34:21Z

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

nodejs-github-bot · 2023-11-17T20:57:46Z

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

nodejs-github-bot · 2023-11-17T23:59:29Z

Landed in 8c7fe47

PR-URL: #50492 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

PR-URL: nodejs#50492 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

PR-URL: #50492 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

perf_hooks: implement performance.now() with fast API calls

b948e83

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 Oct 31, 2023

H4ad approved these changes Oct 31, 2023

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2023

H4ad added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 31, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2023

anonrig approved these changes Oct 31, 2023

View reviewed changes

github-actions bot mentioned this pull request Nov 1, 2023

CI Reliability 2023-11-01 nodejs/reliability#704

Open

24 tasks

bengl approved these changes Nov 1, 2023

View reviewed changes

legendecas reviewed Nov 1, 2023

View reviewed changes

src/node_perf.cc Show resolved Hide resolved

This was referenced Nov 2, 2023

CI Reliability 2023-11-02 nodejs/reliability#705

Open

CI Reliability 2023-11-03 nodejs/reliability#706

Open

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 3, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 3, 2023

H4ad approved these changes Nov 3, 2023

View reviewed changes

github-actions bot mentioned this pull request Nov 4, 2023

CI Reliability 2023-11-04 nodejs/reliability#707

Open

14 tasks

legendecas approved these changes Nov 4, 2023

View reviewed changes

benjamingr approved these changes Nov 4, 2023

View reviewed changes

This was referenced Nov 5, 2023

CI Reliability 2023-11-05 nodejs/reliability#708

Open

CI Reliability 2023-11-06 nodejs/reliability#709

Open

github-actions bot mentioned this pull request Nov 7, 2023

CI Reliability 2023-11-07 nodejs/reliability#710

Open

16 tasks

This was referenced Nov 8, 2023

CI Reliability 2023-11-08 nodejs/reliability#711

Open

CI Reliability 2023-11-09 nodejs/reliability#712

Open

CI Reliability 2023-11-10 nodejs/reliability#713

Open

CI Reliability 2023-11-11 nodejs/reliability#714

Open

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2023

github-actions bot mentioned this pull request Nov 15, 2023

CI Reliability 2023-11-15 nodejs/reliability#718

Open

26 tasks

Qard approved these changes Nov 17, 2023

View reviewed changes

H4ad added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 17, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 17, 2023

nodejs-github-bot merged commit 8c7fe47 into nodejs:main Nov 17, 2023

RafaelGSS mentioned this pull request Nov 28, 2023

v21.3.0 proposal #50954

Merged

UlisesGascon mentioned this pull request Dec 12, 2023

v20.11.0 proposal #51124

Merged

Uh oh!

Conversation

joyeecheung commented Oct 31, 2023

Uh oh!

nodejs-github-bot commented Oct 31, 2023

Uh oh!

nodejs-github-bot commented Oct 31, 2023

Uh oh!

devsnek commented Nov 1, 2023

Uh oh!

joyeecheung commented Nov 1, 2023

Uh oh!

Uh oh!

devsnek commented Nov 1, 2023

Uh oh!

joyeecheung commented Nov 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented Nov 3, 2023

Uh oh!

nodejs-github-bot commented Nov 3, 2023

Uh oh!

nodejs-github-bot commented Nov 4, 2023

Uh oh!

nodejs-github-bot commented Nov 6, 2023

Uh oh!

nodejs-github-bot commented Nov 7, 2023

Uh oh!

nodejs-github-bot commented Nov 14, 2023

Uh oh!

nodejs-github-bot commented Nov 15, 2023

Uh oh!

nodejs-github-bot commented Nov 15, 2023

Uh oh!

nodejs-github-bot commented Nov 17, 2023

Uh oh!

nodejs-github-bot commented Nov 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

joyeecheung commented Nov 3, 2023 •

edited

Loading