From 3c8e59c3965d81ab5c0656423e987665a47ddcdf Mon Sep 17 00:00:00 2001 From: Nathan Woltman Date: Sun, 20 Dec 2015 02:01:34 -0500 Subject: [PATCH] lib: copy arguments object instead of leaking it Instead of leaking the arguments object by passing it as an argument to a function, copy it's contents to a new array, then pass the array. This allows V8 to optimize the function that contains this code, improving performance. PR-URL: https://github.com/nodejs/node/pull/4361 Reviewed-By: James M Snell Reviewed-By: Brian White --- lib/_http_client.js | 12 ++++++++++-- lib/_tls_wrap.js | 6 +++++- lib/assert.js | 22 ++++++++++++++-------- lib/console.js | 5 ++++- lib/net.js | 12 ++++++++++-- 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index 04740d5..cdf0726 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -569,10 +569,18 @@ ClientRequest.prototype.setTimeout = function(msecs, callback) { }; ClientRequest.prototype.setNoDelay = function() { - this._deferToConnect('setNoDelay', arguments); + const argsLen = arguments.length; + const args = new Array(argsLen); + for (var i = 0; i < argsLen; i++) + args[i] = arguments[i]; + this._deferToConnect('setNoDelay', args); }; ClientRequest.prototype.setSocketKeepAlive = function() { - this._deferToConnect('setKeepAlive', arguments); + const argsLen = arguments.length; + const args = new Array(argsLen); + for (var i = 0; i < argsLen; i++) + args[i] = arguments[i]; + this._deferToConnect('setKeepAlive', args); }; ClientRequest.prototype.clearTimeout = function(cb) { diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 39d9198..e340e4a 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -944,7 +944,11 @@ function normalizeConnectArgs(listArgs) { } exports.connect = function(/* [port, host], options, cb */) { - var args = normalizeConnectArgs(arguments); + const argsLen = arguments.length; + var args = new Array(argsLen); + for (var i = 0; i < argsLen; i++) + args[i] = arguments[i]; + args = normalizeConnectArgs(args); var options = args[0]; var cb = args[1]; diff --git a/lib/assert.js b/lib/assert.js index 9284205..b33dbfa 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -290,6 +290,16 @@ function expectedException(actual, expected) { return expected.call({}, actual) === true; } +function _tryBlock(block) { + var error; + try { + block(); + } catch (e) { + error = e; + } + return error; +} + function _throws(shouldThrow, block, expected, message) { var actual; @@ -302,11 +312,7 @@ function _throws(shouldThrow, block, expected, message) { expected = null; } - try { - block(); - } catch (e) { - actual = e; - } + actual = _tryBlock(block); message = (expected && expected.name ? ' (' + expected.name + ').' : '.') + (message ? ' ' + message : '.'); @@ -329,12 +335,12 @@ function _throws(shouldThrow, block, expected, message) { // assert.throws(block, Error_opt, message_opt); assert.throws = function(block, /*optional*/error, /*optional*/message) { - _throws.apply(this, [true].concat(pSlice.call(arguments))); + _throws(true, block, error, message); }; // EXTENSION! This is annoying to write outside this module. -assert.doesNotThrow = function(block, /*optional*/message) { - _throws.apply(this, [false].concat(pSlice.call(arguments))); +assert.doesNotThrow = function(block, /*optional*/error, /*optional*/message) { + _throws(false, block, error, message); }; assert.ifError = function(err) { if (err) throw err; }; diff --git a/lib/console.js b/lib/console.js index 5628c93..cc8e68c 100644 --- a/lib/console.js +++ b/lib/console.js @@ -83,7 +83,10 @@ Console.prototype.trace = function trace() { Console.prototype.assert = function(expression) { if (!expression) { - var arr = Array.prototype.slice.call(arguments, 1); + const argsLen = arguments.length || 1; + const arr = new Array(argsLen - 1); + for (var i = 1; i < argsLen; i++) + arr[i - 1] = arguments[i]; require('assert').ok(false, util.format.apply(null, arr)); } }; diff --git a/lib/net.js b/lib/net.js index 4ecc4e2..1a131e5 100644 --- a/lib/net.js +++ b/lib/net.js @@ -59,7 +59,11 @@ exports.createServer = function(options, connectionListener) { // connect(path, [cb]); // exports.connect = exports.createConnection = function() { - var args = normalizeConnectArgs(arguments); + const argsLen = arguments.length; + var args = new Array(argsLen); + for (var i = 0; i < argsLen; i++) + args[i] = arguments[i]; + args = normalizeConnectArgs(args); debug('createConnection', args); var s = new Socket(args[0]); return Socket.prototype.connect.apply(s, args); @@ -858,7 +862,11 @@ Socket.prototype.connect = function(options, cb) { // Old API: // connect(port, [host], [cb]) // connect(path, [cb]); - var args = normalizeConnectArgs(arguments); + const argsLen = arguments.length; + var args = new Array(argsLen); + for (var i = 0; i < argsLen; i++) + args[i] = arguments[i]; + args = normalizeConnectArgs(args); return Socket.prototype.connect.apply(this, args); } -- 2.7.4