Skip to content

Commit

Permalink
dns: improve callback performance
Browse files Browse the repository at this point in the history
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: nodejs#13261
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
mscdex committed Jun 2, 2017
1 parent 8d7ff6d commit 656bb71
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 43 deletions.
38 changes: 38 additions & 0 deletions benchmark/dns/lookup.js
Original file line number Diff line number Diff line change
@@ -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);
})();
}
}
49 changes: 6 additions & 43 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -153,23 +124,22 @@ 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 {};
}

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 {};
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
};
}
Expand Down

0 comments on commit 656bb71

Please sign in to comment.