From 8d7ff6dd7561c7c8012466542ecabec2a5bc106d Mon Sep 17 00:00:00 2001 From: Brian White Date: Sat, 27 May 2017 20:32:17 -0400 Subject: [PATCH 1/3] test: improve dns internet test case 0.0.0.0 is more common than other special ipv4 addresses, so it is possible that we may not get ENOTFOUND for such addresses. Instead, this commit uses a less common address that is reserved for documentation (RFC) use only. PR-URL: https://github.com/nodejs/node/pull/13261 Reviewed-By: Roman Reiss Reviewed-By: James M Snell --- test/internet/test-dns.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/internet/test-dns.js b/test/internet/test-dns.js index fc001ff96533fc..3096fda5b1ee75 100644 --- a/test/internet/test-dns.js +++ b/test/internet/test-dns.js @@ -492,11 +492,12 @@ TEST(function test_lookupservice_invalid(done) { TEST(function test_reverse_failure(done) { - const req = dns.reverse('0.0.0.0', function(err) { + // 203.0.113.0/24 are addresses reserved for (RFC) documentation use only + const req = dns.reverse('203.0.113.0', function(err) { assert(err instanceof Error); assert.strictEqual(err.code, 'ENOTFOUND'); // Silly error code... - assert.strictEqual(err.hostname, '0.0.0.0'); - assert.ok(/0\.0\.0\.0/.test(err.message)); + assert.strictEqual(err.hostname, '203.0.113.0'); + assert.ok(/203\.0\.113\.0/.test(err.message)); done(); }); From 656bb71867ffb7a0ce9edf5b60cb9303ec915576 Mon Sep 17 00:00:00 2001 From: Brian White Date: Sat, 27 May 2017 20:35:13 -0400 Subject: [PATCH 2/3] dns: improve callback performance It appears that either c-ares no longer calls callbacks synchronously or we have since explicitly taken care of the scenarios in which c-ares would call callbacks synchronously (e.g. resolving an IP address or an empty hostname). Therefore we no longer need to have machinery in place to handle possible synchronous callback invocation. This improves performance significantly. PR-URL: https://github.com/nodejs/node/pull/13261 Reviewed-By: Roman Reiss Reviewed-By: James M Snell --- benchmark/dns/lookup.js | 38 ++++++++++++++++++++++++++++++++ lib/dns.js | 49 +++++------------------------------------ 2 files changed, 44 insertions(+), 43 deletions(-) create mode 100644 benchmark/dns/lookup.js diff --git a/benchmark/dns/lookup.js b/benchmark/dns/lookup.js new file mode 100644 index 00000000000000..ebe9d05695ef23 --- /dev/null +++ b/benchmark/dns/lookup.js @@ -0,0 +1,38 @@ +'use strict'; + +const common = require('../common.js'); +const lookup = require('dns').lookup; + +const bench = common.createBenchmark(main, { + name: ['', '127.0.0.1', '::1'], + all: [true, false], + n: [5e6] +}); + +function main(conf) { + const name = conf.name; + const n = +conf.n; + const all = !!conf.all; + var i = 0; + + if (all) { + const opts = { all: true }; + bench.start(); + (function cb(err, results) { + if (i++ === n) { + bench.end(n); + return; + } + lookup(name, opts, cb); + })(); + } else { + bench.start(); + (function cb(err, result) { + if (i++ === n) { + bench.end(n); + return; + } + lookup(name, cb); + })(); + } +} diff --git a/lib/dns.js b/lib/dns.js index 5b3ac41ba95da6..812045bab80789 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -61,35 +61,6 @@ function errnoException(err, syscall, hostname) { } -// c-ares invokes a callback either synchronously or asynchronously, -// but the dns API should always invoke a callback asynchronously. -// -// This function makes sure that the callback is invoked asynchronously. -// It returns a function that invokes the callback within nextTick(). -// -// To avoid invoking unnecessary nextTick(), `immediately` property of -// returned function should be set to true after c-ares returned. -// -// Usage: -// -// function someAPI(callback) { -// callback = makeAsync(callback); -// channel.someAPI(..., callback); -// callback.immediately = true; -// } -function makeAsync(callback) { - return function asyncCallback(...args) { - if (asyncCallback.immediately) { - // The API already returned, we can invoke the callback immediately. - callback.apply(null, args); - } else { - args.unshift(callback); - process.nextTick.apply(null, args); - } - }; -} - - function onlookup(err, addresses) { if (err) { return this.callback(errnoException(err, 'getaddrinfo', this.hostname)); @@ -153,13 +124,11 @@ function lookup(hostname, options, callback) { if (family !== 0 && family !== 4 && family !== 6) throw new TypeError('Invalid argument: family must be 4 or 6'); - callback = makeAsync(callback); - if (!hostname) { if (all) { - callback(null, []); + process.nextTick(callback, null, []); } else { - callback(null, null, family === 6 ? 6 : 4); + process.nextTick(callback, null, null, family === 6 ? 6 : 4); } return {}; } @@ -167,9 +136,10 @@ function lookup(hostname, options, callback) { var matchedFamily = isIP(hostname); if (matchedFamily) { if (all) { - callback(null, [{address: hostname, family: matchedFamily}]); + process.nextTick( + callback, null, [{address: hostname, family: matchedFamily}]); } else { - callback(null, hostname, matchedFamily); + process.nextTick(callback, null, hostname, matchedFamily); } return {}; } @@ -182,11 +152,9 @@ function lookup(hostname, options, callback) { var err = cares.getaddrinfo(req, hostname, family, hints); if (err) { - callback(errnoException(err, 'getaddrinfo', hostname)); + process.nextTick(callback, errnoException(err, 'getaddrinfo', hostname)); return {}; } - - callback.immediately = true; return req; } @@ -217,7 +185,6 @@ function lookupService(host, port, callback) { throw new TypeError('"callback" argument must be a function'); port = +port; - callback = makeAsync(callback); var req = new GetNameInfoReqWrap(); req.callback = callback; @@ -227,8 +194,6 @@ function lookupService(host, port, callback) { var err = cares.getnameinfo(req, host, port); if (err) throw errnoException(err, 'getnameinfo', host); - - callback.immediately = true; return req; } @@ -263,7 +228,6 @@ function resolver(bindingName) { throw new Error('"callback" argument must be a function'); } - callback = makeAsync(callback); var req = new QueryReqWrap(); req.bindingName = bindingName; req.callback = callback; @@ -272,7 +236,6 @@ function resolver(bindingName) { req.ttl = !!(options && options.ttl); var err = binding(req, name); if (err) throw errnoException(err, bindingName); - callback.immediately = true; return req; }; } From 3c989de2074a1bc103be0dafb5b11c87bbba8351 Mon Sep 17 00:00:00 2001 From: Brian White Date: Sat, 27 May 2017 21:44:03 -0400 Subject: [PATCH 3/3] dns: use faster IP address type check on results PR-URL: https://github.com/nodejs/node/pull/13261 Reviewed-By: Roman Reiss Reviewed-By: James M Snell --- lib/dns.js | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 812045bab80789..6fcf37a85e6c12 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -60,6 +60,31 @@ function errnoException(err, syscall, hostname) { return ex; } +const digits = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0-15 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16-31 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 32-47 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48-63 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 64-79 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 80-95 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 96-111 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 // 112-127 +]; +function isIPv4(str) { + if (!digits[str.charCodeAt(0)]) return false; + if (str.length === 1) return false; + if (str.charCodeAt(1) === 46/*'.'*/) + return true; + else if (!digits[str.charCodeAt(1)]) + return false; + if (str.length === 2) return false; + if (str.charCodeAt(2) === 46/*'.'*/) + return true; + else if (!digits[str.charCodeAt(2)]) + return false; + return (str.length > 3 && str.charCodeAt(3) === 46/*'.'*/); +} + function onlookup(err, addresses) { if (err) { @@ -68,25 +93,26 @@ function onlookup(err, addresses) { if (this.family) { this.callback(null, addresses[0], this.family); } else { - this.callback(null, addresses[0], addresses[0].indexOf(':') >= 0 ? 6 : 4); + this.callback(null, addresses[0], isIPv4(addresses[0]) ? 4 : 6); } } function onlookupall(err, addresses) { - var results = []; if (err) { return this.callback(errnoException(err, 'getaddrinfo', this.hostname)); } + var family = this.family; for (var i = 0; i < addresses.length; i++) { - results.push({ - address: addresses[i], - family: this.family || (addresses[i].indexOf(':') >= 0 ? 6 : 4) - }); + const addr = addresses[i]; + addresses[i] = { + address: addr, + family: family || (isIPv4(addr) ? 4 : 6) + }; } - this.callback(null, results); + this.callback(null, addresses); }