From 7b4830378311fd31f3b79d66d77fe4ebc95ed317 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 11 Apr 2017 16:22:32 +0200 Subject: [PATCH 1/6] net: don't create unnecessary closure Pass arguments to fireErrorCallbacks() explicitly. Saves allocation an unnecessary closure context. PR-URL: https://github.com/nodejs/node/pull/12342 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/net.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/net.js b/lib/net.js index 41a8e24c5a3404..81b679dab3274f 100644 --- a/lib/net.js +++ b/lib/net.js @@ -498,7 +498,7 @@ Socket.prototype.destroySoon = function() { Socket.prototype._destroy = function(exception, cb) { debug('destroy'); - function fireErrorCallbacks(self) { + function fireErrorCallbacks(self, exception, cb) { if (cb) cb(exception); if (exception && !self._writableState.errorEmitted) { process.nextTick(emitErrorNT, self, exception); @@ -508,7 +508,7 @@ Socket.prototype._destroy = function(exception, cb) { if (this.destroyed) { debug('already destroyed, fire error callbacks'); - fireErrorCallbacks(this); + fireErrorCallbacks(this, exception, cb); return; } @@ -540,7 +540,7 @@ Socket.prototype._destroy = function(exception, cb) { // to make it re-entrance safe in case Socket.prototype.destroy() // is called within callbacks this.destroyed = true; - fireErrorCallbacks(this); + fireErrorCallbacks(this, exception, cb); if (this._server) { COUNTER_NET_SERVER_CONNECTION_CLOSE(this); From 117b83c2ddca0c9cd1834889bf322167900a3e53 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 11 Apr 2017 16:40:16 +0200 Subject: [PATCH 2/6] net: don't create unnecessary closure Don't call `Function#bind()` when a direct method call works just as well and is much cheaper. PR-URL: https://github.com/nodejs/node/pull/12342 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/net.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/net.js b/lib/net.js index 81b679dab3274f..47ae799cd1c574 100644 --- a/lib/net.js +++ b/lib/net.js @@ -858,26 +858,20 @@ function internalConnect( var err; if (localAddress || localPort) { - var bind; + debug('binding to localAddress: %s and localPort: %d (addressType: %d)', + localAddress, localPort, addressType); if (addressType === 4) { localAddress = localAddress || '0.0.0.0'; - bind = self._handle.bind; + err = self._handle.bind(localAddress, localPort); } else if (addressType === 6) { localAddress = localAddress || '::'; - bind = self._handle.bind6; + err = self._handle.bind6(localAddress, localPort); } else { self._destroy(new TypeError('Invalid addressType: ' + addressType)); return; } - debug('binding to localAddress: %s and localPort: %d', - localAddress, - localPort); - - bind = bind.bind(self._handle); - err = bind(localAddress, localPort); - if (err) { const ex = exceptionWithHostPort(err, 'bind', localAddress, localPort); self._destroy(ex); From 571882c5a45872ac67e4e29513c4c8f23af9f781 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 11 Apr 2017 16:48:33 +0200 Subject: [PATCH 3/6] net: remove unnecessary process.nextTick() Call internalConnect() directly when the target is an IP address. No delay is necessary because it defers any callbacks it makes. PR-URL: https://github.com/nodejs/node/pull/12342 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/net.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/net.js b/lib/net.js index 47ae799cd1c574..6e99448076ee2c 100644 --- a/lib/net.js +++ b/lib/net.js @@ -991,10 +991,7 @@ function lookupAndConnect(self, options) { // If host is an IP, skip performing a lookup var addressType = cares.isIP(host); if (addressType) { - process.nextTick(function() { - if (self.connecting) - internalConnect(self, host, port, addressType, localAddress, localPort); - }); + internalConnect(self, host, port, addressType, localAddress, localPort); return; } From 94334344618544155cb0d4feadfc19526f746e34 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 11 Apr 2017 17:31:54 +0200 Subject: [PATCH 4/6] net: don't concatenate strings in debug logging Not necessary, not a good idea. PR-URL: https://github.com/nodejs/node/pull/12342 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/net.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/net.js b/lib/net.js index 6e99448076ee2c..c17d0a39286a1e 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1007,7 +1007,7 @@ function lookupAndConnect(self, options) { dnsopts.hints = dns.ADDRCONFIG; } - debug('connect: find host ' + host); + debug('connect: find host', host); debug('connect: dns options', dnsopts); self._host = host; var lookup = options.lookup || dns.lookup; @@ -1183,7 +1183,7 @@ function createServerHandle(address, port, addressType, fd) { handle = createHandle(fd); } catch (e) { // Not a fd we can listen on. This will trigger an error. - debug('listen invalid fd=' + fd + ': ' + e.message); + debug('listen invalid fd=%d:', fd, e.message); return uv.UV_EINVAL; } handle.open(fd); @@ -1204,7 +1204,7 @@ function createServerHandle(address, port, addressType, fd) { } if (address || port || isTCP) { - debug('bind to ' + (address || 'anycast')); + debug('bind to', address || 'any'); if (!address) { // Try binding to ipv6 first err = handle.bind6('::', port); From bb06add4d78535f10d1dc8f0c8c3210fecb96c8a Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 11 Apr 2017 18:05:48 +0200 Subject: [PATCH 5/6] net: don't normalize twice in Socket#connect() Split up Socket#connect() so that we don't call normalizeArgs() twice when invoking net.connect() or net.createConnection(). PR-URL: https://github.com/nodejs/node/pull/12342 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/net.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/net.js b/lib/net.js index c17d0a39286a1e..5babf582ed572e 100644 --- a/lib/net.js +++ b/lib/net.js @@ -94,7 +94,7 @@ function connect() { socket.setTimeout(options.timeout); } - return Socket.prototype.connect.call(socket, options, cb); + return realConnect.call(socket, options, cb); } @@ -921,7 +921,11 @@ Socket.prototype.connect = function() { const normalized = normalizeArgs(args); const options = normalized[0]; const cb = normalized[1]; + return realConnect.call(this, options, cb); +}; + +function realConnect(options, cb) { if (this.write !== Socket.prototype.write) this.write = Socket.prototype.write; @@ -962,7 +966,7 @@ Socket.prototype.connect = function() { lookupAndConnect(this, options); } return this; -}; +} function lookupAndConnect(self, options) { From 5d06e5c30d7350ffe697056bc9a712f8152cecc1 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 11 Apr 2017 18:15:11 +0200 Subject: [PATCH 6/6] net: require 'dns' only once Avoid unnecessary calls to require('dns') by caching the result of the first one. PR-URL: https://github.com/nodejs/node/pull/12342 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/net.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/net.js b/lib/net.js index 5babf582ed572e..898f14e9ae27c0 100644 --- a/lib/net.js +++ b/lib/net.js @@ -40,8 +40,9 @@ const PipeConnectWrap = process.binding('pipe_wrap').PipeConnectWrap; const ShutdownWrap = process.binding('stream_wrap').ShutdownWrap; const WriteWrap = process.binding('stream_wrap').WriteWrap; - var cluster; +var dns; + const errnoException = util._errnoException; const exceptionWithHostPort = util._exceptionWithHostPort; const isLegalPort = internalNet.isLegalPort; @@ -970,7 +971,7 @@ function realConnect(options, cb) { function lookupAndConnect(self, options) { - const dns = require('dns'); + const dns = lazyDns(); var host = options.host || 'localhost'; var port = options.port; var localAddress = options.localAddress; @@ -1309,6 +1310,13 @@ function emitListeningNT(self) { } +function lazyDns() { + if (dns === undefined) + dns = require('dns'); + return dns; +} + + function listenInCluster(server, address, port, addressType, backlog, fd, exclusive) { exclusive = !!exclusive; @@ -1437,7 +1445,7 @@ Server.prototype.listen = function() { }; function lookupAndListen(self, port, address, backlog, exclusive) { - const dns = require('dns'); + const dns = lazyDns(); dns.lookup(address, function doListen(err, ip, addressType) { if (err) { self.emit('error', err);