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

fix node issue 49960 #519

Merged
merged 4 commits into from
Sep 30, 2023
Merged

fix node issue 49960 #519

merged 4 commits into from
Sep 30, 2023

Conversation

lemire
Copy link
Member

@lemire lemire commented Sep 30, 2023

Fix an intermittent issue identified by @isaacs in nodejs/node#49960

I also add quiet to the find_package(ZURI) request since most people will not have ZURI installed on their machine.

Performance

I am using benchdata to assess the performance impact.

There is a tiny performance impact (Apple M2, LLVM 14) of about 1%. It is small enough to be ignored (likely to go away or flip in another direction if you change compiler or processor).

Main branch:

BasicBench_AdaURL_href              24232194 ns     24232103 ns           29 GHz=3.433 cycle/byte=9.31199 cycles/url=808.832 instructions/byte=39.6644 instructions/cycle=4.2595 instructions/ns=14.6229 instructions/url=3.44522k ns/url=235.605 speed=358.536M/s time/byte=2.78912ns time/url=242.26ns url/s=4.12779M/s
BasicBench_AdaURL_aggregator_href   16207592 ns     16206721 ns           43 GHz=3.38162 cycle/byte=6.31842 cycles/url=548.813 instructions/byte=28.1166 instructions/cycle=4.44994 instructions/ns=15.048 instructions/url=2.44219k ns/url=162.293 speed=536.08M/s time/byte=1.86539ns time/url=162.027ns url/s=6.17182M/s

This PR:

BasicBench_AdaURL_href              23978792 ns     23978033 ns           30 GHz=3.37652 cycle/byte=9.39654 cycles/url=816.176 instructions/byte=39.5148 instructions/cycle=4.20525 instructions/ns=14.1991 instructions/url=3.43223k ns/url=241.721 speed=362.335M/s time/byte=2.75987ns time/url=239.72ns url/s=4.17153M/s
BasicBench_AdaURL_aggregator_href   16425182 ns     16425233 ns           43 GHz=3.38103 cycle/byte=6.3707 cycles/url=553.354 instructions/byte=28.2459 instructions/cycle=4.43372 instructions/ns=14.9905 instructions/url=2.45342k ns/url=163.665 speed=528.948M/s time/byte=1.89055ns time/url=164.211ns url/s=6.08972M/s

For x64, I am using an Ice Lake server with GCC 12. The effect of this PR is beneficial.

Main branch:

BasicBench_AdaURL_href              38292389 ns     38241882 ns           16 GHz=3.18867 cycle/byte=13.8632 cycles/url=1.20414k instructions/byte=37.3435 instructions/cycle=2.69372 instructions/ns=8.5894 instructions/url=3.24363k ns/url=377.632 speed=227.188M/s time/byte=4.40164ns time/url=382.323ns url/s=2.61559M/s
BasicBench_AdaURL_aggregator_href   23061240 ns     23034526 ns           30 GHz=3.18847 cycle/byte=8.46244 cycles/url=735.041 instructions/byte=25.2089 instructions/cycle=2.97891 instructions/ns=9.49817 instructions/url=2.18962k ns/url=230.531 speed=377.177M/s time/byte=2.65128ns time/url=230.288ns url/s=4.34239M/s

This PR:

BasicBench_AdaURL_href              36641121 ns     36584387 ns           19 GHz=3.18896 cycle/byte=13.3703 cycles/url=1.16134k instructions/byte=36.9924 instructions/cycle=2.76675 instructions/ns=8.82305 instructions/url=3.21313k ns/url=364.174 speed=237.481M/s time/byte=4.21087ns time/url=365.752ns url/s=2.73409M/s
BasicBench_AdaURL_aggregator_href   22628733 ns     22600326 ns           31 GHz=3.18851 cycle/byte=8.28246 cycles/url=719.408 instructions/byte=24.5663 instructions/cycle=2.96606 instructions/ns=9.45733 instructions/url=2.13381k ns/url=225.625 speed=384.423M/s time/byte=2.6013ns time/url=225.947ns url/s=4.42582M/s

@lemire lemire merged commit a944dd2 into main Sep 30, 2023
31 checks passed
@lemire lemire deleted the node49960 branch September 30, 2023 01:28
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

Successfully merging this pull request may close these issues.

2 participants