-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
dns: improve performance #13261
dns: improve performance #13261
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
})(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,33 +60,29 @@ function errnoException(err, syscall, hostname) { | |
return ex; | ||
} | ||
|
||
|
||
// 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); | ||
} | ||
}; | ||
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any reason to not use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Faster to not have to call into C++ land IIRC, especially since we only need to check at most a few characters anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mscdex how much faster is this compared to something simpler like function digits(code) {
return code >= 48 && code <= 57;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lpinca ~19% faster. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. |
||
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/*'.'*/); | ||
} | ||
|
||
|
||
|
@@ -97,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); | ||
} | ||
|
||
|
||
|
@@ -153,23 +150,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 {}; | ||
} | ||
|
@@ -182,11 +178,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 +211,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 +220,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 +254,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 +262,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; | ||
}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to throw in
localhost
so that there's a hostname lookup since that probably takes a different path through the code?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose this list because they don't actually hit libuv and so (in this particular case) it is a better measure of the forced async callback change. Also when you start passing in non-empty hostnames, the
n
value will need to be lowered quite a bit.