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

process.hrtime.bigint can use v8 Fast API BigInt support #122

Closed
H4ad opened this issue Oct 8, 2023 · 4 comments
Closed

process.hrtime.bigint can use v8 Fast API BigInt support #122

H4ad opened this issue Oct 8, 2023 · 4 comments

Comments

@H4ad
Copy link
Member

H4ad commented Oct 8, 2023

v8 Fast APIs have support for BigInt as arguments and return values: v8/v8@6584b66

This commit hasn't landed on the node yet but once it lands, the most obvious usage can be on process.hrtime.bigint.

@H4ad H4ad changed the title process.hrtime.bigint can use new v8 Fast API process.hrtime.bigint can use v8 Fast API BigInt support Oct 8, 2023
@anonrig
Copy link
Member

anonrig commented Oct 8, 2023

@H4ad We can always backport the commit: https://github.com/nodejs/node/blob/69fb55e6b97c6c48048441d4d3ceef90ce6f2d81/doc/contributing/maintaining/maintaining-V8.md#backporting-to-active-branches

@H4ad
Copy link
Member Author

H4ad commented Oct 9, 2023

I will do that, should I backport first and then create another PR for changing hrtime or I can do both in the same PR?

@anonrig
Copy link
Member

anonrig commented Oct 9, 2023

I will do that, should I backport first and then create another PR for changing hrtime or I can do both in the same PR?

You can do that in the same PR but make sure they are in different commits and the Pr only contains 2 commits

@H4ad
Copy link
Member Author

H4ad commented Oct 11, 2023

I did try using Fast API with BigInt support but I didn't see any major improvement:

                                                confidence improvement accuracy (*)   (**)  (***)
process/bench-hrtime.js type='bigint' n=1000000          *      2.86 %       ±2.69% ±3.58% ±4.66%
process/bench-hrtime.js type='diff' n=1000000                  -0.33 %       ±2.25% ±3.00% ±3.92%
process/bench-hrtime.js type='raw' n=1000000                    0.46 %       ±2.01% ±2.68% ±3.48%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 3 comparisons, you can thus
expect the following amount of false-positive results:
  0.15 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.03 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

The current implementation is already well optimized and I don't think it is worth changing it for no good perf improvement.

I will close this issue but if anyone here is interested in taking a look, H4ad/node@5971701

@H4ad H4ad closed this as completed Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants