From 007a033820845665aee87f148b219ce3a34d5ebb Mon Sep 17 00:00:00 2001
From: Brian White <mscdex@mscdex.net>
Date: Sat, 27 May 2017 20:35:13 -0400
Subject: [PATCH] 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 <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 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 49cd9c929415b8..6d6ad82c895927 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;
   };
 }