From a3a74559da071ea9a6a7ad5734f4d1cdc624d74c Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 26 May 2017 17:07:21 +0200 Subject: [PATCH] chore: improve multi performance by refactoring a array check away --- index.js | 3 ++- lib/multi.js | 37 +++++++++++++++++++++---------------- lib/utils.js | 11 ++--------- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/index.js b/index.js index 6002cb0763..adc165ea13 100644 --- a/index.js +++ b/index.js @@ -119,6 +119,7 @@ function RedisClient (options, stream) { this.timesConnected = 0 this.buffers = options.returnBuffers || options.detectBuffers this.options = options + this._multi = false this.reply = 'ON' // Returning replies is the default this.retryStrategy = options.retryStrategy || function (options) { if (options.attempt > 100) { @@ -619,7 +620,7 @@ RedisClient.prototype.returnError = function (err) { function normalReply (self, reply) { const commandObj = self.commandQueue.shift() - if (commandObj.command !== 'exec') { + if (self._multi === false) { reply = self.handleReply(reply, commandObj.command, commandObj.bufferArgs) } commandObj.callback(null, reply) diff --git a/lib/multi.js b/lib/multi.js index b7a1272f0b..9d894a5253 100644 --- a/lib/multi.js +++ b/lib/multi.js @@ -59,7 +59,7 @@ function multiCallback (multi, replies) { i++ } } - + multi._client._multi = false return replies } @@ -70,33 +70,36 @@ function multiCallback (multi, replies) { * @returns Promise */ function execTransaction (multi) { - if (multi.monitoring || multi._client.monitoring) { + const client = multi._client + const queue = multi._queue + if (multi.monitoring || client.monitoring) { const err = new RangeError( 'Using transaction with a client that is in monitor mode does not work due to faulty return values of Redis.' ) err.command = 'EXEC' err.code = 'EXECABORT' return new Promise((resolve, reject) => { - utils.replyInOrder(multi._client, (err, res) => { + utils.replyInOrder(client, (err, res) => { if (err) return reject(err) resolve(res) }, null, []) }) } - const len = multi._queue.length + const len = queue.length multi.errors = [] - multi._client.cork() + client.cork() + client._multi = true multi.wantsBuffers = new Array(len) // Silently ignore this error. We'll receive the error for the exec as well - const promises = [multi._client.internalSendCommand(new Command('multi', [])).catch(() => {})] + const promises = [client.internalSendCommand(new Command('multi', [])).catch(() => {})] // Drain queue, callback will catch 'QUEUED' or error for (var index = 0; index < len; index++) { // The commands may not be shifted off, since they are needed in the result handler - promises.push(pipelineTransactionCommand(multi, multi._queue.get(index), index).catch((e) => e)) + promises.push(pipelineTransactionCommand(multi, queue.get(index), index).catch((e) => e)) } - const main = multi._client.internalSendCommand(new Command('exec', [])) - multi._client.uncork() + const main = client.internalSendCommand(new Command('exec', [])) + client.uncork() return Promise.all(promises).then(() => main.then((replies) => multiCallback(multi, replies)).catch((err) => { err.errors = multi.errors return Promise.reject(err) @@ -110,25 +113,27 @@ function execTransaction (multi) { * @returns Promise */ function execBatch (multi) { - if (multi._queue.length === 0) { + const client = multi._client + const queue = multi._queue + if (queue.length === 0) { // TODO: return an error if not "ready" return new Promise((resolve) => { - utils.replyInOrder(multi._client, (e, res) => { + utils.replyInOrder(client, (e, res) => { resolve(res) }, null, []) }) } var error = false - multi._client.cork() + client.cork() const promises = [] - while (multi._queue.length) { - const commandObj = multi._queue.shift() - promises.push(multi._client.internalSendCommand(commandObj).catch((e) => { + while (queue.length) { + const command = queue.shift() + promises.push(client.internalSendCommand(command).catch((e) => { error = true return e })) } - multi._client.uncork() + client.uncork() return Promise.all(promises).then((res) => { if (error) { const err = new Error('bla failed') diff --git a/lib/utils.js b/lib/utils.js index 90d43d6742..7cc83b2cec 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -3,18 +3,11 @@ /** * @description Convert an array to an object * - * hgetall converts its replies to an object. If the reply is empty, null is returned. - * The reply might be a string or a buffer if this is called in a transaction (multi) - * because of the queuing response. - * - * reply.length can be undefined but `undefined > 0` === false. - * - * @param {any} reply + * @param {any[]} reply * @returns object */ function replyToObject (reply) { - // The reply might be a string or a buffer if this is called in a transaction (multi) - if (reply.length === 0 || !(reply instanceof Array)) { + if (reply.length === 0) { return null } const obj = {}