Skip to content

dns: improve lookup performance by ~3% - #44629

Closed
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:perf/dns
Closed

dns: improve lookup performance by ~3%#44629
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:perf/dns

Conversation

@anonrig

Copy link
Copy Markdown
Member

This change improves dns lookup performance by roughly 3%.

Referencing: #44627

 confidence improvement accuracy (*) (**) (***)
dns/lookup-promises.js n=5000000 all='false' name='::1' -0.03 % ±2.42% ±3.23% ±4.22%
dns/lookup-promises.js n=5000000 all='false' name='127.0.0.1' 0.01 % ±2.77% ±3.69% ±4.80%
dns/lookup-promises.js n=5000000 all='true' name='::1' 0.29 % ±1.67% ±2.23% ±2.90%
dns/lookup-promises.js n=5000000 all='true' name='127.0.0.1' 2.99 % ±5.73% ±7.70% ±10.16%
dns/lookup.js n=5000000 all='false' name='::1' 0.37 % ±0.86% ±1.14% ±1.49%
dns/lookup.js n=5000000 all='false' name='127.0.0.1' -0.21 % ±0.79% ±1.05% ±1.37%
dns/lookup.js n=5000000 all='true' name='::1' 0.51 % ±0.93% ±1.24% ±1.61%
dns/lookup.js n=5000000 all='true' name='127.0.0.1' -0.50 % ±0.89% ±1.18% ±1.55%
Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 8 comparisons, you can thus expect the following amount of false-positive results:
0.40 false positives, when considering a 5% risk acceptance (*, **, ***),
0.08 false positives, when considering a 1% risk acceptance (**, ***),
0.01 false positives, when considering a 0.1% risk acceptance (***)
`

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-botnodejs-github-bot added dns Issues and PRs related to the dns subsystem. needs-ci PRs that need a full CI run. labels Sep 13, 2022
@mscdex

mscdex commented Sep 13, 2022

Copy link
Copy Markdown
Contributor

I think you're misinterpreting the benchmark comparison output here. You should be looking for repeatable results with *** in the confidence column to ensure that this is indeed an improvement.

@Gordapanela

Copy link
Copy Markdown

Review requested:

@anonrig

Copy link
Copy Markdown
MemberAuthor

You're definitely right. I totally missed it. I'm closing the pull request until I find a better improvement. Thanks!

@anonriganonrig closed this Sep 13, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dnsIssues and PRs related to the dns subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@anonrig@nodejs-github-bot@mscdex@Gordapanela