From f7c1aed2314b39ea830905b3f7df588c9bf2397b Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 16 Aug 2018 14:31:04 +0200 Subject: [PATCH 1/2] os: improve networkInterfaces performance This algorithm uses less data transformations and is therefore significantly faster than the one before. --- lib/internal/os.js | 41 --------------------- lib/os.js | 60 +++++++++++++++++++++++++------ node.gyp | 1 - test/parallel/test-internal-os.js | 32 ----------------- test/parallel/test-os.js | 4 +-- 5 files changed, 52 insertions(+), 86 deletions(-) delete mode 100644 lib/internal/os.js delete mode 100644 test/parallel/test-internal-os.js diff --git a/lib/internal/os.js b/lib/internal/os.js deleted file mode 100644 index 74ed6e767ee16d..00000000000000 --- a/lib/internal/os.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; - -function getCIDRSuffix(mask, protocol = 'ipv4') { - const isV6 = protocol === 'ipv6'; - const bitsString = mask - .split(isV6 ? ':' : '.') - .filter((v) => !!v) - .map((v) => pad(parseInt(v, isV6 ? 16 : 10).toString(2), isV6)) - .join(''); - - if (isValidMask(bitsString)) { - return countOnes(bitsString); - } else { - return null; - } -} - -function pad(binaryString, isV6) { - const groupLength = isV6 ? 16 : 8; - const binLen = binaryString.length; - - return binLen < groupLength ? - `${'0'.repeat(groupLength - binLen)}${binaryString}` : binaryString; -} - -function isValidMask(bitsString) { - const firstIndexOfZero = bitsString.indexOf(0); - const lastIndexOfOne = bitsString.lastIndexOf(1); - - return firstIndexOfZero < 0 || firstIndexOfZero > lastIndexOfOne; -} - -function countOnes(bitsString) { - return bitsString - .split('') - .reduce((acc, bit) => acc += parseInt(bit, 10), 0); -} - -module.exports = { - getCIDRSuffix -}; diff --git a/lib/os.js b/lib/os.js index 08daa182e8b014..9da2cb58a97a52 100644 --- a/lib/os.js +++ b/lib/os.js @@ -24,7 +24,6 @@ const { pushValToArrayMax, safeGetenv } = process.binding('util'); const constants = process.binding('constants').os; const { deprecate } = require('internal/util'); -const { getCIDRSuffix } = require('internal/os'); const isWindows = process.platform === 'win32'; const { ERR_SYSTEM_ERROR } = require('internal/errors'); @@ -144,19 +143,60 @@ function endianness() { } endianness[Symbol.toPrimitive] = () => kEndianness; +function countOnes(n) { + let count = 0; + while (n !== 0) { + n = n & (n - 1); + count++; + } + return count; +} + +function getCIDR({ address, netmask, family }) { + let ones = 0; + let split = '.'; + let range = 10; + let groupLength = 8; + let hasZeros = false; + + if (family === 'IPv6') { + split = ':'; + range = 16; + groupLength = 16; + } + + const parts = netmask.split(split); + for (const part of parts) { + if (part !== '') { + const binary = parseInt(part, range); + const tmp = countOnes(binary); + ones += tmp; + if (hasZeros) { + if (tmp !== 0) { + return null; + } + } else if (tmp !== groupLength) { + if (binary % 2 !== 0) { + return null; + } + hasZeros = true; + } + } + } + + return `${address}/${ones}`; +} + function networkInterfaces() { const interfaceAddresses = getInterfaceAddresses(); - return Object.entries(interfaceAddresses).reduce((acc, [key, val]) => { - acc[key] = val.map((v) => { - const protocol = v.family.toLowerCase(); - const suffix = getCIDRSuffix(v.netmask, protocol); - const cidr = suffix ? `${v.address}/${suffix}` : null; + for (const val of Object.values(interfaceAddresses)) { + for (const v of val) { + v.cidr = getCIDR(v); + } + } - return Object.assign({}, v, { cidr }); - }); - return acc; - }, {}); + return interfaceAddresses; } module.exports = { diff --git a/node.gyp b/node.gyp index 8c42474bbed90a..a33dde4272e994 100644 --- a/node.gyp +++ b/node.gyp @@ -132,7 +132,6 @@ 'lib/internal/modules/esm/translators.js', 'lib/internal/safe_globals.js', 'lib/internal/net.js', - 'lib/internal/os.js', 'lib/internal/priority_queue.js', 'lib/internal/process/esm_loader.js', 'lib/internal/process/main_thread_only.js', diff --git a/test/parallel/test-internal-os.js b/test/parallel/test-internal-os.js deleted file mode 100644 index c4014abc5b1c0f..00000000000000 --- a/test/parallel/test-internal-os.js +++ /dev/null @@ -1,32 +0,0 @@ -// Flags: --expose-internals -'use strict'; - -require('../common'); -const assert = require('assert'); -const getCIDRSuffix = require('internal/os').getCIDRSuffix; - -const specs = [ - // valid - ['128.0.0.0', 'ipv4', 1], - ['255.0.0.0', 'ipv4', 8], - ['255.255.255.128', 'ipv4', 25], - ['255.255.255.255', 'ipv4', 32], - ['ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff', 'ipv6', 128], - ['ffff:ffff:ffff:ffff::', 'ipv6', 64], - ['ffff:ffff:ffff:ff80::', 'ipv6', 57], - // invalid - ['255.0.0.1', 'ipv4', null], - ['255.255.9.0', 'ipv4', null], - ['255.255.1.0', 'ipv4', null], - ['ffff:ffff:43::', 'ipv6', null], - ['ffff:ffff:ffff:1::', 'ipv6', null] -]; - -specs.forEach(([mask, protocol, expectedSuffix]) => { - const actualSuffix = getCIDRSuffix(mask, protocol); - - assert.strictEqual( - actualSuffix, expectedSuffix, - `Mask: ${mask}, expected: ${expectedSuffix}, actual: ${actualSuffix}` - ); -}); diff --git a/test/parallel/test-os.js b/test/parallel/test-os.js index 62a50e6fe706a7..3591ee52104ea4 100644 --- a/test/parallel/test-os.js +++ b/test/parallel/test-os.js @@ -130,8 +130,8 @@ switch (platform) { const expected = [{ address: '127.0.0.1', netmask: '255.0.0.0', - mac: '00:00:00:00:00:00', family: 'IPv4', + mac: '00:00:00:00:00:00', internal: true, cidr: '127.0.0.1/8' }]; @@ -146,8 +146,8 @@ switch (platform) { const expected = [{ address: '127.0.0.1', netmask: '255.0.0.0', - mac: '00:00:00:00:00:00', family: 'IPv4', + mac: '00:00:00:00:00:00', internal: true, cidr: '127.0.0.1/8' }]; From 17f49301a4ed86b6dac48eef49703b17e88cc9bf Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 19 Aug 2018 17:34:28 +0200 Subject: [PATCH 2/2] fixup: address comments --- lib/os.js | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/os.js b/lib/os.js index 9da2cb58a97a52..e65c11fc58ca7f 100644 --- a/lib/os.js +++ b/lib/os.js @@ -143,8 +143,13 @@ function endianness() { } endianness[Symbol.toPrimitive] = () => kEndianness; -function countOnes(n) { +// Returns the number of ones in the binary representation of the decimal +// number. +function countBinaryOnes(n) { let count = 0; + // Remove one "1" bit from n until n is the power of 2. This iterates k times + // while k is the number of "1" in the binary representation. + // For more check https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_Operators while (n !== 0) { n = n & (n - 1); count++; @@ -166,17 +171,17 @@ function getCIDR({ address, netmask, family }) { } const parts = netmask.split(split); - for (const part of parts) { - if (part !== '') { - const binary = parseInt(part, range); - const tmp = countOnes(binary); + for (var i = 0; i < parts.length; i++) { + if (parts[i] !== '') { + const binary = parseInt(parts[i], range); + const tmp = countBinaryOnes(binary); ones += tmp; if (hasZeros) { if (tmp !== 0) { return null; } } else if (tmp !== groupLength) { - if (binary % 2 !== 0) { + if ((binary & 1) !== 0) { return null; } hasZeros = true; @@ -190,9 +195,11 @@ function getCIDR({ address, netmask, family }) { function networkInterfaces() { const interfaceAddresses = getInterfaceAddresses(); - for (const val of Object.values(interfaceAddresses)) { - for (const v of val) { - v.cidr = getCIDR(v); + const keys = Object.keys(interfaceAddresses); + for (var i = 0; i < keys.length; i++) { + const arr = interfaceAddresses[keys[i]]; + for (var j = 0; j < arr.length; j++) { + arr[j].cidr = getCIDR(arr[j]); } }