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

net: Improve performance of isIPv4 and isIPv6 #4691

Closed
wants to merge 1 commit into from
Closed

net: Improve performance of isIPv4 and isIPv6 #4691

wants to merge 1 commit into from

Conversation

Uzlopak
Copy link

@Uzlopak Uzlopak commented Sep 9, 2023

Corresponding PR for nodejs/node#49568

'use strict'

const { isIPv4: isIPv4builtIn } = require('net')
const benchmark = require('benchmark')
const suite = new benchmark.Suite()

const ipMax = '255.255.255.255'
const ipMin = '0.0.0.0'
const invalid = '0.0.0.0.0'

const v4Seg = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])';
const v4Str = `(?:${v4Seg}\\.){3}${v4Seg}`;
const IPv4Reg = new RegExp(`^${v4Str}$`);

const isIPv4R = IPv4Reg.test.bind(IPv4Reg)

suite.add('RE (bound)', function () {
  isIPv4R(ipMin)
  isIPv4R(ipMax)
  isIPv4R(invalid)
})

suite.add('RE wrapped', function () {
  IPv4Reg.test(ipMin)
  IPv4Reg.test(ipMax)
  IPv4Reg.test(invalid)
})

suite.add('builtIn', function () {
  isIPv4builtIn(ipMin)
  isIPv4builtIn(ipMax)
  isIPv4builtIn(invalid)
})

suite.on('cycle', function onCycle(event) {
  console.log(String(event.target))
})

suite.run({ async: false })
aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/proxy-addr$ bun run ./benchmark/is-ipv4.js 
RE (bound) x 194,995,252 ops/sec ±56.36% (25 runs sampled)
RE wrapped x 218,862,836 ops/sec ±54.86% (29 runs sampled)
builtIn x 26,586,761 ops/sec ±4.99% (26 runs sampled)

It has about 55% deviation. If I change minSamples to 100 then I get about

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/proxy-addr$ bun run ./benchmark/is-ipv4.js 
RE (bound) x 73,035,238 ops/sec ±8.13% (107 runs sampled)
RE wrapped x 74,912,245 ops/sec ±8.86% (107 runs sampled)
builtIn x 18,630,794 ops/sec ±2.12% (116 runs sampled)

So maybe some benchmarking from your side would help to verify the benchmark gains?

@Uzlopak
Copy link
Author

Uzlopak commented Sep 13, 2023

@Jarred-Sumner

The corresponding PR for node got merged.

@Uzlopak
Copy link
Author

Uzlopak commented Oct 16, 2023

@Jarred-Sumner
Senpai notice me.

@Uzlopak
Copy link
Author

Uzlopak commented Jan 22, 2024

Was superseeded by #8271

@Uzlopak Uzlopak closed this Jan 22, 2024
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.

1 participant