From 9a0016aa1ac9b887bb9062051185fa129dfc34ba Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 19:21:08 +0100 Subject: [PATCH 01/38] Complete rewrite of javascript redis parser, heavily optimised for performance. Almost 3x faster. --- lib/javascript.js | 406 ++++++++++++++++++++++++++++------------------ 1 file changed, 248 insertions(+), 158 deletions(-) diff --git a/lib/javascript.js b/lib/javascript.js index a110345..6f9c13b 100644 --- a/lib/javascript.js +++ b/lib/javascript.js @@ -1,172 +1,262 @@ 'use strict'; +/*jshint latedef: nofunc */ -function JavascriptReplyParser(options) { - this.name = 'javascript'; - this.buffer = new Buffer(0); - this.offset = 0; - this.bigStrSize = 0; - this.chunksSize = 0; - this.buffers = []; - this.type = 0; - this.protocolError = false; - this.offsetCache = 0; - // If returnBuffers is active, all return values are returned as buffers besides numbers and errors - if (options.return_buffers) { - this.handleReply = function (start, end) { - return this.buffer.slice(start, end); - }; - } else { - this.handleReply = function (start, end) { - return this.buffer.toString('utf-8', start, end); - }; - } - // If stringNumbers is activated the parser always returns numbers as string - // This is important for big numbers (number > Math.pow(2, 53)) as js numbers are 64bit floating point numbers with reduced precision - if (options.string_numbers) { - this.handleNumbers = function (start, end) { - return this.buffer.toString('ascii', start, end); - }; - } else { - this.handleNumbers = function (start, end) { - return +this.buffer.toString('ascii', start, end); - }; - } +/** + * Used for lengths only, faster perf on arrays / bulks + * @param parser + * @returns {*} + */ +function parseSimpleString(parser) { + var offset = parser.offset; + var length = parser.buffer.length; + var string = ''; + + while (offset < length) { + var c1 = parser.buffer[offset++]; + if (c1 === 13) { + var c2 = parser.buffer[offset++]; + if (c2 === 10) { + parser.offset = offset; + return string; + } + string += String.fromCharCode(c1) + String.fromCharCode(c2); + continue; + } + string += String.fromCharCode(c1); + } + return undefined; } -JavascriptReplyParser.prototype.parseResult = function (type) { - var start = 0, - end = 0, - packetHeader = 0, - reply; - - if (type === 36) { // $ - packetHeader = this.parseHeader(); - // Packets with a size of -1 are considered null - if (packetHeader === -1) { - return null; - } - end = this.offset + packetHeader; - start = this.offset; - if (end + 2 > this.buffer.length) { - this.buffers.push(this.offsetCache === 0 ? this.buffer : this.buffer.slice(this.offsetCache)); - this.chunksSize = this.buffers[0].length; - // Include the packetHeader delimiter - this.bigStrSize = packetHeader + 2; - throw new Error('Wait for more data.'); - } - // Set the offset to after the delimiter - this.offset = end + 2; - return this.handleReply(start, end); - } else if (type === 58) { // : - // Up to the delimiter - end = this.packetEndOffset(); - start = this.offset; - // Include the delimiter - this.offset = end + 2; - // Return the coerced numeric value - return this.handleNumbers(start, end); - } else if (type === 43) { // + - end = this.packetEndOffset(); - start = this.offset; - this.offset = end + 2; - return this.handleReply(start, end); - } else if (type === 42) { // * - packetHeader = this.parseHeader(); - if (packetHeader === -1) { - return null; - } - reply = []; - for (var i = 0; i < packetHeader; i++) { - if (this.offset >= this.buffer.length) { - throw new Error('Wait for more data.'); - } - reply.push(this.parseResult(this.buffer[this.offset++])); - } - return reply; - } else if (type === 45) { // - - end = this.packetEndOffset(); - start = this.offset; - this.offset = end + 2; - return new Error(this.buffer.toString('utf-8', start, end)); - } -}; +/** + * Returns a string or buffer of the provided offset start and + * end ranges. Checks `optionReturnBuffers`. + * @param parser + * @param start + * @param end + * @returns {*} + */ +function convertBufferRange(parser, start, end) { + // If returnBuffers is active, all return values are returned as buffers besides numbers and errors + if (parser.optionReturnBuffers === true) { + return parser.buffer.slice(start, end); + } -JavascriptReplyParser.prototype.execute = function (buffer) { - if (this.chunksSize !== 0) { - if (this.bigStrSize > this.chunksSize + buffer.length) { - this.buffers.push(buffer); - this.chunksSize += buffer.length; - return; - } - this.buffers.push(buffer); - this.buffer = Buffer.concat(this.buffers, this.chunksSize + buffer.length); - this.buffers = []; - this.bigStrSize = 0; - this.chunksSize = 0; - } else if (this.offset >= this.buffer.length) { - this.buffer = buffer; - } else { - this.buffer = Buffer.concat([this.buffer.slice(this.offset), buffer]); - } - this.offset = 0; - this.run(); -}; + return parser.buffer.toString('utf-8', start, end); +} -JavascriptReplyParser.prototype.tryParsing = function () { - try { - return this.parseResult(this.type); - } catch (err) { - // Catch the error (not enough data), rewind if it's an array, - // and wait for the next packet to appear - this.offset = this.offsetCache; - // Indicate that there's no protocol error by resetting the type too - this.type = undefined; - } -}; +/** + * Parse a '+' redis simple string response but forward the offsets + * onto convertBufferRange to generate a string. + * @param parser + * @returns {*} + */ +function parseSimpleStringViaOffset(parser) { + var start = parser.offset; + var offset = parser.offset; + var length = parser.buffer.length; -JavascriptReplyParser.prototype.run = function () { - // Set a rewind point. If a failure occurs, wait for the next execute()/append() and try again - this.offsetCache = this.offset; - this.type = this.buffer[this.offset++]; - var reply = this.tryParsing(); - - while (reply !== undefined) { - if (this.type === 45) { // Errors - - this.returnError(reply); - } else { - this.returnReply(reply); // Strings + // Integers : // Bulk strings $ // Arrays * - } - this.offsetCache = this.offset; - this.type = this.buffer[this.offset++]; - reply = this.tryParsing(); - } - if (this.type !== undefined) { - // Reset the buffer so the parser can handle following commands properly - this.buffer = new Buffer(0); - this.returnFatalError(new Error('Protocol error, got ' + JSON.stringify(String.fromCharCode(this.type)) + ' as reply type byte')); - } -}; + while (offset < length) { + var c1 = parser.buffer[offset++]; + if (c1 === 13) { // \r + var c2 = parser.buffer[offset++]; + if (c2 === 10) { // \n + parser.offset = offset; + return convertBufferRange(parser, start, offset - 2); + } + } + } + return undefined; +} -JavascriptReplyParser.prototype.parseHeader = function () { - var end = this.packetEndOffset(), - value = this.buffer.toString('ascii', this.offset, end) | 0; +/** + * Returns the string length via parseSimpleString + * @param parser + * @returns {*} + */ +function parseLength(parser) { + var string = parseSimpleString(parser); + if (string !== undefined) { + var length = +string; + if (length === -1) { + return null; + } + return length; + } +} - this.offset = end + 2; - return value; -}; +/** + * Parse a ':' redis integer response + * @param parser + * @returns {*} + */ +function parseInteger(parser) { + var string = parseSimpleStringViaOffset(parser); + if (string !== undefined) { + // If stringNumbers is activated the parser always returns numbers as string + // This is important for big numbers (number > Math.pow(2, 53)) as js numbers + // are 64bit floating point numbers with reduced precision + if (parser.optionStringNumbers === false) { + return +string; + } + return string; + } +} + +/** + * Parse a '$' redis bulk string response + * @param parser + * @returns {null} + */ +function parseBulkString(parser) { + var length = parseLength(parser); + /* jshint eqnull: true */ + if (length == null) { + return length; + } + var offsetEnd = parser.offset + length; + if ((offsetEnd + 2) > parser.buffer.length) { + return; + } + + var offsetBegin = parser.offset; + parser.offset = offsetEnd + 2; + + return convertBufferRange(parser, offsetBegin, offsetEnd); +} + +/** + * Parse a '-' redis error response + * @param parser + * @returns {Error} + */ +function parseError(parser) { + var string = parseSimpleStringViaOffset(parser); + if (string !== undefined) { + return new Error(string); + } +} + +/** + * Parsing error handler, resets parser buffer + * @param parser + * @param error + */ +function handleError(parser, error) { + parser.buffer = null; + parser.returnFatalError(error); +} + +/** + * Parse a + * @param parser + * @returns {*} + */ +function parseArray(parser) { + var length = parseLength(parser); + /* jshint eqnull: true */ + if (length == null) { // will break if using === + return length; + } + + var responses = new Array(length); + var bufferLength = parser.buffer.length; + for (var i = 0; i < length; i++) { + if (parser.offset >= bufferLength) { + return; + } + var response = parseType(parser, parser.buffer[parser.offset++]); + if (response === undefined) { + return; + } + responses[i] = response; + } + + return responses; +} + +/** + * Called the appropriate parser for the specified type. + * @param parser + * @param type + * @returns {*} + */ +function parseType(parser, type) { + switch (type) { + case 36: // $ + return parseBulkString(parser); + case 58: // : + return parseInteger(parser); + case 43: // + + return parser.optionReturnBuffers ? parseSimpleStringViaOffset(parser) : parseSimpleString(parser); + case 42: // * + return parseArray(parser); + case 45: // - + return parseError(parser); + default: + return handleError(parser, new Error('Protocol error, got ' + JSON.stringify(String.fromCharCode(type)) + ' as reply type byte')); + } +} + +/** + * Quick buffer appending via buffer copy. + * @param parser + * @param buffer + */ +function appendToBuffer(parser, buffer) { + var oldLength = parser.buffer.length; + var remainingLength = oldLength - parser.offset; + var newLength = remainingLength + buffer.length; + var newBuffer = new Buffer(newLength); + parser.buffer.copy(newBuffer, 0, parser.offset, oldLength); + buffer.copy(newBuffer, remainingLength, 0, buffer.length); + parser.buffer = newBuffer; + parser.offset = 0; +} + +/** + * Javascript Redis Parser + * @param options + * @constructor + */ +function JavascriptRedisParser(options) { + this.optionReturnBuffers = !!options.return_buffers; + this.optionStringNumbers = !!options.string_numbers; + this.name = 'javascript'; + this.offset = 0; + this.buffer = null; +} + +/** + * Parse the redis buffer + * @param buffer + */ +JavascriptRedisParser.prototype.execute = function (buffer) { + if (this.buffer === null) { + this.buffer = buffer; + this.offset = 0; + } else { + appendToBuffer(this, buffer); + } + + var length = this.buffer.length; -JavascriptReplyParser.prototype.packetEndOffset = function () { - var offset = this.offset, - len = this.buffer.length - 1; + while (this.offset < length) { + var offset = this.offset; + var type = this.buffer[this.offset++]; + var response = parseType(this, type); + if (response === undefined) { + this.offset = offset; + return; + } - while (this.buffer[offset] !== 0x0d && this.buffer[offset + 1] !== 0x0a) { - offset++; + if (type === 45) { + this.returnError(response); // Errors - + } else { + this.returnReply(response); // Strings + // Integers : // Bulk strings $ // Arrays * + } + } - if (offset >= len) { - throw new Error('Did not see LF after NL reading multi bulk count (' + offset + ' => ' + this.buffer.length + ', ' + this.offset + ')'); - } - } - return offset; + this.buffer = null; }; -module.exports = JavascriptReplyParser; +module.exports = JavascriptRedisParser; From e855650e7f965f35f4d1f265ed31349e06abdbef Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 19:23:14 +0100 Subject: [PATCH 02/38] Benchmark dev deps --- package.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/package.json b/package.json index 26babb8..ffdee7e 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "main": "index.js", "scripts": { "test": "mocha", + "benchmark": "node ./benchmark", "posttest": "jshint . && npm run coverage && npm run coverage:check", "coverage": "node ./node_modules/istanbul/lib/cli.js cover --preserve-comments ./node_modules/mocha/bin/_mocha -- -R spec", "coverage:check": "node ./node_modules/istanbul/lib/cli.js check-coverage --branch 100 --statement 100" @@ -28,10 +29,12 @@ "node": ">=0.10.0" }, "devDependencies": { + "benchmark": "^2.1.0", "codeclimate-test-reporter": "^0.1.1", "intercept-stdout": "^0.1.2", "istanbul": "^0.4.0", "jshint": "^2.8.0", + "microtime": "^2.1.1", "mocha": "^2.3.2" }, "optionalDependency": { From f6da450af71feaf50406ad23c09e4a22b502865f Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 19:31:08 +0100 Subject: [PATCH 03/38] small rewrite of hiredis parser - around 3% perf boost, not much really... --- lib/hiredis.js | 58 +++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/lib/hiredis.js b/lib/hiredis.js index 965b660..75ce4ba 100644 --- a/lib/hiredis.js +++ b/lib/hiredis.js @@ -2,35 +2,45 @@ var hiredis = require('hiredis'); -function HiredisReplyParser(options) { - this.name = 'hiredis'; - this.options = options; - this.reader = new hiredis.Reader(options); +/** + * Parse data + * @param parser + * @returns {*} + */ +function parseData(parser) { + try { + return parser.reader.get(); + } catch (err) { + // Protocol errors land here + // Reset the parser. Otherwise new commands can't be processed properly + parser.reader = new hiredis.Reader(parser.options); + parser.returnFatalError(err); + } } -HiredisReplyParser.prototype.parseData = function () { - try { - return this.reader.get(); - } catch (err) { - // Protocol errors land here - // Reset the parser. Otherwise new commands can't be processed properly - this.reader = new hiredis.Reader(this.options); - this.returnFatalError(err); - } -}; +/** + * Hiredis Parser + * @param options + * @constructor + */ +function HiredisReplyParser(options) { + this.name = 'hiredis'; + this.options = options; + this.reader = new hiredis.Reader(options); +} HiredisReplyParser.prototype.execute = function (data) { - this.reader.feed(data); - var reply = this.parseData(); + this.reader.feed(data); + var reply = parseData(this); - while (reply !== undefined) { - if (reply && reply.name === 'Error') { - this.returnError(reply); - } else { - this.returnReply(reply); - } - reply = this.parseData(); - } + while (reply !== undefined) { + if (reply && reply.name === 'Error') { + this.returnError(reply); + } else { + this.returnReply(reply); + } + reply = parseData(this); + } }; module.exports = HiredisReplyParser; From be81ef61c58362b70e7e02aa8973abd8975fd3a5 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 19:43:32 +0100 Subject: [PATCH 04/38] npm ignore benchmarks --- .npmignore | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .npmignore diff --git a/.npmignore b/.npmignore new file mode 100644 index 0000000..1e83c64 --- /dev/null +++ b/.npmignore @@ -0,0 +1,10 @@ +# Created by .ignore support plugin (hsz.mobi) +### Example user template template +### Example user template + +# IntelliJ project files +.idea +*.iml +out +gen +benchmark \ No newline at end of file From bf077764f7fbdda4b7b57c8eab33045ec1f036cf Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 19:44:43 +0100 Subject: [PATCH 05/38] git ignore .idea --- .gitignore | 1 + .jshintignore | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 594729d..cd32cd1 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ logs *.log coverage node_modules +.idea diff --git a/.jshintignore b/.jshintignore index ed454a5..59a1197 100644 --- a/.jshintignore +++ b/.jshintignore @@ -1,4 +1,6 @@ node_modules/** coverage/** **.md -**.log \ No newline at end of file +**.log +.idea +benchmark \ No newline at end of file From a24e8c3065998e6573a1bcc029db176b93a48708 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 19:48:17 +0100 Subject: [PATCH 06/38] benchmarks --- benchmark/index.js | 138 +++++++++++++++++++++++++++++ benchmark/old/hiredis.js | 36 ++++++++ benchmark/old/javascript.js | 172 ++++++++++++++++++++++++++++++++++++ benchmark/old/parser.js | 59 +++++++++++++ package.json | 3 +- 5 files changed, 406 insertions(+), 2 deletions(-) create mode 100644 benchmark/index.js create mode 100644 benchmark/old/hiredis.js create mode 100644 benchmark/old/javascript.js create mode 100644 benchmark/old/parser.js diff --git a/benchmark/index.js b/benchmark/index.js new file mode 100644 index 0000000..6c32ec5 --- /dev/null +++ b/benchmark/index.js @@ -0,0 +1,138 @@ +var Benchmark = require('benchmark'); +var assert = require('assert'); +var suite = new Benchmark.Suite; + +var Parser = require('./../lib/parser'); +var ParserOLD = require('./old/parser'); + +function returnError(error) { + // throw error; silent for err error perf test +} + +function checkReply() { +} + +var startBuffer = new Buffer('$100\r\nabcdefghij'); +var chunkBuffer = new Buffer('abcdefghijabcdefghijabcdefghij'); +var stringBuffer = new Buffer('+testing a simple string\r\n'); +var integerBuffer = new Buffer(':1237884\r\n'); +var errorBuffer = new Buffer('-Error ohnoesitbroke\r\n'); +var arrayBuffer = new Buffer('*1\r\n*1\r\n$1\r\na\r\n'); +var endBuffer = new Buffer('\r\n'); + +var parserOld = new ParserOLD({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + name: 'javascript' +}); + +var parserHiRedis = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + name: 'hiredis' +}); + +var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + name: 'javascript' +}); + +// BULK STRINGS + +suite.add('OLD CODE: multiple chunks in a bulk string', function () { + parserOld.execute(startBuffer); + parserOld.execute(chunkBuffer); + parserOld.execute(chunkBuffer); + parserOld.execute(chunkBuffer); + parserOld.execute(endBuffer); +}); + +suite.add('HIREDIS: multiple chunks in a bulk string', function () { + parserHiRedis.execute(startBuffer); + parserHiRedis.execute(chunkBuffer); + parserHiRedis.execute(chunkBuffer); + parserHiRedis.execute(chunkBuffer); + parserHiRedis.execute(endBuffer); +}); + +suite.add('NEW CODE: multiple chunks in a bulk string', function () { + parser.execute(startBuffer); + parser.execute(chunkBuffer); + parser.execute(chunkBuffer); + parser.execute(chunkBuffer); + parser.execute(endBuffer); +}); + +// STRINGS + +suite.add('\nOLD CODE: + simple string', function () { + parserOld.execute(stringBuffer); +}); + +suite.add('HIREDIS: + simple string', function () { + parserHiRedis.execute(stringBuffer); +}); + +suite.add('NEW CODE: + simple string', function () { + parser.execute(stringBuffer); +}); + +// INTEGERS + +suite.add('\nOLD CODE: + integer', function () { + parserOld.execute(integerBuffer); +}); + +suite.add('HIREDIS: + integer', function () { + parserHiRedis.execute(integerBuffer); +}); + +suite.add('NEW CODE: + integer', function () { + parser.execute(integerBuffer); +}); + +// ARRAYS + +suite.add('\nOLD CODE: * array', function () { + parserOld.execute(arrayBuffer); +}); + +suite.add('HIREDIS: * array', function () { + parserHiRedis.execute(arrayBuffer); +}); + +suite.add('NEW CODE: * array', function () { + parser.execute(arrayBuffer); +}); + + +// ERRORS + +suite.add('\nOLD CODE: * error', function () { + parserOld.execute(errorBuffer); +}); + +suite.add('HIREDIS: * error', function () { + parserHiRedis.execute(errorBuffer); +}); + +suite.add('NEW CODE: * error', function () { + parser.execute(errorBuffer); +}); + + +// add listeners +suite.on('cycle', function (event) { + console.log(String(event.target)); +}); + +suite.on('complete', function () { + console.log('\n\nFastest is ' + this.filter('fastest').map('name')); +}); + + +suite.run({delay:2, minSamples: 100 }); \ No newline at end of file diff --git a/benchmark/old/hiredis.js b/benchmark/old/hiredis.js new file mode 100644 index 0000000..965b660 --- /dev/null +++ b/benchmark/old/hiredis.js @@ -0,0 +1,36 @@ +'use strict'; + +var hiredis = require('hiredis'); + +function HiredisReplyParser(options) { + this.name = 'hiredis'; + this.options = options; + this.reader = new hiredis.Reader(options); +} + +HiredisReplyParser.prototype.parseData = function () { + try { + return this.reader.get(); + } catch (err) { + // Protocol errors land here + // Reset the parser. Otherwise new commands can't be processed properly + this.reader = new hiredis.Reader(this.options); + this.returnFatalError(err); + } +}; + +HiredisReplyParser.prototype.execute = function (data) { + this.reader.feed(data); + var reply = this.parseData(); + + while (reply !== undefined) { + if (reply && reply.name === 'Error') { + this.returnError(reply); + } else { + this.returnReply(reply); + } + reply = this.parseData(); + } +}; + +module.exports = HiredisReplyParser; diff --git a/benchmark/old/javascript.js b/benchmark/old/javascript.js new file mode 100644 index 0000000..8302a5c --- /dev/null +++ b/benchmark/old/javascript.js @@ -0,0 +1,172 @@ +'use strict'; + +function JavascriptReplyParser(options) { + this.name = 'javascript_old'; + this.buffer = new Buffer(0); + this.offset = 0; + this.bigStrSize = 0; + this.chunksSize = 0; + this.buffers = []; + this.type = 0; + this.protocolError = false; + this.offsetCache = 0; + // If returnBuffers is active, all return values are returned as buffers besides numbers and errors + if (options.return_buffers) { + this.handleReply = function (start, end) { + return this.buffer.slice(start, end); + }; + } else { + this.handleReply = function (start, end) { + return this.buffer.toString('utf-8', start, end); + }; + } + // If stringNumbers is activated the parser always returns numbers as string + // This is important for big numbers (number > Math.pow(2, 53)) as js numbers are 64bit floating point numbers with reduced precision + if (options.string_numbers) { + this.handleNumbers = function (start, end) { + return this.buffer.toString('ascii', start, end); + }; + } else { + this.handleNumbers = function (start, end) { + return +this.buffer.toString('ascii', start, end); + }; + } +} + +JavascriptReplyParser.prototype.parseResult = function (type) { + var start = 0, + end = 0, + packetHeader = 0, + reply; + + if (type === 36) { // $ + packetHeader = this.parseHeader(); + // Packets with a size of -1 are considered null + if (packetHeader === -1) { + return null; + } + end = this.offset + packetHeader; + start = this.offset; + if (end + 2 > this.buffer.length) { + this.buffers.push(this.offsetCache === 0 ? this.buffer : this.buffer.slice(this.offsetCache)); + this.chunksSize = this.buffers[0].length; + // Include the packetHeader delimiter + this.bigStrSize = packetHeader + 2; + throw new Error('Wait for more data.'); + } + // Set the offset to after the delimiter + this.offset = end + 2; + return this.handleReply(start, end); + } else if (type === 58) { // : + // Up to the delimiter + end = this.packetEndOffset(); + start = this.offset; + // Include the delimiter + this.offset = end + 2; + // Return the coerced numeric value + return this.handleNumbers(start, end); + } else if (type === 43) { // + + end = this.packetEndOffset(); + start = this.offset; + this.offset = end + 2; + return this.handleReply(start, end); + } else if (type === 42) { // * + packetHeader = this.parseHeader(); + if (packetHeader === -1) { + return null; + } + reply = []; + for (var i = 0; i < packetHeader; i++) { + if (this.offset >= this.buffer.length) { + throw new Error('Wait for more data.'); + } + reply.push(this.parseResult(this.buffer[this.offset++])); + } + return reply; + } else if (type === 45) { // - + end = this.packetEndOffset(); + start = this.offset; + this.offset = end + 2; + return new Error(this.buffer.toString('utf-8', start, end)); + } +}; + +JavascriptReplyParser.prototype.execute = function (buffer) { + if (this.chunksSize !== 0) { + if (this.bigStrSize > this.chunksSize + buffer.length) { + this.buffers.push(buffer); + this.chunksSize += buffer.length; + return; + } + this.buffers.push(buffer); + this.buffer = Buffer.concat(this.buffers, this.chunksSize + buffer.length); + this.buffers = []; + this.bigStrSize = 0; + this.chunksSize = 0; + } else if (this.offset >= this.buffer.length) { + this.buffer = buffer; + } else { + this.buffer = Buffer.concat([this.buffer.slice(this.offset), buffer]); + } + this.offset = 0; + this.run(); +}; + +JavascriptReplyParser.prototype.tryParsing = function () { + try { + return this.parseResult(this.type); + } catch (err) { + // Catch the error (not enough data), rewind if it's an array, + // and wait for the next packet to appear + this.offset = this.offsetCache; + // Indicate that there's no protocol error by resetting the type too + this.type = undefined; + } +}; + +JavascriptReplyParser.prototype.run = function () { + // Set a rewind point. If a failure occurs, wait for the next execute()/append() and try again + this.offsetCache = this.offset; + this.type = this.buffer[this.offset++]; + var reply = this.tryParsing(); + + while (reply !== undefined) { + if (this.type === 45) { // Errors - + this.returnError(reply); + } else { + this.returnReply(reply); // Strings + // Integers : // Bulk strings $ // Arrays * + } + this.offsetCache = this.offset; + this.type = this.buffer[this.offset++]; + reply = this.tryParsing(); + } + if (this.type !== undefined) { + // Reset the buffer so the parser can handle following commands properly + this.buffer = new Buffer(0); + this.returnFatalError(new Error('Protocol error, got ' + JSON.stringify(String.fromCharCode(this.type)) + ' as reply type byte')); + } +}; + +JavascriptReplyParser.prototype.parseHeader = function () { + var end = this.packetEndOffset(), + value = this.buffer.toString('ascii', this.offset, end) | 0; + + this.offset = end + 2; + return value; +}; + +JavascriptReplyParser.prototype.packetEndOffset = function () { + var offset = this.offset, + len = this.buffer.length - 1; + + while (this.buffer[offset] !== 0x0d && this.buffer[offset + 1] !== 0x0a) { + offset++; + + if (offset >= len) { + throw new Error('Did not see LF after NL reading multi bulk count (' + offset + ' => ' + this.buffer.length + ', ' + this.offset + ')'); + } + } + return offset; +}; + +module.exports = JavascriptReplyParser; \ No newline at end of file diff --git a/benchmark/old/parser.js b/benchmark/old/parser.js new file mode 100644 index 0000000..f176ba6 --- /dev/null +++ b/benchmark/old/parser.js @@ -0,0 +1,59 @@ +'use strict'; + +var parsers = { + javascript: require('./javascript') +}; + +// Hiredis might not be installed +try { + parsers.hiredis = require('./hiredis'); +} catch (err) { /* ignore errors */ } + +function Parser (options) { + var parser, msg; + + if ( + !options || + typeof options.returnError !== 'function' || + typeof options.returnReply !== 'function' + ) { + throw new Error('Please provide all return functions while initiating the parser'); + } + + if (options.name === 'hiredis') { + /* istanbul ignore if: hiredis should always be installed while testing */ + if (!parsers.hiredis) { + msg = 'You explicitly required the hiredis parser but hiredis is not installed. The JS parser is going to be returned instead.'; + } else if (options.stringNumbers) { + msg = 'You explicitly required the hiredis parser in combination with the stringNumbers option. Only the JS parser can handle that and is choosen instead.'; + } + } else if (options.name && !parsers[options.name] && options.name !== 'auto') { + msg = 'The requested parser "' + options.name + '" is unkown and the default parser is choosen instead.'; + } + + if (msg) { + console.warn(new Error(msg).stack.replace('Error: ', 'Warning: ')); + } + + options.name = options.name || 'hiredis'; + options.name = options.name.toLowerCase(); + + var innerOptions = { + // The hiredis parser expects underscores + return_buffers: options.returnBuffers || false, + string_numbers: options.stringNumbers || false + }; + + if (options.name === 'javascript' || !parsers.hiredis || options.stringNumbers) { + parser = new parsers.javascript(innerOptions); + } else { + parser = new parsers.hiredis(innerOptions); + } + + parser.returnError = options.returnError; + parser.returnFatalError = options.returnFatalError || options.returnError; + parser.returnReply = options.returnReply; + return parser; +} + +module.exports = Parser; \ No newline at end of file diff --git a/package.json b/package.json index ffdee7e..13ec435 100644 --- a/package.json +++ b/package.json @@ -30,11 +30,10 @@ }, "devDependencies": { "benchmark": "^2.1.0", - "codeclimate-test-reporter": "^0.1.1", + "codeclimate-test-reporter": "^0.3.1", "intercept-stdout": "^0.1.2", "istanbul": "^0.4.0", "jshint": "^2.8.0", - "microtime": "^2.1.1", "mocha": "^2.3.2" }, "optionalDependency": { From 365b0d0a3cd0145619c37aab68b392be90f04743 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 20:07:49 +0100 Subject: [PATCH 07/38] fixed incorrect ioredis link --- README.md | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 6a5df8c..737256d 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ # redis-parser -A high performance redis parser solution built for [node_redis](https://github.com/NodeRedis/node_redis) and [ioredis](https://github.com/ioredis/luin). +A high performance redis parser solution built for [node_redis](https://github.com/NodeRedis/node_redis) and [ioredis](https://github.com/luin/ioredis). Generally all [RESP](http://redis.io/topics/protocol) data will be properly parsed by the parser. @@ -95,31 +95,22 @@ var parser = new Parser({ The [hiredis](https://github.com/redis/hiredis) parser is still the fasted parser for Node.js and therefor used as default in redis-parser if the hiredis parser is available. -Otherwise the pure js NodeRedis parser is choosen that is almost as fast as the +Otherwise the pure js NodeRedis parser is chosen that is almost as fast as the hiredis parser besides some situations in which it'll be a bit slower. ## Protocol errors -To handle protocol errors (this is very unlikely to happen) gracefuly you should add the returnFatalError option, reject any still running command (they might have been processed properly but the reply is just wrong), destroy the socket and reconnect. +To handle protocol errors (this is very unlikely to happen) gracefully you should add the returnFatalError option, reject any still running command (they might have been processed properly but the reply is just wrong), destroy the socket and reconnect. Otherwise a chunk might still contain partial data of a following command that was already processed properly but answered in the same chunk as the command that resulted in the protocol error. ## Contribute The js parser is already optimized but there are likely further optimizations possible. -Besides running the tests you'll also have to run the change at least against the node_redis benchmark suite and post the improvement in the PR. -If you want to write a own parser benchmark, that would also be great! ``` npm install npm test - -# Run node_redis benchmark (let's guess you cloned node_redis in another folder) -cd ../redis -npm install -npm run benchmark parser=javascript > old.log -# Replace the changed parser in the node_modules -npm run benchmark parser=javascript > new.log -node benchmarks/diff_multi_bench_output.js old.log new.log > improvement.log +npm run benchmark ``` ## License From 4e9ff43370231a85b3e0ac82c2cad06d01cb9690 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 5 May 2016 20:39:42 +0100 Subject: [PATCH 08/38] offset string extract on larger buffers --- .jshintrc | 2 +- lib/javascript.js | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.jshintrc b/.jshintrc index 986423d..36d4df2 100644 --- a/.jshintrc +++ b/.jshintrc @@ -9,7 +9,7 @@ "freeze": true, // prohibit overwriting prototypes of native objects "maxdepth": 4, "latedef": true, - "maxparams": 3, + "maxparams": 4, // Environment options "node": true, // Enable globals available when code is running inside of the NodeJS runtime environment. diff --git a/lib/javascript.js b/lib/javascript.js index 6f9c13b..79da5e9 100644 --- a/lib/javascript.js +++ b/lib/javascript.js @@ -33,11 +33,12 @@ function parseSimpleString(parser) { * @param parser * @param start * @param end + * @param noBuffer * @returns {*} */ -function convertBufferRange(parser, start, end) { +function convertBufferRange(parser, start, end, noBuffer) { // If returnBuffers is active, all return values are returned as buffers besides numbers and errors - if (parser.optionReturnBuffers === true) { + if (!noBuffer && parser.optionReturnBuffers === true) { return parser.buffer.slice(start, end); } @@ -48,9 +49,10 @@ function convertBufferRange(parser, start, end) { * Parse a '+' redis simple string response but forward the offsets * onto convertBufferRange to generate a string. * @param parser + * @param noBuffer * @returns {*} */ -function parseSimpleStringViaOffset(parser) { +function parseSimpleStringViaOffset(parser, noBuffer) { var start = parser.offset; var offset = parser.offset; var length = parser.buffer.length; @@ -61,7 +63,7 @@ function parseSimpleStringViaOffset(parser) { var c2 = parser.buffer[offset++]; if (c2 === 10) { // \n parser.offset = offset; - return convertBufferRange(parser, start, offset - 2); + return convertBufferRange(parser, start, offset - 2, noBuffer); } } } @@ -74,7 +76,14 @@ function parseSimpleStringViaOffset(parser) { * @returns {*} */ function parseLength(parser) { - var string = parseSimpleString(parser); + var string; + /* istanbul ignore if */ + if (parser.buffer.length > 4096) { + string = parseSimpleStringViaOffset(parser, true); + } else { + string = parseSimpleString(parser); + } + if (string !== undefined) { var length = +string; if (length === -1) { From adc47d0b932b2fa197ada6ab0da6d1bd61dbe8c7 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 06:09:02 +0200 Subject: [PATCH 09/38] Improve the js parser further Handle big data in constant time again Small performance improvements here and there Add ReplyError subclass --- index.js | 1 + lib/javascript.js | 101 ++++++++++++++++++++++------------------------ lib/replyError.js | 26 ++++++++++++ 3 files changed, 76 insertions(+), 52 deletions(-) create mode 100644 lib/replyError.js diff --git a/index.js b/index.js index a12aaf2..4ecb075 100644 --- a/index.js +++ b/index.js @@ -1,3 +1,4 @@ 'use strict'; module.exports = require('./lib/parser'); +module.exports.ReplyError = require('./lib/replyError'); diff --git a/lib/javascript.js b/lib/javascript.js index 79da5e9..a0cf332 100644 --- a/lib/javascript.js +++ b/lib/javascript.js @@ -1,5 +1,6 @@ 'use strict'; -/*jshint latedef: nofunc */ + +var ReplyError = require('./replyError'); /** * Used for lengths only, faster perf on arrays / bulks @@ -24,7 +25,6 @@ function parseSimpleString(parser) { } string += String.fromCharCode(c1); } - return undefined; } /** @@ -33,12 +33,11 @@ function parseSimpleString(parser) { * @param parser * @param start * @param end - * @param noBuffer * @returns {*} */ -function convertBufferRange(parser, start, end, noBuffer) { +function convertBufferRange(parser, start, end) { // If returnBuffers is active, all return values are returned as buffers besides numbers and errors - if (!noBuffer && parser.optionReturnBuffers === true) { + if (parser.optionReturnBuffers === true) { return parser.buffer.slice(start, end); } @@ -49,25 +48,20 @@ function convertBufferRange(parser, start, end, noBuffer) { * Parse a '+' redis simple string response but forward the offsets * onto convertBufferRange to generate a string. * @param parser - * @param noBuffer * @returns {*} */ -function parseSimpleStringViaOffset(parser, noBuffer) { +function parseSimpleStringViaOffset(parser) { var start = parser.offset; var offset = parser.offset; var length = parser.buffer.length; while (offset < length) { var c1 = parser.buffer[offset++]; - if (c1 === 13) { // \r - var c2 = parser.buffer[offset++]; - if (c2 === 10) { // \n - parser.offset = offset; - return convertBufferRange(parser, start, offset - 2, noBuffer); - } + if (c1 === 13) { // \r // Finding a \r is sufficient here, since in a simple string \r is always followed by \n + parser.offset = offset + 1; + return convertBufferRange(parser, start, offset - 1); } } - return undefined; } /** @@ -76,14 +70,7 @@ function parseSimpleStringViaOffset(parser, noBuffer) { * @returns {*} */ function parseLength(parser) { - var string; - /* istanbul ignore if */ - if (parser.buffer.length > 4096) { - string = parseSimpleStringViaOffset(parser, true); - } else { - string = parseSimpleString(parser); - } - + var string = parseSimpleString(parser); if (string !== undefined) { var length = +string; if (length === -1) { @@ -99,7 +86,7 @@ function parseLength(parser) { * @returns {*} */ function parseInteger(parser) { - var string = parseSimpleStringViaOffset(parser); + var string = parseSimpleString(parser); if (string !== undefined) { // If stringNumbers is activated the parser always returns numbers as string // This is important for big numbers (number > Math.pow(2, 53)) as js numbers @@ -114,16 +101,18 @@ function parseInteger(parser) { /** * Parse a '$' redis bulk string response * @param parser - * @returns {null} + * @returns {*} */ function parseBulkString(parser) { var length = parseLength(parser); - /* jshint eqnull: true */ - if (length == null) { + if (length === null || length === undefined) { return length; } var offsetEnd = parser.offset + length; - if ((offsetEnd + 2) > parser.buffer.length) { + if (offsetEnd + 2 > parser.buffer.length) { + parser.bufferCache.push(parser.buffer); + parser.totalChunkSize = parser.buffer.length; + parser.bigStrSize = length + 2; return; } @@ -139,9 +128,9 @@ function parseBulkString(parser) { * @returns {Error} */ function parseError(parser) { - var string = parseSimpleStringViaOffset(parser); - if (string !== undefined) { - return new Error(string); + var str = parseSimpleString(parser); + if (str !== undefined) { + return new ReplyError(str); } } @@ -162,8 +151,7 @@ function handleError(parser, error) { */ function parseArray(parser) { var length = parseLength(parser); - /* jshint eqnull: true */ - if (length == null) { // will break if using === + if (length === null || length === undefined) { return length; } @@ -189,6 +177,7 @@ function parseArray(parser) { * @param type * @returns {*} */ + function parseType(parser, type) { switch (type) { case 36: // $ @@ -202,37 +191,24 @@ function parseType(parser, type) { case 45: // - return parseError(parser); default: - return handleError(parser, new Error('Protocol error, got ' + JSON.stringify(String.fromCharCode(type)) + ' as reply type byte')); + return handleError(parser, new ReplyError('Protocol error, got ' + JSON.stringify(String.fromCharCode(type)) + ' as reply type byte')); } } -/** - * Quick buffer appending via buffer copy. - * @param parser - * @param buffer - */ -function appendToBuffer(parser, buffer) { - var oldLength = parser.buffer.length; - var remainingLength = oldLength - parser.offset; - var newLength = remainingLength + buffer.length; - var newBuffer = new Buffer(newLength); - parser.buffer.copy(newBuffer, 0, parser.offset, oldLength); - buffer.copy(newBuffer, remainingLength, 0, buffer.length); - parser.buffer = newBuffer; - parser.offset = 0; -} - /** * Javascript Redis Parser * @param options * @constructor */ function JavascriptRedisParser(options) { - this.optionReturnBuffers = !!options.return_buffers; - this.optionStringNumbers = !!options.string_numbers; + this.optionReturnBuffers = !!options.returnBuffers; + this.optionStringNumbers = !!options.stringNumbers; this.name = 'javascript'; this.offset = 0; this.buffer = null; + this.bigStrSize = 0; + this.totalChunkSize = 0; + this.bufferCache = []; } /** @@ -243,8 +219,29 @@ JavascriptRedisParser.prototype.execute = function (buffer) { if (this.buffer === null) { this.buffer = buffer; this.offset = 0; + } else if (this.bigStrSize === 0) { + var oldLength = this.buffer.length; + var remainingLength = oldLength - this.offset; + var newLength = remainingLength + buffer.length; + var newBuffer = new Buffer(newLength); + this.buffer.copy(newBuffer, 0, this.offset, oldLength); + buffer.copy(newBuffer, remainingLength, 0, buffer.length); + this.buffer = newBuffer; + this.offset = 0; + } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { + this.bufferCache.push(buffer); + if (this.offset !== 0) { + this.bufferCache[0] = this.bufferCache[0].slice(this.offset); + } + this.buffer = Buffer.concat(this.bufferCache, this.totalChunkSize + buffer.length - this.offset); + this.bigStrSize = 0; + this.totalChunkSize = 0; + this.bufferCache = []; + this.offset = 0; } else { - appendToBuffer(this, buffer); + this.bufferCache.push(buffer); + this.totalChunkSize += buffer.length; + return; } var length = this.buffer.length; diff --git a/lib/replyError.js b/lib/replyError.js new file mode 100644 index 0000000..c88290b --- /dev/null +++ b/lib/replyError.js @@ -0,0 +1,26 @@ +'use strict'; + +var util = require('util'); + +function ReplyError (message) { + var limit = Error.stackTraceLimit; + Error.stackTraceLimit = 2; + Error.captureStackTrace(this, this.constructor); + Error.stackTraceLimit = limit; + Object.defineProperty(this, 'name', { + value: 'ReplyError', + configurable: false, + enumerable: false, + writable: true + }); + Object.defineProperty(this, 'message', { + value: message || '', + configurable: false, + enumerable: false, + writable: true + }); +} + +util.inherits(ReplyError, Error); + +module.exports = ReplyError; From 1184bd68508c87a4dfafbebb2154c2a50c04f04d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 06:14:53 +0200 Subject: [PATCH 10/38] Add big data to benchmark --- benchmark/index.js | 69 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/benchmark/index.js b/benchmark/index.js index 6c32ec5..f608e76 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -1,15 +1,33 @@ var Benchmark = require('benchmark'); -var assert = require('assert'); -var suite = new Benchmark.Suite; +var suite = new Benchmark.Suite(); -var Parser = require('./../lib/parser'); +var Parser = require('./../'); var ParserOLD = require('./old/parser'); -function returnError(error) { - // throw error; silent for err error perf test +function returnError (error) { + error = null; } -function checkReply() { +function checkReply () {} + +function shuffle (array) { + var currentIndex = array.length; + var temporaryValue; + var randomIndex; + + // While there remain elements to shuffle... + while (currentIndex !== 0) { + // Pick a remaining element... + randomIndex = Math.floor(Math.random() * currentIndex); + currentIndex -= 1; + + // And swap it with the current element. + temporaryValue = array[currentIndex]; + array[currentIndex] = array[randomIndex]; + array[randomIndex] = temporaryValue; + } + + return array; } var startBuffer = new Buffer('$100\r\nabcdefghij'); @@ -19,6 +37,16 @@ var integerBuffer = new Buffer(':1237884\r\n'); var errorBuffer = new Buffer('-Error ohnoesitbroke\r\n'); var arrayBuffer = new Buffer('*1\r\n*1\r\n$1\r\na\r\n'); var endBuffer = new Buffer('\r\n'); +var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in'; // 256 chars +var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' '); // Math.pow(2, 16) chars long +var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n' + lorem); +var chunks = new Array(64); +for (var i = 0; i < 64; i++) { + chunks[i] = new Buffer(shuffle(bigStringArray).join(' ') + '.'); // Math.pow(2, 16) chars long +} var parserOld = new ParserOLD({ returnReply: checkReply, @@ -67,6 +95,32 @@ suite.add('NEW CODE: multiple chunks in a bulk string', function () { parser.execute(endBuffer); }); +// BIG BULK STRING + +suite.add('\nOLD CODE: 4mb bulk string', function () { + parserOld.execute(startBigBuffer); + for (var i = 0; i < 64; i++) { + parserOld.execute(chunks[i]); + } + parserOld.execute(endBuffer); +}); + +suite.add('HIREDIS: 4mb bulk string', function () { + parserHiRedis.execute(startBigBuffer); + for (var i = 0; i < 64; i++) { + parserHiRedis.execute(chunks[i]); + } + parserHiRedis.execute(endBuffer); +}); + +suite.add('NEW CODE: 4mb bulk string', function () { + parser.execute(startBigBuffer); + for (var i = 0; i < 64; i++) { + parser.execute(chunks[i]); + } + parser.execute(endBuffer); +}); + // STRINGS suite.add('\nOLD CODE: + simple string', function () { @@ -134,5 +188,4 @@ suite.on('complete', function () { console.log('\n\nFastest is ' + this.filter('fastest').map('name')); }); - -suite.run({delay:2, minSamples: 100 }); \ No newline at end of file +suite.run({ delay: 1, minSamples: 150 }); From 2e3ec4a505919ff703719a9d16ea8a07e4359b5c Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 06:44:11 +0200 Subject: [PATCH 11/38] Remove hiredis support --- README.md | 21 +-- changelog.md | 18 +++ lib/javascript.js | 268 ------------------------------- lib/parser.js | 338 ++++++++++++++++++++++++++++++++------- {lib => test}/hiredis.js | 25 ++- test/parsers.spec.js | 143 ++++++----------- 6 files changed, 371 insertions(+), 442 deletions(-) delete mode 100644 lib/javascript.js rename {lib => test}/hiredis.js (53%) diff --git a/README.md b/README.md index 737256d..24f985f 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ # redis-parser -A high performance redis parser solution built for [node_redis](https://github.com/NodeRedis/node_redis) and [ioredis](https://github.com/luin/ioredis). +A high performance javascript redis parser built for [node_redis](https://github.com/NodeRedis/node_redis) and [ioredis](https://github.com/luin/ioredis). Generally all [RESP](http://redis.io/topics/protocol) data will be properly parsed by the parser. @@ -30,8 +30,7 @@ new Parser(options); * `returnError`: *function*; mandatory * `returnFatalError`: *function*; optional, defaults to the returnError function * `returnBuffers`: *boolean*; optional, defaults to false -* `name`: *'javascript'|'hiredis'|'auto'|null*; optional, defaults to hiredis and falls back to the js parser if not available or if the stringNumbers option is choosen. Setting this to 'auto' or null is going to automatically determine what parser is available and chooses that one. -* `stringNumbers`: *boolean*; optional, defaults to false. This is only available for the javascript parser at the moment! +* `stringNumbers`: *boolean*; optional, defaults to false ### Example @@ -55,8 +54,7 @@ var parser = new Parser({ }, returnFatalError: function (err) { lib.returnFatalError(err); - }, - name: 'auto' // This returns either the hiredis or the js parser instance depending on what's available + } }); Library.prototype.streamHandler = function () { @@ -82,7 +80,6 @@ var parser = new Parser({ returnError: function(err) { lib.returnError(err); }, - name: 'javascript', // Use the Javascript parser stringNumbers: true, // Return all numbers as string instead of a js number returnBuffers: true // All strings are returned as buffer e.g. }); @@ -90,22 +87,14 @@ var parser = new Parser({ // The streamHandler as above ``` -## Further info - -The [hiredis](https://github.com/redis/hiredis) parser is still the fasted parser for -Node.js and therefor used as default in redis-parser if the hiredis parser is available. - -Otherwise the pure js NodeRedis parser is chosen that is almost as fast as the -hiredis parser besides some situations in which it'll be a bit slower. - ## Protocol errors -To handle protocol errors (this is very unlikely to happen) gracefully you should add the returnFatalError option, reject any still running command (they might have been processed properly but the reply is just wrong), destroy the socket and reconnect. +To handle protocol errors (this is very unlikely to happen) gracefully you should add the returnFatalError option, reject any still running command (they might have been processed properly but the reply is just wrong), destroy the socket and reconnect. Be aware that while doing this, no new command may be added, so all new commands have to be buffered in the meanwhile. Otherwise a chunk might still contain partial data of a following command that was already processed properly but answered in the same chunk as the command that resulted in the protocol error. ## Contribute -The js parser is already optimized but there are likely further optimizations possible. +The parser is already optimized but there are likely further optimizations possible. ``` npm install diff --git a/changelog.md b/changelog.md index 4efb816..44b9183 100644 --- a/changelog.md +++ b/changelog.md @@ -1,3 +1,21 @@ +## v.2.0.0 - 0x May, 2016 + +The javascript parser got completly rewritten by [Michael Diarmid](https://github.com/Salakar) and [Ruben Bridgewater](https://github.com/BridgeAR) and is now a lot faster than the hiredis parser. +Therefore the hiredis parser was removed and is only used for testing purposes and benchmarking comparison. + +All Errors returned by the parser are from now on of class ReplyError + +Features + +- Added ReplyError Class +- Added parser benchmark + +Removed + +- Dropped support of hiredis +- The `name` option is "removed" + - It is still available for backwards compatibility but it is strongly recommended not to use it + ## v.1.3.0 - 27 Mar, 2016 Features diff --git a/lib/javascript.js b/lib/javascript.js deleted file mode 100644 index a0cf332..0000000 --- a/lib/javascript.js +++ /dev/null @@ -1,268 +0,0 @@ -'use strict'; - -var ReplyError = require('./replyError'); - -/** - * Used for lengths only, faster perf on arrays / bulks - * @param parser - * @returns {*} - */ -function parseSimpleString(parser) { - var offset = parser.offset; - var length = parser.buffer.length; - var string = ''; - - while (offset < length) { - var c1 = parser.buffer[offset++]; - if (c1 === 13) { - var c2 = parser.buffer[offset++]; - if (c2 === 10) { - parser.offset = offset; - return string; - } - string += String.fromCharCode(c1) + String.fromCharCode(c2); - continue; - } - string += String.fromCharCode(c1); - } -} - -/** - * Returns a string or buffer of the provided offset start and - * end ranges. Checks `optionReturnBuffers`. - * @param parser - * @param start - * @param end - * @returns {*} - */ -function convertBufferRange(parser, start, end) { - // If returnBuffers is active, all return values are returned as buffers besides numbers and errors - if (parser.optionReturnBuffers === true) { - return parser.buffer.slice(start, end); - } - - return parser.buffer.toString('utf-8', start, end); -} - -/** - * Parse a '+' redis simple string response but forward the offsets - * onto convertBufferRange to generate a string. - * @param parser - * @returns {*} - */ -function parseSimpleStringViaOffset(parser) { - var start = parser.offset; - var offset = parser.offset; - var length = parser.buffer.length; - - while (offset < length) { - var c1 = parser.buffer[offset++]; - if (c1 === 13) { // \r // Finding a \r is sufficient here, since in a simple string \r is always followed by \n - parser.offset = offset + 1; - return convertBufferRange(parser, start, offset - 1); - } - } -} - -/** - * Returns the string length via parseSimpleString - * @param parser - * @returns {*} - */ -function parseLength(parser) { - var string = parseSimpleString(parser); - if (string !== undefined) { - var length = +string; - if (length === -1) { - return null; - } - return length; - } -} - -/** - * Parse a ':' redis integer response - * @param parser - * @returns {*} - */ -function parseInteger(parser) { - var string = parseSimpleString(parser); - if (string !== undefined) { - // If stringNumbers is activated the parser always returns numbers as string - // This is important for big numbers (number > Math.pow(2, 53)) as js numbers - // are 64bit floating point numbers with reduced precision - if (parser.optionStringNumbers === false) { - return +string; - } - return string; - } -} - -/** - * Parse a '$' redis bulk string response - * @param parser - * @returns {*} - */ -function parseBulkString(parser) { - var length = parseLength(parser); - if (length === null || length === undefined) { - return length; - } - var offsetEnd = parser.offset + length; - if (offsetEnd + 2 > parser.buffer.length) { - parser.bufferCache.push(parser.buffer); - parser.totalChunkSize = parser.buffer.length; - parser.bigStrSize = length + 2; - return; - } - - var offsetBegin = parser.offset; - parser.offset = offsetEnd + 2; - - return convertBufferRange(parser, offsetBegin, offsetEnd); -} - -/** - * Parse a '-' redis error response - * @param parser - * @returns {Error} - */ -function parseError(parser) { - var str = parseSimpleString(parser); - if (str !== undefined) { - return new ReplyError(str); - } -} - -/** - * Parsing error handler, resets parser buffer - * @param parser - * @param error - */ -function handleError(parser, error) { - parser.buffer = null; - parser.returnFatalError(error); -} - -/** - * Parse a - * @param parser - * @returns {*} - */ -function parseArray(parser) { - var length = parseLength(parser); - if (length === null || length === undefined) { - return length; - } - - var responses = new Array(length); - var bufferLength = parser.buffer.length; - for (var i = 0; i < length; i++) { - if (parser.offset >= bufferLength) { - return; - } - var response = parseType(parser, parser.buffer[parser.offset++]); - if (response === undefined) { - return; - } - responses[i] = response; - } - - return responses; -} - -/** - * Called the appropriate parser for the specified type. - * @param parser - * @param type - * @returns {*} - */ - -function parseType(parser, type) { - switch (type) { - case 36: // $ - return parseBulkString(parser); - case 58: // : - return parseInteger(parser); - case 43: // + - return parser.optionReturnBuffers ? parseSimpleStringViaOffset(parser) : parseSimpleString(parser); - case 42: // * - return parseArray(parser); - case 45: // - - return parseError(parser); - default: - return handleError(parser, new ReplyError('Protocol error, got ' + JSON.stringify(String.fromCharCode(type)) + ' as reply type byte')); - } -} - -/** - * Javascript Redis Parser - * @param options - * @constructor - */ -function JavascriptRedisParser(options) { - this.optionReturnBuffers = !!options.returnBuffers; - this.optionStringNumbers = !!options.stringNumbers; - this.name = 'javascript'; - this.offset = 0; - this.buffer = null; - this.bigStrSize = 0; - this.totalChunkSize = 0; - this.bufferCache = []; -} - -/** - * Parse the redis buffer - * @param buffer - */ -JavascriptRedisParser.prototype.execute = function (buffer) { - if (this.buffer === null) { - this.buffer = buffer; - this.offset = 0; - } else if (this.bigStrSize === 0) { - var oldLength = this.buffer.length; - var remainingLength = oldLength - this.offset; - var newLength = remainingLength + buffer.length; - var newBuffer = new Buffer(newLength); - this.buffer.copy(newBuffer, 0, this.offset, oldLength); - buffer.copy(newBuffer, remainingLength, 0, buffer.length); - this.buffer = newBuffer; - this.offset = 0; - } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { - this.bufferCache.push(buffer); - if (this.offset !== 0) { - this.bufferCache[0] = this.bufferCache[0].slice(this.offset); - } - this.buffer = Buffer.concat(this.bufferCache, this.totalChunkSize + buffer.length - this.offset); - this.bigStrSize = 0; - this.totalChunkSize = 0; - this.bufferCache = []; - this.offset = 0; - } else { - this.bufferCache.push(buffer); - this.totalChunkSize += buffer.length; - return; - } - - var length = this.buffer.length; - - while (this.offset < length) { - var offset = this.offset; - var type = this.buffer[this.offset++]; - var response = parseType(this, type); - if (response === undefined) { - this.offset = offset; - return; - } - - if (type === 45) { - this.returnError(response); // Errors - - } else { - this.returnReply(response); // Strings + // Integers : // Bulk strings $ // Arrays * - } - } - - this.buffer = null; -}; - -module.exports = JavascriptRedisParser; diff --git a/lib/parser.js b/lib/parser.js index 2180652..5d47792 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -1,59 +1,287 @@ 'use strict'; -var parsers = { - javascript: require('./javascript') +var ReplyError = require('./replyError'); + +/** + * Used for lengths only, faster perf on arrays / bulks + * @param parser + * @returns {*} + */ +function parseSimpleString(parser) { + var offset = parser.offset; + var length = parser.buffer.length; + var string = ''; + + while (offset < length) { + var c1 = parser.buffer[offset++]; + if (c1 === 13) { + var c2 = parser.buffer[offset++]; + if (c2 === 10) { + parser.offset = offset; + return string; + } + string += String.fromCharCode(c1) + String.fromCharCode(c2); + continue; + } + string += String.fromCharCode(c1); + } +} + +/** + * Returns a string or buffer of the provided offset start and + * end ranges. Checks `optionReturnBuffers`. + * @param parser + * @param start + * @param end + * @returns {*} + */ +function convertBufferRange(parser, start, end) { + // If returnBuffers is active, all return values are returned as buffers besides numbers and errors + if (parser.optionReturnBuffers === true) { + return parser.buffer.slice(start, end); + } + + return parser.buffer.toString('utf-8', start, end); +} + +/** + * Parse a '+' redis simple string response but forward the offsets + * onto convertBufferRange to generate a string. + * @param parser + * @returns {*} + */ +function parseSimpleStringViaOffset(parser) { + var start = parser.offset; + var offset = parser.offset; + var length = parser.buffer.length; + + while (offset < length) { + var c1 = parser.buffer[offset++]; + if (c1 === 13) { // \r // Finding a \r is sufficient here, since in a simple string \r is always followed by \n + parser.offset = offset + 1; + return convertBufferRange(parser, start, offset - 1); + } + } +} + +/** + * Returns the string length via parseSimpleString + * @param parser + * @returns {*} + */ +function parseLength(parser) { + var string = parseSimpleString(parser); + if (string !== undefined) { + var length = +string; + if (length === -1) { + return null; + } + return length; + } +} + +/** + * Parse a ':' redis integer response + * @param parser + * @returns {*} + */ +function parseInteger(parser) { + var string = parseSimpleString(parser); + if (string !== undefined) { + // If stringNumbers is activated the parser always returns numbers as string + // This is important for big numbers (number > Math.pow(2, 53)) as js numbers + // are 64bit floating point numbers with reduced precision + if (parser.optionStringNumbers === false) { + return +string; + } + return string; + } +} + +/** + * Parse a '$' redis bulk string response + * @param parser + * @returns {*} + */ +function parseBulkString(parser) { + var length = parseLength(parser); + if (length === null || length === undefined) { + return length; + } + var offsetEnd = parser.offset + length; + if (offsetEnd + 2 > parser.buffer.length) { + parser.bufferCache.push(parser.buffer); + parser.totalChunkSize = parser.buffer.length; + parser.bigStrSize = length + 2; + return; + } + + var offsetBegin = parser.offset; + parser.offset = offsetEnd + 2; + + return convertBufferRange(parser, offsetBegin, offsetEnd); +} + +/** + * Parse a '-' redis error response + * @param parser + * @returns {Error} + */ +function parseError(parser) { + var str = parseSimpleString(parser); + if (str !== undefined) { + return new ReplyError(str); + } +} + +/** + * Parsing error handler, resets parser buffer + * @param parser + * @param error + */ +function handleError(parser, error) { + parser.buffer = null; + parser.returnFatalError(error); +} + +/** + * Parse a + * @param parser + * @returns {*} + */ +function parseArray(parser) { + var length = parseLength(parser); + if (length === null || length === undefined) { + return length; + } + + var responses = new Array(length); + var bufferLength = parser.buffer.length; + for (var i = 0; i < length; i++) { + if (parser.offset >= bufferLength) { + return; + } + var response = parseType(parser, parser.buffer[parser.offset++]); + if (response === undefined) { + return; + } + responses[i] = response; + } + + return responses; +} + +/** + * Called the appropriate parser for the specified type. + * @param parser + * @param type + * @returns {*} + */ + +function parseType(parser, type) { + switch (type) { + case 36: // $ + return parseBulkString(parser); + case 58: // : + return parseInteger(parser); + case 43: // + + return parser.optionReturnBuffers ? parseSimpleStringViaOffset(parser) : parseSimpleString(parser); + case 42: // * + return parseArray(parser); + case 45: // - + return parseError(parser); + default: + return handleError(parser, new ReplyError('Protocol error, got ' + JSON.stringify(String.fromCharCode(type)) + ' as reply type byte')); + } +} + +/** + * Javascript Redis Parser + * @param options + * @constructor + */ +function JavascriptRedisParser(options) { + if (!(this instanceof JavascriptRedisParser)) { + return new JavascriptRedisParser(options); + } + if ( + !options || + typeof options.returnError !== 'function' || + typeof options.returnReply !== 'function' + ) { + throw new TypeError('Please provide all return functions while initiating the parser'); + } + if (options.name === 'hiredis') { + try { + var hiredis = require('../test/hiredis'); + return new hiredis(options); + } catch (e) {} + } + this.optionReturnBuffers = !!options.returnBuffers; + this.optionStringNumbers = !!options.stringNumbers; + this.returnError = options.returnError; + this.returnFatalError = options.returnFatalError || options.returnError; + this.returnReply = options.returnReply; + this.name = 'javascript'; + this.offset = 0; + this.buffer = null; + this.bigStrSize = 0; + this.totalChunkSize = 0; + this.bufferCache = []; +} + +/** + * Parse the redis buffer + * @param buffer + */ +JavascriptRedisParser.prototype.execute = function (buffer) { + if (this.buffer === null) { + this.buffer = buffer; + this.offset = 0; + } else if (this.bigStrSize === 0) { + var oldLength = this.buffer.length; + var remainingLength = oldLength - this.offset; + var newLength = remainingLength + buffer.length; + var newBuffer = new Buffer(newLength); + this.buffer.copy(newBuffer, 0, this.offset, oldLength); + buffer.copy(newBuffer, remainingLength, 0, buffer.length); + this.buffer = newBuffer; + this.offset = 0; + } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { + this.bufferCache.push(buffer); + if (this.offset !== 0) { + this.bufferCache[0] = this.bufferCache[0].slice(this.offset); + } + this.buffer = Buffer.concat(this.bufferCache, this.totalChunkSize + buffer.length - this.offset); + this.bigStrSize = 0; + this.totalChunkSize = 0; + this.bufferCache = []; + this.offset = 0; + } else { + this.bufferCache.push(buffer); + this.totalChunkSize += buffer.length; + return; + } + + var length = this.buffer.length; + + while (this.offset < length) { + var offset = this.offset; + var type = this.buffer[this.offset++]; + var response = parseType(this, type); + if (response === undefined) { + this.offset = offset; + return; + } + + if (type === 45) { + this.returnError(response); // Errors - + } else { + this.returnReply(response); // Strings + // Integers : // Bulk strings $ // Arrays * + } + } + + this.buffer = null; }; -// Hiredis might not be installed -try { - parsers.hiredis = require('./hiredis'); -} catch (err) { /* ignore errors */ } - -function Parser (options) { - var parser, msg; - - if ( - !options || - typeof options.returnError !== 'function' || - typeof options.returnReply !== 'function' - ) { - throw new Error('Please provide all return functions while initiating the parser'); - } - - if (options.name === 'hiredis') { - /* istanbul ignore if: hiredis should always be installed while testing */ - if (!parsers.hiredis) { - msg = 'You explicitly required the hiredis parser but hiredis is not installed. The JS parser is going to be returned instead.'; - } else if (options.stringNumbers) { - msg = 'You explicitly required the hiredis parser in combination with the stringNumbers option. Only the JS parser can handle that and is choosen instead.'; - } - } else if (options.name && !parsers[options.name] && options.name !== 'auto') { - msg = 'The requested parser "' + options.name + '" is unkown and the default parser is choosen instead.'; - } - - if (msg) { - console.warn(new Error(msg).stack.replace('Error: ', 'Warning: ')); - } - - options.name = options.name || 'hiredis'; - options.name = options.name.toLowerCase(); - - var innerOptions = { - // The hiredis parser expects underscores - return_buffers: options.returnBuffers || false, - string_numbers: options.stringNumbers || false - }; - - if (options.name === 'javascript' || !parsers.hiredis || options.stringNumbers) { - parser = new parsers.javascript(innerOptions); - } else { - parser = new parsers.hiredis(innerOptions); - } - - parser.returnError = options.returnError; - parser.returnFatalError = options.returnFatalError || options.returnError; - parser.returnReply = options.returnReply; - return parser; -} - -module.exports = Parser; +module.exports = JavascriptRedisParser; diff --git a/lib/hiredis.js b/test/hiredis.js similarity index 53% rename from lib/hiredis.js rename to test/hiredis.js index 75ce4ba..c14d2a8 100644 --- a/lib/hiredis.js +++ b/test/hiredis.js @@ -1,20 +1,21 @@ 'use strict'; var hiredis = require('hiredis'); +var ReplyError = require('../lib/replyError'); /** - * Parse data + * Parse data * @param parser * @returns {*} */ -function parseData(parser) { +function parseData (parser) { try { return parser.reader.get(); } catch (err) { // Protocol errors land here // Reset the parser. Otherwise new commands can't be processed properly parser.reader = new hiredis.Reader(parser.options); - parser.returnFatalError(err); + parser.returnFatalError(new ReplyError(err.message)); } } @@ -23,10 +24,18 @@ function parseData(parser) { * @param options * @constructor */ -function HiredisReplyParser(options) { +function HiredisReplyParser (options) { + if (!(this instanceof HiredisReplyParser)) { + return new HiredisReplyParser(options); + } + this.returnError = options.returnError; + this.returnFatalError = options.returnFatalError || options.returnError; + this.returnReply = options.returnReply; this.name = 'hiredis'; - this.options = options; - this.reader = new hiredis.Reader(options); + this.options = { + return_buffers: !!options.returnBuffers + }; + this.reader = new hiredis.Reader(this.options); } HiredisReplyParser.prototype.execute = function (data) { @@ -35,12 +44,12 @@ HiredisReplyParser.prototype.execute = function (data) { while (reply !== undefined) { if (reply && reply.name === 'Error') { - this.returnError(reply); + this.returnError(new ReplyError(reply.message)); } else { this.returnReply(reply); } reply = parseData(this); } -}; +} module.exports = HiredisReplyParser; diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 4a5a486..a1add74 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -2,8 +2,10 @@ var intercept = require('intercept-stdout'); var assert = require('assert'); -var Parser = require('../'); -var parsers = ['javascript', 'hiredis']; +var JavascriptParser = require('../'); +var HiredisParser = require('./hiredis'); +var ReplyError = JavascriptParser.ReplyError; +var parsers = [JavascriptParser, HiredisParser]; // Mock the not needed return functions function returnReply () { throw new Error('failed'); } @@ -14,38 +16,21 @@ describe('parsers', function () { describe('general parser functionality', function () { - it('use default values', function () { - var parser = new Parser({ - returnReply: returnReply, - returnError: returnError - }); - assert.strictEqual(parser.returnError, parser.returnFatalError); - assert.strictEqual(parser.name, 'hiredis'); - }); - - it('auto parser', function () { - var parser = new Parser({ - returnReply: returnReply, - returnError: returnError, - name: 'auto' - }); - assert.strictEqual(parser.name, 'hiredis'); - }); - - it('auto parser v2', function () { - var parser = new Parser({ + it('backwards compatibility with hiredis', function () { + var parser = new JavascriptParser({ returnReply: returnReply, returnError: returnError, - name: null + name: 'hiredis' }); assert.strictEqual(parser.name, 'hiredis'); }); it('fail for missing options', function () { assert.throws(function() { - new Parser({ + new JavascriptParser({ returnReply: returnReply, - returnBuffers: true + returnBuffers: true, + name: 'hiredis' }); }, function (err) { assert.strictEqual(err.message, 'Please provide all return functions while initiating the parser'); @@ -54,44 +39,11 @@ describe('parsers', function () { }); - it('unknown parser', function () { - var str = ''; - var unhookIntercept = intercept(function (data) { - str += data; - return ''; - }); - var parser = new Parser({ - returnReply: returnReply, - returnError: returnError, - name: 'something_unknown' - }); - unhookIntercept(); - assert.strictEqual(parser.name, 'hiredis'); - assert(/^Warning: The requested parser "something_unknown" is unkown and the default parser is choosen instead\.\n +at new Parser/.test(str), str); - }); - - it('hiredis and stringNumbers', function () { - var str = ''; - var unhookIntercept = intercept(function (data) { - str += data; - return ''; - }); - var parser = new Parser({ - returnReply: returnReply, - returnError: returnError, - name: 'hiredis', - stringNumbers: true - }); - unhookIntercept(); - assert.strictEqual(parser.name, 'javascript'); - assert(/^Warning: You explicitly required the hiredis parser in combination with the stringNumbers option. .+.\.\n +at new Parser/.test(str), str); - }); - }); - parsers.forEach(function (parserName) { + parsers.forEach(function (Parser) { - describe(parserName, function () { + describe(Parser.name, function () { it('handles multi-bulk reply and check context binding', function () { var replyCount = 0; @@ -108,8 +60,7 @@ describe('parsers', function () { test.checkReply(reply); }, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer('*1\r\n*1\r\n$1\r\na\r\n')); @@ -128,9 +79,12 @@ describe('parsers', function () { it('parser error', function () { var replyCount = 0; function Abc () {} - Abc.prototype.checkReply = function (reply) { + Abc.prototype.checkReply = function (err) { assert.strictEqual(typeof this.log, 'function'); - assert.strictEqual(reply.message, 'Protocol error, got "a" as reply type byte'); + assert.strictEqual(err.message, 'Protocol error, got "a" as reply type byte'); + assert.strictEqual(err.name, 'ReplyError'); + assert(err instanceof ReplyError); + assert(err instanceof Error); replyCount++; }; Abc.prototype.log = console.log; @@ -140,8 +94,7 @@ describe('parsers', function () { returnError: returnError, returnFatalError: function (err) { test.checkReply(err); - }, - name: parserName + } }); parser.execute(new Buffer('a*1\r*1\r$1`zasd\r\na')); @@ -165,7 +118,6 @@ describe('parsers', function () { returnReply: checkReply, returnError: returnError, returnFatalError: checkError, - name: parserName, returnBuffers: true }); @@ -190,8 +142,7 @@ describe('parsers', function () { } var parser = new Parser({ returnReply: checkReply, - returnError: checkError, - name: parserName + returnError: checkError }); parser.execute(new Buffer('*1\r\n+OK\r\n\n+zasd\r\n')); @@ -210,8 +161,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer('$4\r\nfoo\r\r\n$8\r\nfoo\r\nbar\r\n$5\r\n\r\n')); @@ -229,8 +179,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer('*1\r\n*1\r\n$1\r\na')); @@ -252,8 +201,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer('$100\r\nabcdefghij')); @@ -289,15 +237,25 @@ describe('parsers', function () { 'abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij', 'test', 100, - new Error('Error message'), - ['The force awakens'] + new ReplyError('Error message'), + ['The force awakens'], + new ReplyError() ]; function checkReply(reply) { for (var i = 0; i < reply.length; i++) { - if (i < 3) { - assert.strictEqual(reply[i], predefined_data[i]); + if (Array.isArray(reply[i])) { + reply[i].forEach(function (reply, j) { + assert.strictEqual(reply, predefined_data[i][j]); + }); + } else if (reply[i] instanceof Error) { + if (Parser.name !== 'HiredisReplyParser') { // The hiredis always returns normal errors in case of nested ones + assert(reply[i] instanceof ReplyError); + assert.strictEqual(reply[i].name, predefined_data[i].name); + assert.deepStrictEqual(reply[i], predefined_data[i]); + } + assert.strictEqual(reply[i].message, predefined_data[i].message); } else { - assert.deepEqual(reply[i], predefined_data[i]); + assert.strictEqual(reply[i], predefined_data[i]); } } replyCount++; @@ -306,11 +264,10 @@ describe('parsers', function () { returnReply: checkReply, returnError: returnError, returnFatalError: returnFatalError, - name: parserName, returnBuffers: false }); - parser.execute(new Buffer('*5\r\n$100\r\nabcdefghij')); + parser.execute(new Buffer('*6\r\n$100\r\nabcdefghij')); parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij\r\n')); @@ -319,7 +276,7 @@ describe('parsers', function () { parser.execute(new Buffer('\r\n-Error message')); parser.execute(new Buffer('\r\n*1\r\n$17\r\nThe force')); assert.strictEqual(replyCount, 0); - parser.execute(new Buffer(' awakens\r\n$5')); + parser.execute(new Buffer(' awakens\r\n-\r\n$5')); assert.strictEqual(replyCount, 1); }); @@ -332,8 +289,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: returnError, returnError: checkReply, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer('-Error ')); @@ -352,8 +308,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer('$-1\r\n*-')); @@ -373,8 +328,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer(':')); @@ -396,8 +350,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer(':1\r\n:')); @@ -424,7 +377,6 @@ describe('parsers', function () { returnReply: checkReply, returnError: returnError, returnFatalError: returnFatalError, - name: parserName, returnBuffers: true }); @@ -446,8 +398,7 @@ describe('parsers', function () { var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnFatalError, - name: parserName + returnFatalError: returnFatalError }); parser.execute(new Buffer('$10\r\ntest ')); assert.strictEqual(replyCount, 0); @@ -458,6 +409,9 @@ describe('parsers', function () { }); it('return numbers as strings', function () { + if (Parser.name === 'HiredisReplyParser') { + return this.skip(); + } var replyCount = 0; var entries = ['123', '590295810358705700002', '-99999999999999999']; function checkReply(reply) { @@ -472,7 +426,6 @@ describe('parsers', function () { returnReply: checkReply, returnError: returnError, returnFatalError: returnFatalError, - name: parserName, stringNumbers: true }); unhookIntercept(); From 742226e806cc48837aeb675f1936a6cf2bfaca17 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 06:44:35 +0200 Subject: [PATCH 12/38] Update npmignore file --- .npmignore | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.npmignore b/.npmignore index 1e83c64..1c22fdc 100644 --- a/.npmignore +++ b/.npmignore @@ -1,10 +1,12 @@ -# Created by .ignore support plugin (hsz.mobi) -### Example user template template -### Example user template - # IntelliJ project files .idea *.iml out gen -benchmark \ No newline at end of file + +# Unrelevant files and folders +benchmark +coverage +test +.travis.yml +.gitignore From 298d074a7d9a6f9f53f41f512488728ded055d02 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 06:49:03 +0200 Subject: [PATCH 13/38] Replace jshint with standard --- .jshintignore | 6 - .jshintrc | 20 - README.md | 1 + benchmark/index.js | 234 +++++----- benchmark/old/hiredis.js | 52 +-- benchmark/old/javascript.js | 304 ++++++------- benchmark/old/parser.js | 98 ++-- index.js | 6 +- lib/parser.js | 370 ++++++++-------- lib/replyError.js | 40 +- package.json | 8 +- test/hiredis.js | 66 +-- test/parsers.spec.js | 859 ++++++++++++++++++------------------ 13 files changed, 1015 insertions(+), 1049 deletions(-) delete mode 100644 .jshintignore delete mode 100644 .jshintrc diff --git a/.jshintignore b/.jshintignore deleted file mode 100644 index 59a1197..0000000 --- a/.jshintignore +++ /dev/null @@ -1,6 +0,0 @@ -node_modules/** -coverage/** -**.md -**.log -.idea -benchmark \ No newline at end of file diff --git a/.jshintrc b/.jshintrc deleted file mode 100644 index 36d4df2..0000000 --- a/.jshintrc +++ /dev/null @@ -1,20 +0,0 @@ -{ - "eqeqeq": true, // Prohibits the use of == and != in favor of === and !== - "noarg": true, // Prohibit use of `arguments.caller` and `arguments.callee` - "undef": true, // Require all non-global variables be declared before they are used. - "unused": "vars", // Warn unused variables, but not unused params - "strict": true, // Require `use strict` pragma in every file. - "nonbsp": true, // don't allow non utf-8 pages to break - "forin": true, // don't allow not filtert for in loops - "freeze": true, // prohibit overwriting prototypes of native objects - "maxdepth": 4, - "latedef": true, - "maxparams": 4, - - // Environment options - "node": true, // Enable globals available when code is running inside of the NodeJS runtime environment. - "mocha": true, - - // Relaxing options - "boss": true // Accept statements like `while (key = keys.pop()) {}` -} diff --git a/README.md b/README.md index 24f985f..5278ac6 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ [![Build Status](https://travis-ci.org/NodeRedis/node-redis-parser.png?branch=master)](https://travis-ci.org/NodeRedis/node-redis-parser) [![Code Climate](https://codeclimate.com/github/NodeRedis/node-redis-parser/badges/gpa.svg)](https://codeclimate.com/github/NodeRedis/node-redis-parser) [![Test Coverage](https://codeclimate.com/github/NodeRedis/node-redis-parser/badges/coverage.svg)](https://codeclimate.com/github/NodeRedis/node-redis-parser/coverage) +[![js-standard-style](https://img.shields.io/badge/code%20style-standard-brightgreen.svg)](http://standardjs.com/) # redis-parser diff --git a/benchmark/index.js b/benchmark/index.js index f608e76..abc0d9d 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -1,191 +1,189 @@ -var Benchmark = require('benchmark'); -var suite = new Benchmark.Suite(); +var Benchmark = require('benchmark') +var suite = new Benchmark.Suite() -var Parser = require('./../'); -var ParserOLD = require('./old/parser'); +var Parser = require('./../') +var ParserOLD = require('./old/parser') function returnError (error) { - error = null; + error = null } function checkReply () {} function shuffle (array) { - var currentIndex = array.length; - var temporaryValue; - var randomIndex; - - // While there remain elements to shuffle... - while (currentIndex !== 0) { - // Pick a remaining element... - randomIndex = Math.floor(Math.random() * currentIndex); - currentIndex -= 1; - - // And swap it with the current element. - temporaryValue = array[currentIndex]; - array[currentIndex] = array[randomIndex]; - array[randomIndex] = temporaryValue; - } - - return array; + var currentIndex = array.length + var temporaryValue + var randomIndex + + // While there remain elements to shuffle... + while (currentIndex !== 0) { + // Pick a remaining element... + randomIndex = Math.floor(Math.random() * currentIndex) + currentIndex -= 1 + + // And swap it with the current element. + temporaryValue = array[currentIndex] + array[currentIndex] = array[randomIndex] + array[randomIndex] = temporaryValue + } + + return array } -var startBuffer = new Buffer('$100\r\nabcdefghij'); -var chunkBuffer = new Buffer('abcdefghijabcdefghijabcdefghij'); -var stringBuffer = new Buffer('+testing a simple string\r\n'); -var integerBuffer = new Buffer(':1237884\r\n'); -var errorBuffer = new Buffer('-Error ohnoesitbroke\r\n'); -var arrayBuffer = new Buffer('*1\r\n*1\r\n$1\r\na\r\n'); -var endBuffer = new Buffer('\r\n'); +var startBuffer = new Buffer('$100\r\nabcdefghij') +var chunkBuffer = new Buffer('abcdefghijabcdefghijabcdefghij') +var stringBuffer = new Buffer('+testing a simple string\r\n') +var integerBuffer = new Buffer(':1237884\r\n') +var errorBuffer = new Buffer('-Error ohnoesitbroke\r\n') +var arrayBuffer = new Buffer('*1\r\n*1\r\n$1\r\na\r\n') +var endBuffer = new Buffer('\r\n') var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + - 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + - 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + - 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in'; // 256 chars -var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' '); // Math.pow(2, 16) chars long -var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n' + lorem); -var chunks = new Array(64); + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars +var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long +var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n' + lorem) +var chunks = new Array(64) for (var i = 0; i < 64; i++) { - chunks[i] = new Buffer(shuffle(bigStringArray).join(' ') + '.'); // Math.pow(2, 16) chars long + chunks[i] = new Buffer(shuffle(bigStringArray).join(' ') + '.') // Math.pow(2, 16) chars long } var parserOld = new ParserOLD({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnError, - name: 'javascript' -}); + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + name: 'javascript' +}) var parserHiRedis = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnError, - name: 'hiredis' -}); + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + name: 'hiredis' +}) var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnError, - name: 'javascript' -}); + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + name: 'javascript' +}) // BULK STRINGS suite.add('OLD CODE: multiple chunks in a bulk string', function () { - parserOld.execute(startBuffer); - parserOld.execute(chunkBuffer); - parserOld.execute(chunkBuffer); - parserOld.execute(chunkBuffer); - parserOld.execute(endBuffer); -}); + parserOld.execute(startBuffer) + parserOld.execute(chunkBuffer) + parserOld.execute(chunkBuffer) + parserOld.execute(chunkBuffer) + parserOld.execute(endBuffer) +}) suite.add('HIREDIS: multiple chunks in a bulk string', function () { - parserHiRedis.execute(startBuffer); - parserHiRedis.execute(chunkBuffer); - parserHiRedis.execute(chunkBuffer); - parserHiRedis.execute(chunkBuffer); - parserHiRedis.execute(endBuffer); -}); + parserHiRedis.execute(startBuffer) + parserHiRedis.execute(chunkBuffer) + parserHiRedis.execute(chunkBuffer) + parserHiRedis.execute(chunkBuffer) + parserHiRedis.execute(endBuffer) +}) suite.add('NEW CODE: multiple chunks in a bulk string', function () { - parser.execute(startBuffer); - parser.execute(chunkBuffer); - parser.execute(chunkBuffer); - parser.execute(chunkBuffer); - parser.execute(endBuffer); -}); + parser.execute(startBuffer) + parser.execute(chunkBuffer) + parser.execute(chunkBuffer) + parser.execute(chunkBuffer) + parser.execute(endBuffer) +}) // BIG BULK STRING suite.add('\nOLD CODE: 4mb bulk string', function () { - parserOld.execute(startBigBuffer); - for (var i = 0; i < 64; i++) { - parserOld.execute(chunks[i]); - } - parserOld.execute(endBuffer); -}); + parserOld.execute(startBigBuffer) + for (var i = 0; i < 64; i++) { + parserOld.execute(chunks[i]) + } + parserOld.execute(endBuffer) +}) suite.add('HIREDIS: 4mb bulk string', function () { - parserHiRedis.execute(startBigBuffer); - for (var i = 0; i < 64; i++) { - parserHiRedis.execute(chunks[i]); - } - parserHiRedis.execute(endBuffer); -}); + parserHiRedis.execute(startBigBuffer) + for (var i = 0; i < 64; i++) { + parserHiRedis.execute(chunks[i]) + } + parserHiRedis.execute(endBuffer) +}) suite.add('NEW CODE: 4mb bulk string', function () { - parser.execute(startBigBuffer); - for (var i = 0; i < 64; i++) { - parser.execute(chunks[i]); - } - parser.execute(endBuffer); -}); + parser.execute(startBigBuffer) + for (var i = 0; i < 64; i++) { + parser.execute(chunks[i]) + } + parser.execute(endBuffer) +}) // STRINGS suite.add('\nOLD CODE: + simple string', function () { - parserOld.execute(stringBuffer); -}); + parserOld.execute(stringBuffer) +}) suite.add('HIREDIS: + simple string', function () { - parserHiRedis.execute(stringBuffer); -}); + parserHiRedis.execute(stringBuffer) +}) suite.add('NEW CODE: + simple string', function () { - parser.execute(stringBuffer); -}); + parser.execute(stringBuffer) +}) // INTEGERS suite.add('\nOLD CODE: + integer', function () { - parserOld.execute(integerBuffer); -}); + parserOld.execute(integerBuffer) +}) suite.add('HIREDIS: + integer', function () { - parserHiRedis.execute(integerBuffer); -}); + parserHiRedis.execute(integerBuffer) +}) suite.add('NEW CODE: + integer', function () { - parser.execute(integerBuffer); -}); + parser.execute(integerBuffer) +}) // ARRAYS suite.add('\nOLD CODE: * array', function () { - parserOld.execute(arrayBuffer); -}); + parserOld.execute(arrayBuffer) +}) suite.add('HIREDIS: * array', function () { - parserHiRedis.execute(arrayBuffer); -}); + parserHiRedis.execute(arrayBuffer) +}) suite.add('NEW CODE: * array', function () { - parser.execute(arrayBuffer); -}); - + parser.execute(arrayBuffer) +}) // ERRORS suite.add('\nOLD CODE: * error', function () { - parserOld.execute(errorBuffer); -}); + parserOld.execute(errorBuffer) +}) suite.add('HIREDIS: * error', function () { - parserHiRedis.execute(errorBuffer); -}); + parserHiRedis.execute(errorBuffer) +}) suite.add('NEW CODE: * error', function () { - parser.execute(errorBuffer); -}); - + parser.execute(errorBuffer) +}) // add listeners suite.on('cycle', function (event) { - console.log(String(event.target)); -}); + console.log(String(event.target)) +}) suite.on('complete', function () { - console.log('\n\nFastest is ' + this.filter('fastest').map('name')); -}); + console.log('\n\nFastest is ' + this.filter('fastest').map('name')) +}) -suite.run({ delay: 1, minSamples: 150 }); +suite.run({ delay: 1, minSamples: 150 }) diff --git a/benchmark/old/hiredis.js b/benchmark/old/hiredis.js index 965b660..9e3fe45 100644 --- a/benchmark/old/hiredis.js +++ b/benchmark/old/hiredis.js @@ -1,36 +1,36 @@ -'use strict'; +'use strict' -var hiredis = require('hiredis'); +var hiredis = require('hiredis') -function HiredisReplyParser(options) { - this.name = 'hiredis'; - this.options = options; - this.reader = new hiredis.Reader(options); +function HiredisReplyParser (options) { + this.name = 'hiredis' + this.options = options + this.reader = new hiredis.Reader(options) } HiredisReplyParser.prototype.parseData = function () { - try { - return this.reader.get(); - } catch (err) { - // Protocol errors land here - // Reset the parser. Otherwise new commands can't be processed properly - this.reader = new hiredis.Reader(this.options); - this.returnFatalError(err); - } -}; + try { + return this.reader.get() + } catch (err) { + // Protocol errors land here + // Reset the parser. Otherwise new commands can't be processed properly + this.reader = new hiredis.Reader(this.options) + this.returnFatalError(err) + } +} HiredisReplyParser.prototype.execute = function (data) { - this.reader.feed(data); - var reply = this.parseData(); + this.reader.feed(data) + var reply = this.parseData() - while (reply !== undefined) { - if (reply && reply.name === 'Error') { - this.returnError(reply); - } else { - this.returnReply(reply); - } - reply = this.parseData(); + while (reply !== undefined) { + if (reply && reply.name === 'Error') { + this.returnError(reply) + } else { + this.returnReply(reply) } -}; + reply = this.parseData() + } +} -module.exports = HiredisReplyParser; +module.exports = HiredisReplyParser diff --git a/benchmark/old/javascript.js b/benchmark/old/javascript.js index 8302a5c..d742e98 100644 --- a/benchmark/old/javascript.js +++ b/benchmark/old/javascript.js @@ -1,172 +1,172 @@ -'use strict'; +'use strict' -function JavascriptReplyParser(options) { - this.name = 'javascript_old'; - this.buffer = new Buffer(0); - this.offset = 0; - this.bigStrSize = 0; - this.chunksSize = 0; - this.buffers = []; - this.type = 0; - this.protocolError = false; - this.offsetCache = 0; - // If returnBuffers is active, all return values are returned as buffers besides numbers and errors - if (options.return_buffers) { - this.handleReply = function (start, end) { - return this.buffer.slice(start, end); - }; - } else { - this.handleReply = function (start, end) { - return this.buffer.toString('utf-8', start, end); - }; - } - // If stringNumbers is activated the parser always returns numbers as string - // This is important for big numbers (number > Math.pow(2, 53)) as js numbers are 64bit floating point numbers with reduced precision - if (options.string_numbers) { - this.handleNumbers = function (start, end) { - return this.buffer.toString('ascii', start, end); - }; - } else { - this.handleNumbers = function (start, end) { - return +this.buffer.toString('ascii', start, end); - }; - } +function JavascriptReplyParser (options) { + this.name = 'javascript_old' + this.buffer = new Buffer(0) + this.offset = 0 + this.bigStrSize = 0 + this.chunksSize = 0 + this.buffers = [] + this.type = 0 + this.protocolError = false + this.offsetCache = 0 + // If returnBuffers is active, all return values are returned as buffers besides numbers and errors + if (options.return_buffers) { + this.handleReply = function (start, end) { + return this.buffer.slice(start, end) + } + } else { + this.handleReply = function (start, end) { + return this.buffer.toString('utf-8', start, end) + } + } + // If stringNumbers is activated the parser always returns numbers as string + // This is important for big numbers (number > Math.pow(2, 53)) as js numbers are 64bit floating point numbers with reduced precision + if (options.string_numbers) { + this.handleNumbers = function (start, end) { + return this.buffer.toString('ascii', start, end) + } + } else { + this.handleNumbers = function (start, end) { + return +this.buffer.toString('ascii', start, end) + } + } } JavascriptReplyParser.prototype.parseResult = function (type) { - var start = 0, - end = 0, - packetHeader = 0, - reply; + var start = 0 + var end = 0 + var packetHeader = 0 + var reply - if (type === 36) { // $ - packetHeader = this.parseHeader(); - // Packets with a size of -1 are considered null - if (packetHeader === -1) { - return null; - } - end = this.offset + packetHeader; - start = this.offset; - if (end + 2 > this.buffer.length) { - this.buffers.push(this.offsetCache === 0 ? this.buffer : this.buffer.slice(this.offsetCache)); - this.chunksSize = this.buffers[0].length; - // Include the packetHeader delimiter - this.bigStrSize = packetHeader + 2; - throw new Error('Wait for more data.'); - } - // Set the offset to after the delimiter - this.offset = end + 2; - return this.handleReply(start, end); - } else if (type === 58) { // : - // Up to the delimiter - end = this.packetEndOffset(); - start = this.offset; - // Include the delimiter - this.offset = end + 2; - // Return the coerced numeric value - return this.handleNumbers(start, end); - } else if (type === 43) { // + - end = this.packetEndOffset(); - start = this.offset; - this.offset = end + 2; - return this.handleReply(start, end); - } else if (type === 42) { // * - packetHeader = this.parseHeader(); - if (packetHeader === -1) { - return null; - } - reply = []; - for (var i = 0; i < packetHeader; i++) { - if (this.offset >= this.buffer.length) { - throw new Error('Wait for more data.'); - } - reply.push(this.parseResult(this.buffer[this.offset++])); - } - return reply; - } else if (type === 45) { // - - end = this.packetEndOffset(); - start = this.offset; - this.offset = end + 2; - return new Error(this.buffer.toString('utf-8', start, end)); - } -}; + if (type === 36) { // $ + packetHeader = this.parseHeader() + // Packets with a size of -1 are considered null + if (packetHeader === -1) { + return null + } + end = this.offset + packetHeader + start = this.offset + if (end + 2 > this.buffer.length) { + this.buffers.push(this.offsetCache === 0 ? this.buffer : this.buffer.slice(this.offsetCache)) + this.chunksSize = this.buffers[0].length + // Include the packetHeader delimiter + this.bigStrSize = packetHeader + 2 + throw new Error('Wait for more data.') + } + // Set the offset to after the delimiter + this.offset = end + 2 + return this.handleReply(start, end) + } else if (type === 58) { // : + // Up to the delimiter + end = this.packetEndOffset() + start = this.offset + // Include the delimiter + this.offset = end + 2 + // Return the coerced numeric value + return this.handleNumbers(start, end) + } else if (type === 43) { // + + end = this.packetEndOffset() + start = this.offset + this.offset = end + 2 + return this.handleReply(start, end) + } else if (type === 42) { // * + packetHeader = this.parseHeader() + if (packetHeader === -1) { + return null + } + reply = [] + for (var i = 0; i < packetHeader; i++) { + if (this.offset >= this.buffer.length) { + throw new Error('Wait for more data.') + } + reply.push(this.parseResult(this.buffer[this.offset++])) + } + return reply + } else if (type === 45) { // - + end = this.packetEndOffset() + start = this.offset + this.offset = end + 2 + return new Error(this.buffer.toString('utf-8', start, end)) + } +} JavascriptReplyParser.prototype.execute = function (buffer) { - if (this.chunksSize !== 0) { - if (this.bigStrSize > this.chunksSize + buffer.length) { - this.buffers.push(buffer); - this.chunksSize += buffer.length; - return; - } - this.buffers.push(buffer); - this.buffer = Buffer.concat(this.buffers, this.chunksSize + buffer.length); - this.buffers = []; - this.bigStrSize = 0; - this.chunksSize = 0; - } else if (this.offset >= this.buffer.length) { - this.buffer = buffer; - } else { - this.buffer = Buffer.concat([this.buffer.slice(this.offset), buffer]); - } - this.offset = 0; - this.run(); -}; + if (this.chunksSize !== 0) { + if (this.bigStrSize > this.chunksSize + buffer.length) { + this.buffers.push(buffer) + this.chunksSize += buffer.length + return + } + this.buffers.push(buffer) + this.buffer = Buffer.concat(this.buffers, this.chunksSize + buffer.length) + this.buffers = [] + this.bigStrSize = 0 + this.chunksSize = 0 + } else if (this.offset >= this.buffer.length) { + this.buffer = buffer + } else { + this.buffer = Buffer.concat([this.buffer.slice(this.offset), buffer]) + } + this.offset = 0 + this.run() +} JavascriptReplyParser.prototype.tryParsing = function () { - try { - return this.parseResult(this.type); - } catch (err) { - // Catch the error (not enough data), rewind if it's an array, - // and wait for the next packet to appear - this.offset = this.offsetCache; - // Indicate that there's no protocol error by resetting the type too - this.type = undefined; - } -}; + try { + return this.parseResult(this.type) + } catch (err) { + // Catch the error (not enough data), rewind if it's an array, + // and wait for the next packet to appear + this.offset = this.offsetCache + // Indicate that there's no protocol error by resetting the type too + this.type = undefined + } +} JavascriptReplyParser.prototype.run = function () { - // Set a rewind point. If a failure occurs, wait for the next execute()/append() and try again - this.offsetCache = this.offset; - this.type = this.buffer[this.offset++]; - var reply = this.tryParsing(); + // Set a rewind point. If a failure occurs, wait for the next execute()/append() and try again + this.offsetCache = this.offset + this.type = this.buffer[this.offset++] + var reply = this.tryParsing() - while (reply !== undefined) { - if (this.type === 45) { // Errors - - this.returnError(reply); - } else { - this.returnReply(reply); // Strings + // Integers : // Bulk strings $ // Arrays * - } - this.offsetCache = this.offset; - this.type = this.buffer[this.offset++]; - reply = this.tryParsing(); - } - if (this.type !== undefined) { - // Reset the buffer so the parser can handle following commands properly - this.buffer = new Buffer(0); - this.returnFatalError(new Error('Protocol error, got ' + JSON.stringify(String.fromCharCode(this.type)) + ' as reply type byte')); - } -}; + while (reply !== undefined) { + if (this.type === 45) { // Errors - + this.returnError(reply) + } else { + this.returnReply(reply) // Strings + // Integers : // Bulk strings $ // Arrays * + } + this.offsetCache = this.offset + this.type = this.buffer[this.offset++] + reply = this.tryParsing() + } + if (this.type !== undefined) { + // Reset the buffer so the parser can handle following commands properly + this.buffer = new Buffer(0) + this.returnFatalError(new Error('Protocol error, got ' + JSON.stringify(String.fromCharCode(this.type)) + ' as reply type byte')) + } +} JavascriptReplyParser.prototype.parseHeader = function () { - var end = this.packetEndOffset(), - value = this.buffer.toString('ascii', this.offset, end) | 0; + var end = this.packetEndOffset() + var value = this.buffer.toString('ascii', this.offset, end) | 0 - this.offset = end + 2; - return value; -}; + this.offset = end + 2 + return value +} JavascriptReplyParser.prototype.packetEndOffset = function () { - var offset = this.offset, - len = this.buffer.length - 1; + var offset = this.offset + var len = this.buffer.length - 1 - while (this.buffer[offset] !== 0x0d && this.buffer[offset + 1] !== 0x0a) { - offset++; + while (this.buffer[offset] !== 0x0d && this.buffer[offset + 1] !== 0x0a) { + offset++ - if (offset >= len) { - throw new Error('Did not see LF after NL reading multi bulk count (' + offset + ' => ' + this.buffer.length + ', ' + this.offset + ')'); - } - } - return offset; -}; + if (offset >= len) { + throw new Error('Did not see LF after NL reading multi bulk count (' + offset + ' => ' + this.buffer.length + ', ' + this.offset + ')') + } + } + return offset +} -module.exports = JavascriptReplyParser; \ No newline at end of file +module.exports = JavascriptReplyParser diff --git a/benchmark/old/parser.js b/benchmark/old/parser.js index f176ba6..e09cba2 100644 --- a/benchmark/old/parser.js +++ b/benchmark/old/parser.js @@ -1,59 +1,59 @@ -'use strict'; +'use strict' var parsers = { - javascript: require('./javascript') -}; + Javascript: require('./javascript') +} // Hiredis might not be installed try { - parsers.hiredis = require('./hiredis'); + parsers.Hiredis = require('./hiredis') } catch (err) { /* ignore errors */ } function Parser (options) { - var parser, msg; - - if ( - !options || - typeof options.returnError !== 'function' || - typeof options.returnReply !== 'function' - ) { - throw new Error('Please provide all return functions while initiating the parser'); - } - - if (options.name === 'hiredis') { - /* istanbul ignore if: hiredis should always be installed while testing */ - if (!parsers.hiredis) { - msg = 'You explicitly required the hiredis parser but hiredis is not installed. The JS parser is going to be returned instead.'; - } else if (options.stringNumbers) { - msg = 'You explicitly required the hiredis parser in combination with the stringNumbers option. Only the JS parser can handle that and is choosen instead.'; - } - } else if (options.name && !parsers[options.name] && options.name !== 'auto') { - msg = 'The requested parser "' + options.name + '" is unkown and the default parser is choosen instead.'; - } - - if (msg) { - console.warn(new Error(msg).stack.replace('Error: ', 'Warning: ')); - } - - options.name = options.name || 'hiredis'; - options.name = options.name.toLowerCase(); - - var innerOptions = { - // The hiredis parser expects underscores - return_buffers: options.returnBuffers || false, - string_numbers: options.stringNumbers || false - }; - - if (options.name === 'javascript' || !parsers.hiredis || options.stringNumbers) { - parser = new parsers.javascript(innerOptions); - } else { - parser = new parsers.hiredis(innerOptions); - } - - parser.returnError = options.returnError; - parser.returnFatalError = options.returnFatalError || options.returnError; - parser.returnReply = options.returnReply; - return parser; + var parser, msg + + if ( + !options || + typeof options.returnError !== 'function' || + typeof options.returnReply !== 'function' + ) { + throw new Error('Please provide all return functions while initiating the parser') + } + + if (options.name === 'hiredis') { + /* istanbul ignore if: hiredis should always be installed while testing */ + if (!parsers.hiredis) { + msg = 'You explicitly required the hiredis parser but hiredis is not installed. The JS parser is going to be returned instead.' + } else if (options.stringNumbers) { + msg = 'You explicitly required the hiredis parser in combination with the stringNumbers option. Only the JS parser can handle that and is choosen instead.' + } + } else if (options.name && !parsers[options.name] && options.name !== 'auto') { + msg = 'The requested parser "' + options.name + '" is unkown and the default parser is choosen instead.' + } + + if (msg) { + console.warn(new Error(msg).stack.replace('Error: ', 'Warning: ')) + } + + options.name = options.name || 'hiredis' + options.name = options.name.toLowerCase() + + var innerOptions = { + // The hiredis parser expects underscores + return_buffers: options.returnBuffers || false, + string_numbers: options.stringNumbers || false + } + + if (options.name === 'javascript' || !parsers.hiredis || options.stringNumbers) { + parser = new parsers.Javascript(innerOptions) + } else { + parser = new parsers.Hiredis(innerOptions) + } + + parser.returnError = options.returnError + parser.returnFatalError = options.returnFatalError || options.returnError + parser.returnReply = options.returnReply + return parser } -module.exports = Parser; \ No newline at end of file +module.exports = Parser diff --git a/index.js b/index.js index 4ecb075..918a9fe 100644 --- a/index.js +++ b/index.js @@ -1,4 +1,4 @@ -'use strict'; +'use strict' -module.exports = require('./lib/parser'); -module.exports.ReplyError = require('./lib/replyError'); +module.exports = require('./lib/parser') +module.exports.ReplyError = require('./lib/replyError') diff --git a/lib/parser.js b/lib/parser.js index 5d47792..7368635 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -1,30 +1,30 @@ -'use strict'; +'use strict' -var ReplyError = require('./replyError'); +var ReplyError = require('./replyError') /** * Used for lengths only, faster perf on arrays / bulks * @param parser * @returns {*} */ -function parseSimpleString(parser) { - var offset = parser.offset; - var length = parser.buffer.length; - var string = ''; +function parseSimpleString (parser) { + var offset = parser.offset + var length = parser.buffer.length + var string = '' - while (offset < length) { - var c1 = parser.buffer[offset++]; - if (c1 === 13) { - var c2 = parser.buffer[offset++]; - if (c2 === 10) { - parser.offset = offset; - return string; - } - string += String.fromCharCode(c1) + String.fromCharCode(c2); - continue; - } - string += String.fromCharCode(c1); - } + while (offset < length) { + var c1 = parser.buffer[offset++] + if (c1 === 13) { + var c2 = parser.buffer[offset++] + if (c2 === 10) { + parser.offset = offset + return string + } + string += String.fromCharCode(c1) + String.fromCharCode(c2) + continue + } + string += String.fromCharCode(c1) + } } /** @@ -35,13 +35,13 @@ function parseSimpleString(parser) { * @param end * @returns {*} */ -function convertBufferRange(parser, start, end) { - // If returnBuffers is active, all return values are returned as buffers besides numbers and errors - if (parser.optionReturnBuffers === true) { - return parser.buffer.slice(start, end); - } +function convertBufferRange (parser, start, end) { + // If returnBuffers is active, all return values are returned as buffers besides numbers and errors + if (parser.optionReturnBuffers === true) { + return parser.buffer.slice(start, end) + } - return parser.buffer.toString('utf-8', start, end); + return parser.buffer.toString('utf-8', start, end) } /** @@ -50,18 +50,18 @@ function convertBufferRange(parser, start, end) { * @param parser * @returns {*} */ -function parseSimpleStringViaOffset(parser) { - var start = parser.offset; - var offset = parser.offset; - var length = parser.buffer.length; +function parseSimpleStringViaOffset (parser) { + var start = parser.offset + var offset = parser.offset + var length = parser.buffer.length - while (offset < length) { - var c1 = parser.buffer[offset++]; - if (c1 === 13) { // \r // Finding a \r is sufficient here, since in a simple string \r is always followed by \n - parser.offset = offset + 1; - return convertBufferRange(parser, start, offset - 1); - } - } + while (offset < length) { + var c1 = parser.buffer[offset++] + if (c1 === 13) { // \r // Finding a \r is sufficient here, since in a simple string \r is always followed by \n + parser.offset = offset + 1 + return convertBufferRange(parser, start, offset - 1) + } + } } /** @@ -69,15 +69,15 @@ function parseSimpleStringViaOffset(parser) { * @param parser * @returns {*} */ -function parseLength(parser) { - var string = parseSimpleString(parser); - if (string !== undefined) { - var length = +string; - if (length === -1) { - return null; - } - return length; - } +function parseLength (parser) { + var string = parseSimpleString(parser) + if (string !== undefined) { + var length = +string + if (length === -1) { + return null + } + return length + } } /** @@ -85,17 +85,17 @@ function parseLength(parser) { * @param parser * @returns {*} */ -function parseInteger(parser) { - var string = parseSimpleString(parser); - if (string !== undefined) { - // If stringNumbers is activated the parser always returns numbers as string - // This is important for big numbers (number > Math.pow(2, 53)) as js numbers - // are 64bit floating point numbers with reduced precision - if (parser.optionStringNumbers === false) { - return +string; - } - return string; - } +function parseInteger (parser) { + var string = parseSimpleString(parser) + if (string !== undefined) { + // If stringNumbers is activated the parser always returns numbers as string + // This is important for big numbers (number > Math.pow(2, 53)) as js numbers + // are 64bit floating point numbers with reduced precision + if (parser.optionStringNumbers === false) { + return +string + } + return string + } } /** @@ -103,23 +103,23 @@ function parseInteger(parser) { * @param parser * @returns {*} */ -function parseBulkString(parser) { - var length = parseLength(parser); - if (length === null || length === undefined) { - return length; - } - var offsetEnd = parser.offset + length; - if (offsetEnd + 2 > parser.buffer.length) { - parser.bufferCache.push(parser.buffer); - parser.totalChunkSize = parser.buffer.length; - parser.bigStrSize = length + 2; - return; - } +function parseBulkString (parser) { + var length = parseLength(parser) + if (length === null || length === undefined) { + return length + } + var offsetEnd = parser.offset + length + if (offsetEnd + 2 > parser.buffer.length) { + parser.bufferCache.push(parser.buffer) + parser.totalChunkSize = parser.buffer.length + parser.bigStrSize = length + 2 + return + } - var offsetBegin = parser.offset; - parser.offset = offsetEnd + 2; + var offsetBegin = parser.offset + parser.offset = offsetEnd + 2 - return convertBufferRange(parser, offsetBegin, offsetEnd); + return convertBufferRange(parser, offsetBegin, offsetEnd) } /** @@ -127,11 +127,11 @@ function parseBulkString(parser) { * @param parser * @returns {Error} */ -function parseError(parser) { - var str = parseSimpleString(parser); - if (str !== undefined) { - return new ReplyError(str); - } +function parseError (parser) { + var str = parseSimpleString(parser) + if (str !== undefined) { + return new ReplyError(str) + } } /** @@ -139,9 +139,9 @@ function parseError(parser) { * @param parser * @param error */ -function handleError(parser, error) { - parser.buffer = null; - parser.returnFatalError(error); +function handleError (parser, error) { + parser.buffer = null + parser.returnFatalError(error) } /** @@ -149,26 +149,26 @@ function handleError(parser, error) { * @param parser * @returns {*} */ -function parseArray(parser) { - var length = parseLength(parser); - if (length === null || length === undefined) { - return length; - } +function parseArray (parser) { + var length = parseLength(parser) + if (length === null || length === undefined) { + return length + } - var responses = new Array(length); - var bufferLength = parser.buffer.length; - for (var i = 0; i < length; i++) { - if (parser.offset >= bufferLength) { - return; - } - var response = parseType(parser, parser.buffer[parser.offset++]); - if (response === undefined) { - return; - } - responses[i] = response; - } + var responses = new Array(length) + var bufferLength = parser.buffer.length + for (var i = 0; i < length; i++) { + if (parser.offset >= bufferLength) { + return + } + var response = parseType(parser, parser.buffer[parser.offset++]) + if (response === undefined) { + return + } + responses[i] = response + } - return responses; + return responses } /** @@ -178,21 +178,21 @@ function parseArray(parser) { * @returns {*} */ -function parseType(parser, type) { - switch (type) { - case 36: // $ - return parseBulkString(parser); - case 58: // : - return parseInteger(parser); - case 43: // + - return parser.optionReturnBuffers ? parseSimpleStringViaOffset(parser) : parseSimpleString(parser); - case 42: // * - return parseArray(parser); - case 45: // - - return parseError(parser); - default: - return handleError(parser, new ReplyError('Protocol error, got ' + JSON.stringify(String.fromCharCode(type)) + ' as reply type byte')); - } +function parseType (parser, type) { + switch (type) { + case 36: // $ + return parseBulkString(parser) + case 58: // : + return parseInteger(parser) + case 43: // + + return parser.optionReturnBuffers ? parseSimpleStringViaOffset(parser) : parseSimpleString(parser) + case 42: // * + return parseArray(parser) + case 45: // - + return parseError(parser) + default: + return handleError(parser, new ReplyError('Protocol error, got ' + JSON.stringify(String.fromCharCode(type)) + ' as reply type byte')) + } } /** @@ -200,34 +200,34 @@ function parseType(parser, type) { * @param options * @constructor */ -function JavascriptRedisParser(options) { - if (!(this instanceof JavascriptRedisParser)) { - return new JavascriptRedisParser(options); - } - if ( - !options || - typeof options.returnError !== 'function' || - typeof options.returnReply !== 'function' - ) { - throw new TypeError('Please provide all return functions while initiating the parser'); - } - if (options.name === 'hiredis') { - try { - var hiredis = require('../test/hiredis'); - return new hiredis(options); - } catch (e) {} - } - this.optionReturnBuffers = !!options.returnBuffers; - this.optionStringNumbers = !!options.stringNumbers; - this.returnError = options.returnError; - this.returnFatalError = options.returnFatalError || options.returnError; - this.returnReply = options.returnReply; - this.name = 'javascript'; - this.offset = 0; - this.buffer = null; - this.bigStrSize = 0; - this.totalChunkSize = 0; - this.bufferCache = []; +function JavascriptRedisParser (options) { + if (!(this instanceof JavascriptRedisParser)) { + return new JavascriptRedisParser(options) + } + if ( + !options || + typeof options.returnError !== 'function' || + typeof options.returnReply !== 'function' + ) { + throw new TypeError('Please provide all return functions while initiating the parser') + } + if (options.name === 'hiredis') { + try { + var Hiredis = require('../test/hiredis') + return new Hiredis(options) + } catch (e) {} + } + this.optionReturnBuffers = !!options.returnBuffers + this.optionStringNumbers = !!options.stringNumbers + this.returnError = options.returnError + this.returnFatalError = options.returnFatalError || options.returnError + this.returnReply = options.returnReply + this.name = 'javascript' + this.offset = 0 + this.buffer = null + this.bigStrSize = 0 + this.totalChunkSize = 0 + this.bufferCache = [] } /** @@ -235,53 +235,53 @@ function JavascriptRedisParser(options) { * @param buffer */ JavascriptRedisParser.prototype.execute = function (buffer) { - if (this.buffer === null) { - this.buffer = buffer; - this.offset = 0; - } else if (this.bigStrSize === 0) { - var oldLength = this.buffer.length; - var remainingLength = oldLength - this.offset; - var newLength = remainingLength + buffer.length; - var newBuffer = new Buffer(newLength); - this.buffer.copy(newBuffer, 0, this.offset, oldLength); - buffer.copy(newBuffer, remainingLength, 0, buffer.length); - this.buffer = newBuffer; - this.offset = 0; - } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { - this.bufferCache.push(buffer); - if (this.offset !== 0) { - this.bufferCache[0] = this.bufferCache[0].slice(this.offset); - } - this.buffer = Buffer.concat(this.bufferCache, this.totalChunkSize + buffer.length - this.offset); - this.bigStrSize = 0; - this.totalChunkSize = 0; - this.bufferCache = []; - this.offset = 0; - } else { - this.bufferCache.push(buffer); - this.totalChunkSize += buffer.length; - return; - } + if (this.buffer === null) { + this.buffer = buffer + this.offset = 0 + } else if (this.bigStrSize === 0) { + var oldLength = this.buffer.length + var remainingLength = oldLength - this.offset + var newLength = remainingLength + buffer.length + var newBuffer = new Buffer(newLength) + this.buffer.copy(newBuffer, 0, this.offset, oldLength) + buffer.copy(newBuffer, remainingLength, 0, buffer.length) + this.buffer = newBuffer + this.offset = 0 + } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { + this.bufferCache.push(buffer) + if (this.offset !== 0) { + this.bufferCache[0] = this.bufferCache[0].slice(this.offset) + } + this.buffer = Buffer.concat(this.bufferCache, this.totalChunkSize + buffer.length - this.offset) + this.bigStrSize = 0 + this.totalChunkSize = 0 + this.bufferCache = [] + this.offset = 0 + } else { + this.bufferCache.push(buffer) + this.totalChunkSize += buffer.length + return + } - var length = this.buffer.length; + var length = this.buffer.length - while (this.offset < length) { - var offset = this.offset; - var type = this.buffer[this.offset++]; - var response = parseType(this, type); - if (response === undefined) { - this.offset = offset; - return; - } + while (this.offset < length) { + var offset = this.offset + var type = this.buffer[this.offset++] + var response = parseType(this, type) + if (response === undefined) { + this.offset = offset + return + } - if (type === 45) { - this.returnError(response); // Errors - - } else { - this.returnReply(response); // Strings + // Integers : // Bulk strings $ // Arrays * - } - } + if (type === 45) { + this.returnError(response) // Errors - + } else { + this.returnReply(response) // Strings + // Integers : // Bulk strings $ // Arrays * + } + } - this.buffer = null; -}; + this.buffer = null +} -module.exports = JavascriptRedisParser; +module.exports = JavascriptRedisParser diff --git a/lib/replyError.js b/lib/replyError.js index c88290b..e9e4c33 100644 --- a/lib/replyError.js +++ b/lib/replyError.js @@ -1,26 +1,26 @@ -'use strict'; +'use strict' -var util = require('util'); +var util = require('util') function ReplyError (message) { - var limit = Error.stackTraceLimit; - Error.stackTraceLimit = 2; - Error.captureStackTrace(this, this.constructor); - Error.stackTraceLimit = limit; - Object.defineProperty(this, 'name', { - value: 'ReplyError', - configurable: false, - enumerable: false, - writable: true - }); - Object.defineProperty(this, 'message', { - value: message || '', - configurable: false, - enumerable: false, - writable: true - }); + var limit = Error.stackTraceLimit + Error.stackTraceLimit = 2 + Error.captureStackTrace(this, this.constructor) + Error.stackTraceLimit = limit + Object.defineProperty(this, 'name', { + value: 'ReplyError', + configurable: false, + enumerable: false, + writable: true + }) + Object.defineProperty(this, 'message', { + value: message || '', + configurable: false, + enumerable: false, + writable: true + }) } -util.inherits(ReplyError, Error); +util.inherits(ReplyError, Error) -module.exports = ReplyError; +module.exports = ReplyError diff --git a/package.json b/package.json index 13ec435..09cd44c 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "scripts": { "test": "mocha", "benchmark": "node ./benchmark", - "posttest": "jshint . && npm run coverage && npm run coverage:check", + "posttest": "standard && npm run coverage && npm run coverage:check", "coverage": "node ./node_modules/istanbul/lib/cli.js cover --preserve-comments ./node_modules/mocha/bin/_mocha -- -R spec", "coverage:check": "node ./node_modules/istanbul/lib/cli.js check-coverage --branch 100 --statement 100" }, @@ -33,10 +33,8 @@ "codeclimate-test-reporter": "^0.3.1", "intercept-stdout": "^0.1.2", "istanbul": "^0.4.0", - "jshint": "^2.8.0", - "mocha": "^2.3.2" - }, - "optionalDependency": { + "standard": "^7.0.1", + "mocha": "^2.3.2", "hiredis": "^0.4.1" }, "author": "Ruben Bridgewater", diff --git a/test/hiredis.js b/test/hiredis.js index c14d2a8..3cf7b53 100644 --- a/test/hiredis.js +++ b/test/hiredis.js @@ -1,7 +1,7 @@ -'use strict'; +'use strict' -var hiredis = require('hiredis'); -var ReplyError = require('../lib/replyError'); +var hiredis = require('hiredis') +var ReplyError = require('../lib/replyError') /** * Parse data @@ -9,14 +9,14 @@ var ReplyError = require('../lib/replyError'); * @returns {*} */ function parseData (parser) { - try { - return parser.reader.get(); - } catch (err) { - // Protocol errors land here - // Reset the parser. Otherwise new commands can't be processed properly - parser.reader = new hiredis.Reader(parser.options); - parser.returnFatalError(new ReplyError(err.message)); - } + try { + return parser.reader.get() + } catch (err) { + // Protocol errors land here + // Reset the parser. Otherwise new commands can't be processed properly + parser.reader = new hiredis.Reader(parser.options) + parser.returnFatalError(new ReplyError(err.message)) + } } /** @@ -25,31 +25,31 @@ function parseData (parser) { * @constructor */ function HiredisReplyParser (options) { - if (!(this instanceof HiredisReplyParser)) { - return new HiredisReplyParser(options); - } - this.returnError = options.returnError; - this.returnFatalError = options.returnFatalError || options.returnError; - this.returnReply = options.returnReply; - this.name = 'hiredis'; - this.options = { - return_buffers: !!options.returnBuffers - }; - this.reader = new hiredis.Reader(this.options); + if (!(this instanceof HiredisReplyParser)) { + return new HiredisReplyParser(options) + } + this.returnError = options.returnError + this.returnFatalError = options.returnFatalError || options.returnError + this.returnReply = options.returnReply + this.name = 'hiredis' + this.options = { + return_buffers: !!options.returnBuffers + } + this.reader = new hiredis.Reader(this.options) } HiredisReplyParser.prototype.execute = function (data) { - this.reader.feed(data); - var reply = parseData(this); + this.reader.feed(data) + var reply = parseData(this) - while (reply !== undefined) { - if (reply && reply.name === 'Error') { - this.returnError(new ReplyError(reply.message)); - } else { - this.returnReply(reply); - } - reply = parseData(this); - } + while (reply !== undefined) { + if (reply && reply.name === 'Error') { + this.returnError(new ReplyError(reply.message)) + } else { + this.returnReply(reply) + } + reply = parseData(this) + } } -module.exports = HiredisReplyParser; +module.exports = HiredisReplyParser diff --git a/test/parsers.spec.js b/test/parsers.spec.js index a1add74..836f19f 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -1,437 +1,432 @@ -'use strict'; +'use strict' -var intercept = require('intercept-stdout'); -var assert = require('assert'); -var JavascriptParser = require('../'); -var HiredisParser = require('./hiredis'); -var ReplyError = JavascriptParser.ReplyError; -var parsers = [JavascriptParser, HiredisParser]; +/* eslint-env mocha */ + +var intercept = require('intercept-stdout') +var assert = require('assert') +var JavascriptParser = require('../') +var HiredisParser = require('./hiredis') +var ReplyError = JavascriptParser.ReplyError +var parsers = [JavascriptParser, HiredisParser] // Mock the not needed return functions -function returnReply () { throw new Error('failed'); } -function returnError () { throw new Error('failed'); } -function returnFatalError () { throw new Error('failed'); } +function returnReply () { throw new Error('failed') } +function returnError () { throw new Error('failed') } +function returnFatalError () { throw new Error('failed') } describe('parsers', function () { - - describe('general parser functionality', function () { - - it('backwards compatibility with hiredis', function () { - var parser = new JavascriptParser({ - returnReply: returnReply, - returnError: returnError, - name: 'hiredis' - }); - assert.strictEqual(parser.name, 'hiredis'); - }); - - it('fail for missing options', function () { - assert.throws(function() { - new JavascriptParser({ - returnReply: returnReply, - returnBuffers: true, - name: 'hiredis' - }); - }, function (err) { - assert.strictEqual(err.message, 'Please provide all return functions while initiating the parser'); - return true; - }); - - }); - - }); - - parsers.forEach(function (Parser) { - - describe(Parser.name, function () { - - it('handles multi-bulk reply and check context binding', function () { - var replyCount = 0; - function Abc () {} - Abc.prototype.checkReply = function (reply) { - assert.strictEqual(typeof this.log, 'function'); - assert.deepEqual(reply, [['a']], 'Expecting multi-bulk reply of [["a"]]'); - replyCount++; - }; - Abc.prototype.log = console.log; - var test = new Abc(); - var parser = new Parser({ - returnReply: function (reply) { - test.checkReply(reply); - }, - returnError: returnError, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer('*1\r\n*1\r\n$1\r\na\r\n')); - assert.strictEqual(replyCount, 1); - - parser.execute(new Buffer('*1\r\n*1\r')); - parser.execute(new Buffer('\n$1\r\na\r\n')); - assert.strictEqual(replyCount, 2); - - parser.execute(new Buffer('*1\r\n*1\r\n')); - parser.execute(new Buffer('$1\r\na\r\n')); - - assert.equal(replyCount, 3, 'check reply should have been called three times'); - }); - - it('parser error', function () { - var replyCount = 0; - function Abc () {} - Abc.prototype.checkReply = function (err) { - assert.strictEqual(typeof this.log, 'function'); - assert.strictEqual(err.message, 'Protocol error, got "a" as reply type byte'); - assert.strictEqual(err.name, 'ReplyError'); - assert(err instanceof ReplyError); - assert(err instanceof Error); - replyCount++; - }; - Abc.prototype.log = console.log; - var test = new Abc(); - var parser = new Parser({ - returnReply: returnReply, - returnError: returnError, - returnFatalError: function (err) { - test.checkReply(err); - } - }); - - parser.execute(new Buffer('a*1\r*1\r$1`zasd\r\na')); - assert.equal(replyCount, 1); - }); - - it('parser error resets the buffer', function () { - var replyCount = 0; - var errCount = 0; - function checkReply (reply) { - assert.strictEqual(reply.length, 1); - assert(Buffer.isBuffer(reply[0])); - assert.strictEqual(reply[0].toString(), 'CCC'); - replyCount++; - } - function checkError (err) { - assert.strictEqual(err.message, 'Protocol error, got "b" as reply type byte'); - errCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: checkError, - returnBuffers: true - }); - - // The chunk contains valid data after the protocol error - parser.execute(new Buffer('*1\r\n+CCC\r\nb$1\r\nz\r\n+abc\r\n')); - assert.strictEqual(replyCount, 1); - assert.strictEqual(errCount, 1); - parser.execute(new Buffer('*1\r\n+CCC\r\n')); - assert.strictEqual(replyCount, 2); - }); - - it('parser error v3 without returnFatalError specified', function () { - var replyCount = 0; - var errCount = 0; - function checkReply (reply) { - assert.strictEqual(reply[0], 'OK'); - replyCount++; - } - function checkError (err) { - assert.strictEqual(err.message, 'Protocol error, got "\\n" as reply type byte'); - errCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: checkError - }); - - parser.execute(new Buffer('*1\r\n+OK\r\n\n+zasd\r\n')); - assert.strictEqual(replyCount, 1); - assert.strictEqual(errCount, 1); - }); - - it('should handle \\r and \\n characters properly', function () { - // If a string contains \r or \n characters it will always be send as a bulk string - var replyCount = 0; - var entries = ['foo\r', 'foo\r\nbar', '\r\nfoo', 'foo\r\n']; - function checkReply (reply) { - assert.strictEqual(reply, entries[replyCount]); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer('$4\r\nfoo\r\r\n$8\r\nfoo\r\nbar\r\n$5\r\n\r\n')); - assert.strictEqual(replyCount, 2); - parser.execute(new Buffer('foo\r\n$5\r\nfoo\r\n\r\n')); - assert.strictEqual(replyCount, 4); - }); - - it('line breaks in the beginning of the last chunk', function () { - var replyCount = 0; - function checkReply(reply) { - assert.deepEqual(reply, [['a']], 'Expecting multi-bulk reply of [["a"]]'); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer('*1\r\n*1\r\n$1\r\na')); - assert.equal(replyCount, 0); - - parser.execute(new Buffer('\r\n*1\r\n*1\r')); - assert.equal(replyCount, 1); - parser.execute(new Buffer('\n$1\r\na\r\n*1\r\n*1\r\n$1\r\na\r\n')); - - assert.equal(replyCount, 3, 'check reply should have been called three times'); - }); - - it('multiple chunks in a bulk string', function () { - var replyCount = 0; - function checkReply(reply) { - assert.strictEqual(reply, 'abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij'); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer('$100\r\nabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); - assert.strictEqual(replyCount, 0); - parser.execute(new Buffer('\r\n')); - assert.strictEqual(replyCount, 1); - - parser.execute(new Buffer('$100\r')); - parser.execute(new Buffer('\nabcdefghijabcdefghijabcdefghijabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghij')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer( - 'abcdefghij\r\n' + - '$100\r\nabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij\r\n' + - '$100\r\nabcdefghijabcdefghijabcdefghijabcdefghij' - )); - assert.strictEqual(replyCount, 3); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij\r')); - assert.strictEqual(replyCount, 3); - parser.execute(new Buffer('\n')); - - assert.equal(replyCount, 4, 'check reply should have been called three times'); - }); - - it('multiple chunks with arrays different types', function () { - var replyCount = 0; - var predefined_data = [ - 'abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij', - 'test', - 100, - new ReplyError('Error message'), - ['The force awakens'], - new ReplyError() - ]; - function checkReply(reply) { - for (var i = 0; i < reply.length; i++) { - if (Array.isArray(reply[i])) { - reply[i].forEach(function (reply, j) { - assert.strictEqual(reply, predefined_data[i][j]); - }); - } else if (reply[i] instanceof Error) { - if (Parser.name !== 'HiredisReplyParser') { // The hiredis always returns normal errors in case of nested ones - assert(reply[i] instanceof ReplyError); - assert.strictEqual(reply[i].name, predefined_data[i].name); - assert.deepStrictEqual(reply[i], predefined_data[i]); - } - assert.strictEqual(reply[i].message, predefined_data[i].message); - } else { - assert.strictEqual(reply[i], predefined_data[i]); - } - } - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError, - returnBuffers: false - }); - - parser.execute(new Buffer('*6\r\n$100\r\nabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')); - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij\r\n')); - parser.execute(new Buffer('+test\r')); - parser.execute(new Buffer('\n:100')); - parser.execute(new Buffer('\r\n-Error message')); - parser.execute(new Buffer('\r\n*1\r\n$17\r\nThe force')); - assert.strictEqual(replyCount, 0); - parser.execute(new Buffer(' awakens\r\n-\r\n$5')); - assert.strictEqual(replyCount, 1); - }); - - it('return normal errors', function () { - var replyCount = 0; - function checkReply(reply) { - assert.equal(reply.message, 'Error message'); - replyCount++; - } - var parser = new Parser({ - returnReply: returnError, - returnError: checkReply, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer('-Error ')); - parser.execute(new Buffer('message\r\n*3\r\n$17\r\nThe force')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer(' awakens\r\n$5')); - assert.strictEqual(replyCount, 1); - }); - - it('return null for empty arrays and empty bulk strings', function () { - var replyCount = 0; - function checkReply(reply) { - assert.equal(reply, null); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer('$-1\r\n*-')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer('1')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer('\r\n$-')); - assert.strictEqual(replyCount, 2); - }); - - it('return value even if all chunks are only 1 character long', function () { - var replyCount = 0; - function checkReply(reply) { - assert.equal(reply, 1); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer(':')); - assert.strictEqual(replyCount, 0); - parser.execute(new Buffer('1')); - assert.strictEqual(replyCount, 0); - parser.execute(new Buffer('\r')); - assert.strictEqual(replyCount, 0); - parser.execute(new Buffer('\n')); - assert.strictEqual(replyCount, 1); - }); - - it('do not return before \\r\\n', function () { - var replyCount = 0; - function checkReply(reply) { - assert.equal(reply, 1); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError - }); - - parser.execute(new Buffer(':1\r\n:')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer('1')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer('\r')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer('\n')); - assert.strictEqual(replyCount, 2); - }); - - it('return data as buffer if requested', function () { - var replyCount = 0; - function checkReply(reply) { - if (Array.isArray(reply)) { - reply = reply[0]; - } - assert(Buffer.isBuffer(reply)); - assert.strictEqual(reply.inspect(), new Buffer('test').inspect()); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError, - returnBuffers: true - }); - - parser.execute(new Buffer('+test\r\n')); - assert.strictEqual(replyCount, 1); - parser.execute(new Buffer('$4\r\ntest\r\n')); - assert.strictEqual(replyCount, 2); - parser.execute(new Buffer('*1\r\n$4\r\ntest\r\n')); - assert.strictEqual(replyCount, 3); - }); - - it('handle special case buffer sizes properly', function () { - var replyCount = 0; - var entries = ['test test ', 'test test test test ', 1234]; - function checkReply(reply) { - assert.strictEqual(reply, entries[replyCount]); - replyCount++; - } - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError - }); - parser.execute(new Buffer('$10\r\ntest ')); - assert.strictEqual(replyCount, 0); - parser.execute(new Buffer('test \r\n$20\r\ntest test test test \r\n:1234\r')); - assert.strictEqual(replyCount, 2); - parser.execute(new Buffer('\n')); - assert.strictEqual(replyCount, 3); - }); - - it('return numbers as strings', function () { - if (Parser.name === 'HiredisReplyParser') { - return this.skip(); - } - var replyCount = 0; - var entries = ['123', '590295810358705700002', '-99999999999999999']; - function checkReply(reply) { - assert.strictEqual(typeof reply, 'string'); - assert.strictEqual(reply, entries[replyCount]); - replyCount++; - } - var unhookIntercept = intercept(function () { - return ''; - }); - var parser = new Parser({ - returnReply: checkReply, - returnError: returnError, - returnFatalError: returnFatalError, - stringNumbers: true - }); - unhookIntercept(); - parser.execute(new Buffer(':123\r\n:590295810358705700002\r\n:-99999999999999999\r\n')); - assert.strictEqual(replyCount, 3); - }); - }); - }); -}); + describe('general parser functionality', function () { + it('backwards compatibility with hiredis', function () { + var parser = new JavascriptParser({ + returnReply: returnReply, + returnError: returnError, + name: 'hiredis' + }) + assert.strictEqual(parser.name, 'hiredis') + }) + + it('fail for missing options', function () { + assert.throws(function () { + JavascriptParser({ + returnReply: returnReply, + returnBuffers: true, + name: 'hiredis' + }) + }, function (err) { + assert.strictEqual(err.message, 'Please provide all return functions while initiating the parser') + return true + }) + }) + }) + + parsers.forEach(function (Parser) { + describe(Parser.name, function () { + it('handles multi-bulk reply and check context binding', function () { + var replyCount = 0 + function Abc () {} + Abc.prototype.checkReply = function (reply) { + assert.strictEqual(typeof this.log, 'function') + assert.deepEqual(reply, [['a']], 'Expecting multi-bulk reply of [["a"]]') + replyCount++ + } + Abc.prototype.log = console.log + var test = new Abc() + var parser = new Parser({ + returnReply: function (reply) { + test.checkReply(reply) + }, + returnError: returnError, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer('*1\r\n*1\r\n$1\r\na\r\n')) + assert.strictEqual(replyCount, 1) + + parser.execute(new Buffer('*1\r\n*1\r')) + parser.execute(new Buffer('\n$1\r\na\r\n')) + assert.strictEqual(replyCount, 2) + + parser.execute(new Buffer('*1\r\n*1\r\n')) + parser.execute(new Buffer('$1\r\na\r\n')) + + assert.equal(replyCount, 3, 'check reply should have been called three times') + }) + + it('parser error', function () { + var replyCount = 0 + function Abc () {} + Abc.prototype.checkReply = function (err) { + assert.strictEqual(typeof this.log, 'function') + assert.strictEqual(err.message, 'Protocol error, got "a" as reply type byte') + assert.strictEqual(err.name, 'ReplyError') + assert(err instanceof ReplyError) + assert(err instanceof Error) + replyCount++ + } + Abc.prototype.log = console.log + var test = new Abc() + var parser = new Parser({ + returnReply: returnReply, + returnError: returnError, + returnFatalError: function (err) { + test.checkReply(err) + } + }) + + parser.execute(new Buffer('a*1\r*1\r$1`zasd\r\na')) + assert.equal(replyCount, 1) + }) + + it('parser error resets the buffer', function () { + var replyCount = 0 + var errCount = 0 + function checkReply (reply) { + assert.strictEqual(reply.length, 1) + assert(Buffer.isBuffer(reply[0])) + assert.strictEqual(reply[0].toString(), 'CCC') + replyCount++ + } + function checkError (err) { + assert.strictEqual(err.message, 'Protocol error, got "b" as reply type byte') + errCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: checkError, + returnBuffers: true + }) + + // The chunk contains valid data after the protocol error + parser.execute(new Buffer('*1\r\n+CCC\r\nb$1\r\nz\r\n+abc\r\n')) + assert.strictEqual(replyCount, 1) + assert.strictEqual(errCount, 1) + parser.execute(new Buffer('*1\r\n+CCC\r\n')) + assert.strictEqual(replyCount, 2) + }) + + it('parser error v3 without returnFatalError specified', function () { + var replyCount = 0 + var errCount = 0 + function checkReply (reply) { + assert.strictEqual(reply[0], 'OK') + replyCount++ + } + function checkError (err) { + assert.strictEqual(err.message, 'Protocol error, got "\\n" as reply type byte') + errCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: checkError + }) + + parser.execute(new Buffer('*1\r\n+OK\r\n\n+zasd\r\n')) + assert.strictEqual(replyCount, 1) + assert.strictEqual(errCount, 1) + }) + + it('should handle \\r and \\n characters properly', function () { + // If a string contains \r or \n characters it will always be send as a bulk string + var replyCount = 0 + var entries = ['foo\r', 'foo\r\nbar', '\r\nfoo', 'foo\r\n'] + function checkReply (reply) { + assert.strictEqual(reply, entries[replyCount]) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer('$4\r\nfoo\r\r\n$8\r\nfoo\r\nbar\r\n$5\r\n\r\n')) + assert.strictEqual(replyCount, 2) + parser.execute(new Buffer('foo\r\n$5\r\nfoo\r\n\r\n')) + assert.strictEqual(replyCount, 4) + }) + + it('line breaks in the beginning of the last chunk', function () { + var replyCount = 0 + function checkReply (reply) { + assert.deepEqual(reply, [['a']], 'Expecting multi-bulk reply of [["a"]]') + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer('*1\r\n*1\r\n$1\r\na')) + assert.equal(replyCount, 0) + + parser.execute(new Buffer('\r\n*1\r\n*1\r')) + assert.equal(replyCount, 1) + parser.execute(new Buffer('\n$1\r\na\r\n*1\r\n*1\r\n$1\r\na\r\n')) + + assert.equal(replyCount, 3, 'check reply should have been called three times') + }) + + it('multiple chunks in a bulk string', function () { + var replyCount = 0 + function checkReply (reply) { + assert.strictEqual(reply, 'abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij') + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer('$100\r\nabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('\r\n')) + assert.strictEqual(replyCount, 1) + + parser.execute(new Buffer('$100\r')) + parser.execute(new Buffer('\nabcdefghijabcdefghijabcdefghijabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghij')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer( + 'abcdefghij\r\n' + + '$100\r\nabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij\r\n' + + '$100\r\nabcdefghijabcdefghijabcdefghijabcdefghij' + )) + assert.strictEqual(replyCount, 3) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij\r')) + assert.strictEqual(replyCount, 3) + parser.execute(new Buffer('\n')) + + assert.equal(replyCount, 4, 'check reply should have been called three times') + }) + + it('multiple chunks with arrays different types', function () { + var replyCount = 0 + var predefinedData = [ + 'abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij', + 'test', + 100, + new ReplyError('Error message'), + ['The force awakens'], + new ReplyError() + ] + function checkReply (reply) { + for (var i = 0; i < reply.length; i++) { + if (Array.isArray(reply[i])) { + reply[i].forEach(function (reply, j) { + assert.strictEqual(reply, predefinedData[i][j]) + }) + } else if (reply[i] instanceof Error) { + if (Parser.name !== 'HiredisReplyParser') { // The hiredis always returns normal errors in case of nested ones + assert(reply[i] instanceof ReplyError) + assert.strictEqual(reply[i].name, predefinedData[i].name) + } + assert.strictEqual(reply[i].message, predefinedData[i].message) + } else { + assert.strictEqual(reply[i], predefinedData[i]) + } + } + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError, + returnBuffers: false + }) + + parser.execute(new Buffer('*6\r\n$100\r\nabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) + parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij\r\n')) + parser.execute(new Buffer('+test\r')) + parser.execute(new Buffer('\n:100')) + parser.execute(new Buffer('\r\n-Error message')) + parser.execute(new Buffer('\r\n*1\r\n$17\r\nThe force')) + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer(' awakens\r\n-\r\n$5')) + assert.strictEqual(replyCount, 1) + }) + + it('return normal errors', function () { + var replyCount = 0 + function checkReply (reply) { + assert.equal(reply.message, 'Error message') + replyCount++ + } + var parser = new Parser({ + returnReply: returnError, + returnError: checkReply, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer('-Error ')) + parser.execute(new Buffer('message\r\n*3\r\n$17\r\nThe force')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer(' awakens\r\n$5')) + assert.strictEqual(replyCount, 1) + }) + + it('return null for empty arrays and empty bulk strings', function () { + var replyCount = 0 + function checkReply (reply) { + assert.equal(reply, null) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer('$-1\r\n*-')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('1')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('\r\n$-')) + assert.strictEqual(replyCount, 2) + }) + + it('return value even if all chunks are only 1 character long', function () { + var replyCount = 0 + function checkReply (reply) { + assert.equal(reply, 1) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer(':')) + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('1')) + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('\r')) + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('\n')) + assert.strictEqual(replyCount, 1) + }) + + it('do not return before \\r\\n', function () { + var replyCount = 0 + function checkReply (reply) { + assert.equal(reply, 1) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + + parser.execute(new Buffer(':1\r\n:')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('1')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('\r')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('\n')) + assert.strictEqual(replyCount, 2) + }) + + it('return data as buffer if requested', function () { + var replyCount = 0 + function checkReply (reply) { + if (Array.isArray(reply)) { + reply = reply[0] + } + assert(Buffer.isBuffer(reply)) + assert.strictEqual(reply.inspect(), new Buffer('test').inspect()) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError, + returnBuffers: true + }) + + parser.execute(new Buffer('+test\r\n')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('$4\r\ntest\r\n')) + assert.strictEqual(replyCount, 2) + parser.execute(new Buffer('*1\r\n$4\r\ntest\r\n')) + assert.strictEqual(replyCount, 3) + }) + + it('handle special case buffer sizes properly', function () { + var replyCount = 0 + var entries = ['test test ', 'test test test test ', 1234] + function checkReply (reply) { + assert.strictEqual(reply, entries[replyCount]) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + parser.execute(new Buffer('$10\r\ntest ')) + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('test \r\n$20\r\ntest test test test \r\n:1234\r')) + assert.strictEqual(replyCount, 2) + parser.execute(new Buffer('\n')) + assert.strictEqual(replyCount, 3) + }) + + it('return numbers as strings', function () { + if (Parser.name === 'HiredisReplyParser') { + return this.skip() + } + var replyCount = 0 + var entries = ['123', '590295810358705700002', '-99999999999999999'] + function checkReply (reply) { + assert.strictEqual(typeof reply, 'string') + assert.strictEqual(reply, entries[replyCount]) + replyCount++ + } + var unhookIntercept = intercept(function () { + return '' + }) + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError, + stringNumbers: true + }) + unhookIntercept() + parser.execute(new Buffer(':123\r\n:590295810358705700002\r\n:-99999999999999999\r\n')) + assert.strictEqual(replyCount, 3) + }) + }) + }) +}) From f6cd8ccf084fa2329bfa951d4cc2124070a05811 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 06:56:22 +0200 Subject: [PATCH 14/38] Warn in case of using the name option with hiredis set --- benchmark/index.js | 3 +-- benchmark/old/parser.js | 8 ++++---- lib/parser.js | 1 + 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/benchmark/index.js b/benchmark/index.js index abc0d9d..eea32d8 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -65,8 +65,7 @@ var parserHiRedis = new Parser({ var parser = new Parser({ returnReply: checkReply, returnError: returnError, - returnFatalError: returnError, - name: 'javascript' + returnFatalError: returnError }) // BULK STRINGS diff --git a/benchmark/old/parser.js b/benchmark/old/parser.js index e09cba2..e8ca306 100644 --- a/benchmark/old/parser.js +++ b/benchmark/old/parser.js @@ -1,12 +1,12 @@ 'use strict' var parsers = { - Javascript: require('./javascript') + javascript: require('./javascript') } // Hiredis might not be installed try { - parsers.Hiredis = require('./hiredis') + parsers.hiredis = require('./hiredis') } catch (err) { /* ignore errors */ } function Parser (options) { @@ -45,9 +45,9 @@ function Parser (options) { } if (options.name === 'javascript' || !parsers.hiredis || options.stringNumbers) { - parser = new parsers.Javascript(innerOptions) + parser = new parsers.javascript(innerOptions) // eslint-disable-line new-cap } else { - parser = new parsers.Hiredis(innerOptions) + parser = new parsers.hiredis(innerOptions) // eslint-disable-line new-cap } parser.returnError = options.returnError diff --git a/lib/parser.js b/lib/parser.js index 7368635..70d2b74 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -214,6 +214,7 @@ function JavascriptRedisParser (options) { if (options.name === 'hiredis') { try { var Hiredis = require('../test/hiredis') + console.error(new TypeError('Using the hiredis parser is discouraged. Please remove the name option.').stack) return new Hiredis(options) } catch (e) {} } From 16534dafcd44f6472bff6e542e71e33b5d1bd147 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 07:02:20 +0200 Subject: [PATCH 15/38] Fix jsdoc statement --- lib/parser.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/parser.js b/lib/parser.js index 70d2b74..fa147fe 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -145,7 +145,7 @@ function handleError (parser, error) { } /** - * Parse a + * Parse a '*' redis array response * @param parser * @returns {*} */ @@ -234,6 +234,7 @@ function JavascriptRedisParser (options) { /** * Parse the redis buffer * @param buffer + * @returns {undefined} */ JavascriptRedisParser.prototype.execute = function (buffer) { if (this.buffer === null) { From b245cd6350c07a819a9289742de300a9f20c867a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 07:11:35 +0200 Subject: [PATCH 16/38] Add support for NodeJS 6 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a7c67a9..e412156 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,7 @@ node_js: - "0.10" - "0.12" - "4" - - "5" + - "6" install: - npm install - npm install hiredis From 1a974c6e87c52ef059a6bdc87ef9260b0aa51f35 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 7 May 2016 17:48:10 +0200 Subject: [PATCH 17/38] Further optimizations Don't use `continue` Make sure the returned strings are always proper utf8 values Make sure the jit knows has as much information as possible Don't return a bulk string before `\n` is present Improve hiredis warnings --- lib/parser.js | 50 ++++++++++++++++++++++---------------------- test/parsers.spec.js | 6 +++++- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index fa147fe..ba8011e 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -3,7 +3,8 @@ var ReplyError = require('./replyError') /** - * Used for lengths only, faster perf on arrays / bulks + * Used for lengths and numbers only, faster perf on arrays / bulks + * Don't use this for strings, as fromCharCode returns utf-16 and breaks utf-8 compatibility * @param parser * @returns {*} */ @@ -14,14 +15,9 @@ function parseSimpleString (parser) { while (offset < length) { var c1 = parser.buffer[offset++] - if (c1 === 13) { - var c2 = parser.buffer[offset++] - if (c2 === 10) { - parser.offset = offset - return string - } - string += String.fromCharCode(c1) + String.fromCharCode(c2) - continue + if (c1 === 13 && parser.buffer[offset] === 10) { // \r\n + parser.offset = offset + 1 + return string } string += String.fromCharCode(c1) } @@ -57,7 +53,7 @@ function parseSimpleStringViaOffset (parser) { while (offset < length) { var c1 = parser.buffer[offset++] - if (c1 === 13) { // \r // Finding a \r is sufficient here, since in a simple string \r is always followed by \n + if (c1 === 13 && parser.buffer[offset] === 10) { // \r\n parser.offset = offset + 1 return convertBufferRange(parser, start, offset - 1) } @@ -72,11 +68,7 @@ function parseSimpleStringViaOffset (parser) { function parseLength (parser) { var string = parseSimpleString(parser) if (string !== undefined) { - var length = +string - if (length === -1) { - return null - } - return length + return +string } } @@ -105,8 +97,11 @@ function parseInteger (parser) { */ function parseBulkString (parser) { var length = parseLength(parser) - if (length === null || length === undefined) { - return length + if (length === undefined) { + return + } + if (length === -1) { + return null } var offsetEnd = parser.offset + length if (offsetEnd + 2 > parser.buffer.length) { @@ -128,9 +123,9 @@ function parseBulkString (parser) { * @returns {Error} */ function parseError (parser) { - var str = parseSimpleString(parser) - if (str !== undefined) { - return new ReplyError(str) + var string = parseSimpleStringViaOffset(parser) + if (string !== undefined) { + return new ReplyError(string) } } @@ -151,8 +146,11 @@ function handleError (parser, error) { */ function parseArray (parser) { var length = parseLength(parser) - if (length === null || length === undefined) { - return length + if (length === undefined) { + return + } + if (length === -1) { + return null } var responses = new Array(length) @@ -185,7 +183,7 @@ function parseType (parser, type) { case 58: // : return parseInteger(parser) case 43: // + - return parser.optionReturnBuffers ? parseSimpleStringViaOffset(parser) : parseSimpleString(parser) + return parseSimpleStringViaOffset(parser) case 42: // * return parseArray(parser) case 45: // - @@ -214,9 +212,11 @@ function JavascriptRedisParser (options) { if (options.name === 'hiredis') { try { var Hiredis = require('../test/hiredis') - console.error(new TypeError('Using the hiredis parser is discouraged. Please remove the name option.').stack) + console.error(new TypeError('Using the hiredis parser is discouraged. Please remove the name option.').stack.replace('Error', 'Warning')) return new Hiredis(options) - } catch (e) {} + } catch (e) { + console.error(new TypeError('Hiredis is not installed. Please remove the `name` option. The (faster) JS parser is used instead.').stack.replace('Error', 'Warning')) + } } this.optionReturnBuffers = !!options.returnBuffers this.optionStringNumbers = !!options.stringNumbers diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 836f19f..13045e6 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -149,7 +149,7 @@ describe('parsers', function () { it('should handle \\r and \\n characters properly', function () { // If a string contains \r or \n characters it will always be send as a bulk string var replyCount = 0 - var entries = ['foo\r', 'foo\r\nbar', '\r\nfoo', 'foo\r\n'] + var entries = ['foo\r', 'foo\r\nbar', '\r\nfoo', 'foo\r\n', 'foo'] function checkReply (reply) { assert.strictEqual(reply, entries[replyCount]) replyCount++ @@ -164,6 +164,10 @@ describe('parsers', function () { assert.strictEqual(replyCount, 2) parser.execute(new Buffer('foo\r\n$5\r\nfoo\r\n\r\n')) assert.strictEqual(replyCount, 4) + parser.execute(new Buffer('+foo\r')) + assert.strictEqual(replyCount, 4) + parser.execute(new Buffer('\n')) + assert.strictEqual(replyCount, 5) }) it('line breaks in the beginning of the last chunk', function () { From 7b76bfccce148c930e77f2cb3d593ce97a305613 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 8 May 2016 19:31:14 +0200 Subject: [PATCH 18/38] Add istanbul ignore statement for the hiredis warnings --- lib/parser.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/parser.js b/lib/parser.js index ba8011e..4512522 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -210,6 +210,7 @@ function JavascriptRedisParser (options) { throw new TypeError('Please provide all return functions while initiating the parser') } if (options.name === 'hiredis') { + /* istanbul ignore next: hiredis is only supported for legacy usage */ try { var Hiredis = require('../test/hiredis') console.error(new TypeError('Using the hiredis parser is discouraged. Please remove the name option.').stack.replace('Error', 'Warning')) From 3061e8591271b78a6bd002e39842850f612f5ecd Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 9 May 2016 07:48:50 +0200 Subject: [PATCH 19/38] Add big arrays benchmark --- benchmark/index.js | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/benchmark/index.js b/benchmark/index.js index eea32d8..804cdcb 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -42,12 +42,22 @@ var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long -var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n' + lorem) +var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n') var chunks = new Array(64) for (var i = 0; i < 64; i++) { chunks[i] = new Buffer(shuffle(bigStringArray).join(' ') + '.') // Math.pow(2, 16) chars long } +var bigArraySize = 100 +var bigArray = '*' + bigArraySize + '\r\n' +for (i = 0; i < bigArraySize; i++) { + bigArray += '$' + var size = (Math.random() * 10 | 0) + 1 + bigArray += size + '\r\n' + lorem.slice(0, size) + '\r\n' +} + +var bigArrayBuffer = new Buffer(bigArray) + var parserOld = new ParserOLD({ returnReply: checkReply, returnError: returnError, @@ -162,6 +172,20 @@ suite.add('NEW CODE: * array', function () { parser.execute(arrayBuffer) }) +// BIG ARRAYS + +suite.add('\nOLD CODE: * bigArray', function () { + parserOld.execute(bigArrayBuffer) +}) + +suite.add('HIREDIS: * bigArray', function () { + parserHiRedis.execute(bigArrayBuffer) +}) + +suite.add('NEW CODE: * bigArray', function () { + parser.execute(bigArrayBuffer) +}) + // ERRORS suite.add('\nOLD CODE: * error', function () { From 5bc17089f73e70f68404d9383b95d981942e344f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 9 May 2016 08:26:05 +0200 Subject: [PATCH 20/38] Improve number parsing --- lib/parser.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 4512522..e10dacd 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -8,18 +8,25 @@ var ReplyError = require('./replyError') * @param parser * @returns {*} */ -function parseSimpleString (parser) { +function parseSimpleNumbers (parser) { var offset = parser.offset var length = parser.buffer.length var string = '' + var c1 = parser.buffer[offset++] + + if (c1 === 45) { + string += '-' + } else { + string += c1 - 48 + } while (offset < length) { - var c1 = parser.buffer[offset++] + c1 = parser.buffer[offset++] if (c1 === 13 && parser.buffer[offset] === 10) { // \r\n parser.offset = offset + 1 return string } - string += String.fromCharCode(c1) + string += c1 - 48 } } @@ -61,12 +68,12 @@ function parseSimpleStringViaOffset (parser) { } /** - * Returns the string length via parseSimpleString + * Returns the string length via parseSimpleNumbers * @param parser * @returns {*} */ function parseLength (parser) { - var string = parseSimpleString(parser) + var string = parseSimpleNumbers(parser) if (string !== undefined) { return +string } @@ -78,7 +85,7 @@ function parseLength (parser) { * @returns {*} */ function parseInteger (parser) { - var string = parseSimpleString(parser) + var string = parseSimpleNumbers(parser) if (string !== undefined) { // If stringNumbers is activated the parser always returns numbers as string // This is important for big numbers (number > Math.pow(2, 53)) as js numbers From 534deff9382926f63a2d211f990e3df7f70e441a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 9 May 2016 08:26:23 +0200 Subject: [PATCH 21/38] Fix error regression to contain buffers instead of strings --- lib/parser.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/parser.js b/lib/parser.js index e10dacd..c83917d 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -132,6 +132,9 @@ function parseBulkString (parser) { function parseError (parser) { var string = parseSimpleStringViaOffset(parser) if (string !== undefined) { + if (parser.optionReturnBuffers === true) { + string = string.toString() + } return new ReplyError(string) } } From ad578bf0986db70219ce31fa71579c2b8349725d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 9 May 2016 08:31:59 +0200 Subject: [PATCH 22/38] Add testcase for the error regression --- test/parsers.spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 13045e6..d7fd649 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -112,7 +112,7 @@ describe('parsers', function () { } var parser = new Parser({ returnReply: checkReply, - returnError: returnError, + returnError: checkError, returnFatalError: checkError, returnBuffers: true }) @@ -123,6 +123,8 @@ describe('parsers', function () { assert.strictEqual(errCount, 1) parser.execute(new Buffer('*1\r\n+CCC\r\n')) assert.strictEqual(replyCount, 2) + parser.execute(new Buffer('-Protocol error, got "b" as reply type byte\r\n')) + assert.strictEqual(errCount, 2) }) it('parser error v3 without returnFatalError specified', function () { From 189444a060e024390856ad5041d0b0c24a85ee13 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 May 2016 16:34:49 +0200 Subject: [PATCH 23/38] Improve number parsing further --- benchmark/index.js | 15 ++++++++++++ lib/parser.js | 58 +++++++++++++++++++++++++++++--------------- test/parsers.spec.js | 26 ++++++++++++++++---- 3 files changed, 75 insertions(+), 24 deletions(-) diff --git a/benchmark/index.js b/benchmark/index.js index 804cdcb..d7b7f0e 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -34,6 +34,7 @@ var startBuffer = new Buffer('$100\r\nabcdefghij') var chunkBuffer = new Buffer('abcdefghijabcdefghijabcdefghij') var stringBuffer = new Buffer('+testing a simple string\r\n') var integerBuffer = new Buffer(':1237884\r\n') +var bigIntegerBuffer = new Buffer(':18446744073709551617\r\n') // 2^64 + 1 var errorBuffer = new Buffer('-Error ohnoesitbroke\r\n') var arrayBuffer = new Buffer('*1\r\n*1\r\n$1\r\na\r\n') var endBuffer = new Buffer('\r\n') @@ -158,6 +159,20 @@ suite.add('NEW CODE: + integer', function () { parser.execute(integerBuffer) }) +// BIG INTEGER + +suite.add('\nOLD CODE: + big integer', function () { + parserOld.execute(bigIntegerBuffer) +}) + +suite.add('HIREDIS: + big integer', function () { + parserHiRedis.execute(bigIntegerBuffer) +}) + +suite.add('NEW CODE: + big integer', function () { + parser.execute(bigIntegerBuffer) +}) + // ARRAYS suite.add('\nOLD CODE: * array', function () { diff --git a/lib/parser.js b/lib/parser.js index c83917d..4138594 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -4,29 +4,52 @@ var ReplyError = require('./replyError') /** * Used for lengths and numbers only, faster perf on arrays / bulks - * Don't use this for strings, as fromCharCode returns utf-16 and breaks utf-8 compatibility * @param parser * @returns {*} */ function parseSimpleNumbers (parser) { var offset = parser.offset var length = parser.buffer.length - var string = '' - var c1 = parser.buffer[offset++] + var number = 0 + var sign = false - if (c1 === 45) { - string += '-' - } else { - string += c1 - 48 + if (parser.buffer[offset] === 45) { + sign = true + offset++ + } + + while (offset < length) { + var c1 = parser.buffer[offset++] + if (c1 === 13 && parser.buffer[offset] === 10) { // \r\n + parser.offset = offset + 1 + return sign ? -number : number + } + number = (number * 10) + (c1 - 48) + } +} + +/** + * Used for integer numbers in case of the returnNumbers option + * @param parser + * @returns {*} + */ +function parseStringNumbers (parser) { + var offset = parser.offset + var length = parser.buffer.length + var number = '' + + if (parser.buffer[offset] === 45) { + number += '-' + offset++ } while (offset < length) { - c1 = parser.buffer[offset++] + var c1 = parser.buffer[offset++] if (c1 === 13 && parser.buffer[offset] === 10) { // \r\n parser.offset = offset + 1 - return string + return number } - string += c1 - 48 + number += c1 - 48 } } @@ -85,16 +108,13 @@ function parseLength (parser) { * @returns {*} */ function parseInteger (parser) { - var string = parseSimpleNumbers(parser) - if (string !== undefined) { - // If stringNumbers is activated the parser always returns numbers as string - // This is important for big numbers (number > Math.pow(2, 53)) as js numbers - // are 64bit floating point numbers with reduced precision - if (parser.optionStringNumbers === false) { - return +string - } - return string + // If stringNumbers is activated the parser always returns numbers as string + // This is important for big numbers (number > Math.pow(2, 53)) as js numbers + // are 64bit floating point numbers with reduced precision + if (parser.optionStringNumbers) { + return parseStringNumbers(parser) } + return parseSimpleNumbers(parser) } /** diff --git a/test/parsers.spec.js b/test/parsers.spec.js index d7fd649..739dcff 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -2,7 +2,6 @@ /* eslint-env mocha */ -var intercept = require('intercept-stdout') var assert = require('assert') var JavascriptParser = require('../') var HiredisParser = require('./hiredis') @@ -420,19 +419,36 @@ describe('parsers', function () { assert.strictEqual(reply, entries[replyCount]) replyCount++ } - var unhookIntercept = intercept(function () { - return '' - }) var parser = new Parser({ returnReply: checkReply, returnError: returnError, returnFatalError: returnFatalError, stringNumbers: true }) - unhookIntercept() parser.execute(new Buffer(':123\r\n:590295810358705700002\r\n:-99999999999999999\r\n')) assert.strictEqual(replyCount, 3) }) + + it('handle big numbers', function () { + if (Parser.name === 'HiredisReplyParser') { + return this.skip() + } + var replyCount = 0 + var number = 9007199254740991 // Number.MAX_SAFE_INTEGER + function checkReply (reply) { + assert.strictEqual(reply, number++) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + parser.execute(new Buffer(':' + number + '\r\n')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer(':' + number + '\r\n')) + assert.strictEqual(replyCount, 2) + }) }) }) }) From d7549525fdec0ee9dec11a8c33ffba3f1d3dd4f7 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 May 2016 16:36:20 +0200 Subject: [PATCH 24/38] Improve string parsing minimal --- lib/parser.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 4138594..70c8708 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -82,10 +82,9 @@ function parseSimpleStringViaOffset (parser) { var length = parser.buffer.length while (offset < length) { - var c1 = parser.buffer[offset++] - if (c1 === 13 && parser.buffer[offset] === 10) { // \r\n - parser.offset = offset + 1 - return convertBufferRange(parser, start, offset - 1) + if (parser.buffer[offset++] === 10) { // \r\n + parser.offset = offset + return convertBufferRange(parser, start, offset - 2) } } } From 047120968ba7feea0ddca89cbaea5e51074d3eb3 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 May 2016 18:05:01 +0200 Subject: [PATCH 25/38] Refactor some lines away --- lib/parser.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 70c8708..d8e9751 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -269,7 +269,6 @@ function JavascriptRedisParser (options) { JavascriptRedisParser.prototype.execute = function (buffer) { if (this.buffer === null) { this.buffer = buffer - this.offset = 0 } else if (this.bigStrSize === 0) { var oldLength = this.buffer.length var remainingLength = oldLength - this.offset @@ -278,7 +277,6 @@ JavascriptRedisParser.prototype.execute = function (buffer) { this.buffer.copy(newBuffer, 0, this.offset, oldLength) buffer.copy(newBuffer, remainingLength, 0, buffer.length) this.buffer = newBuffer - this.offset = 0 } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { this.bufferCache.push(buffer) if (this.offset !== 0) { @@ -288,16 +286,15 @@ JavascriptRedisParser.prototype.execute = function (buffer) { this.bigStrSize = 0 this.totalChunkSize = 0 this.bufferCache = [] - this.offset = 0 } else { this.bufferCache.push(buffer) this.totalChunkSize += buffer.length return } - var length = this.buffer.length + this.offset = 0 - while (this.offset < length) { + while (this.offset < this.buffer.length) { var offset = this.offset var type = this.buffer[this.offset++] var response = parseType(this, type) From 304ca09a14d774aaa7b810632373ba09861db6f1 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 09:10:47 +0200 Subject: [PATCH 26/38] Improve multiple chunks being concated by using a bufferPool It is going to increase in size if required --- benchmark/index.js | 19 +++++++++++++ lib/parser.js | 28 ++++++++++++++---- test/parsers.spec.js | 68 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 6 deletions(-) diff --git a/benchmark/index.js b/benchmark/index.js index d7b7f0e..a78c6a8 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -58,6 +58,8 @@ for (i = 0; i < bigArraySize; i++) { } var bigArrayBuffer = new Buffer(bigArray) +var chunkedStringPart1 = new Buffer('+foobar') +var chunkedStringPart2 = new Buffer('bazEND\r\n') var parserOld = new ParserOLD({ returnReply: checkReply, @@ -105,6 +107,23 @@ suite.add('NEW CODE: multiple chunks in a bulk string', function () { parser.execute(endBuffer) }) +// CHUNKED STRINGS + +suite.add('\nOLD CODE: multiple chunks in a string', function () { + parserOld.execute(chunkedStringPart1) + parserOld.execute(chunkedStringPart2) +}) + +suite.add('HIREDIS: multiple chunks in a string', function () { + parserHiRedis.execute(chunkedStringPart1) + parserHiRedis.execute(chunkedStringPart2) +}) + +suite.add('NEW CODE: multiple chunks in a string', function () { + parser.execute(chunkedStringPart1) + parser.execute(chunkedStringPart2) +}) + // BIG BULK STRING suite.add('\nOLD CODE: 4mb bulk string', function () { diff --git a/lib/parser.js b/lib/parser.js index d8e9751..0ba8c48 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -1,6 +1,8 @@ 'use strict' var ReplyError = require('./replyError') +// TODO: Consider shrinking the bufferPool if it's not used a lot by using interval check +var bufferPool = new Buffer(64 * 1024) /** * Used for lengths and numbers only, faster perf on arrays / bulks @@ -261,6 +263,19 @@ function JavascriptRedisParser (options) { this.bufferCache = [] } +function concat (parser, length) { + var list = parser.bufferCache + var pos = 0 + if (bufferPool.length < length) { + bufferPool = new Buffer(length) + } + for (var i = 0; i < list.length; i++) { + list[i].copy(bufferPool, pos) + pos += list[i].length + } + return bufferPool.slice(parser.offset, length) +} + /** * Parse the redis buffer * @param buffer @@ -273,16 +288,17 @@ JavascriptRedisParser.prototype.execute = function (buffer) { var oldLength = this.buffer.length var remainingLength = oldLength - this.offset var newLength = remainingLength + buffer.length - var newBuffer = new Buffer(newLength) + // ~ 5% speed increase over using new Buffer(length) all the time + if (bufferPool.length < newLength) { // We can't rely on the chunk size + bufferPool = new Buffer(newLength) + } + var newBuffer = bufferPool this.buffer.copy(newBuffer, 0, this.offset, oldLength) buffer.copy(newBuffer, remainingLength, 0, buffer.length) - this.buffer = newBuffer + this.buffer = newBuffer.slice(0, newLength) } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { this.bufferCache.push(buffer) - if (this.offset !== 0) { - this.bufferCache[0] = this.bufferCache[0].slice(this.offset) - } - this.buffer = Buffer.concat(this.bufferCache, this.totalChunkSize + buffer.length - this.offset) + this.buffer = concat(this, this.totalChunkSize + buffer.length) this.bigStrSize = 0 this.totalChunkSize = 0 this.bufferCache = [] diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 739dcff..4634902 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -449,6 +449,74 @@ describe('parsers', function () { parser.execute(new Buffer(':' + number + '\r\n')) assert.strictEqual(replyCount, 2) }) + + it('handle big data', function () { + if (Parser.name === 'HiredisReplyParser') { + return this.skip() + } + var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars + var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long + var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n') + var chunks = new Array(64) + for (var i = 0; i < 64; i++) { + chunks[i] = new Buffer(bigStringArray.join(' ') + '.') // Math.pow(2, 16) chars long + } + var replyCount = 0 + function checkReply (reply) { + assert.strictEqual(reply.length, 4 * 1024 * 1024) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + parser.execute(startBigBuffer) + for (i = 0; i < 64; i++) { + assert.strictEqual(parser.bufferCache.length, i + 1) + parser.execute(chunks[i]) + } + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('\r\n')) + assert.strictEqual(replyCount, 1) + }) + + it('handle big data 2', function () { + if (Parser.name === 'HiredisReplyParser') { + return this.skip() + } + var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars + var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long + var startBigBuffer = new Buffer('\r\n$' + (4 * 1024 * 1024) + '\r\n') + var chunks = new Array(64) + for (var i = 0; i < 64; i++) { + chunks[i] = new Buffer(bigStringArray.join(' ') + '.') // Math.pow(2, 16) chars long + } + var replyCount = 0 + function checkReply (reply) { + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + parser.execute(new Buffer('+test')) + parser.execute(startBigBuffer) + for (i = 0; i < 64; i++) { + assert.strictEqual(parser.bufferCache.length, i + 1) + parser.execute(chunks[i]) + } + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('\r\n')) + assert.strictEqual(replyCount, 2) + }) }) }) }) From 2642f6034ce7d8ebf9abcf459a71d2d917ba8a4a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 09:11:48 +0200 Subject: [PATCH 27/38] Fix bulk string offset cache This improves bulk strings with multiple chunks significantly --- lib/parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/parser.js b/lib/parser.js index 0ba8c48..f34995a 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -135,7 +135,7 @@ function parseBulkString (parser) { if (offsetEnd + 2 > parser.buffer.length) { parser.bufferCache.push(parser.buffer) parser.totalChunkSize = parser.buffer.length - parser.bigStrSize = length + 2 + parser.bigStrSize = offsetEnd + 2 return } From 40018912c9563db6cb0923878d79fe280a900bae Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 09:16:07 +0200 Subject: [PATCH 28/38] Add jsdoc comment --- lib/parser.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/parser.js b/lib/parser.js index f34995a..25ae385 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -206,7 +206,6 @@ function parseArray (parser) { * @param type * @returns {*} */ - function parseType (parser, type) { switch (type) { case 36: // $ @@ -263,6 +262,12 @@ function JavascriptRedisParser (options) { this.bufferCache = [] } +/** + * Concat the collected chunks from parser.bufferCache + * @param parser + * @param length + * @returns {Buffer} + */ function concat (parser, length) { var list = parser.bufferCache var pos = 0 From 6f82d62f761bea34b2aa900af419809a6a2bc308 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 12:56:23 +0200 Subject: [PATCH 29/38] Add additional benchmarks for returning buffers and stringNumbers --- benchmark/index.js | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/benchmark/index.js b/benchmark/index.js index a78c6a8..a2ba0ff 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -81,6 +81,20 @@ var parser = new Parser({ returnFatalError: returnError }) +var parserBuffer = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + returnBuffers: true +}) + +var parserStr = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnError, + stringNumbers: true +}) + // BULK STRINGS suite.add('OLD CODE: multiple chunks in a bulk string', function () { @@ -107,6 +121,14 @@ suite.add('NEW CODE: multiple chunks in a bulk string', function () { parser.execute(endBuffer) }) +suite.add('NEW BUF: multiple chunks in a bulk string', function () { + parserBuffer.execute(startBuffer) + parserBuffer.execute(chunkBuffer) + parserBuffer.execute(chunkBuffer) + parserBuffer.execute(chunkBuffer) + parserBuffer.execute(endBuffer) +}) + // CHUNKED STRINGS suite.add('\nOLD CODE: multiple chunks in a string', function () { @@ -124,6 +146,11 @@ suite.add('NEW CODE: multiple chunks in a string', function () { parser.execute(chunkedStringPart2) }) +suite.add('NEW BUF: multiple chunks in a string', function () { + parserBuffer.execute(chunkedStringPart1) + parserBuffer.execute(chunkedStringPart2) +}) + // BIG BULK STRING suite.add('\nOLD CODE: 4mb bulk string', function () { @@ -150,6 +177,14 @@ suite.add('NEW CODE: 4mb bulk string', function () { parser.execute(endBuffer) }) +suite.add('NEW BUF: 4mb bulk string', function () { + parserBuffer.execute(startBigBuffer) + for (var i = 0; i < 64; i++) { + parserBuffer.execute(chunks[i]) + } + parserBuffer.execute(endBuffer) +}) + // STRINGS suite.add('\nOLD CODE: + simple string', function () { @@ -164,6 +199,10 @@ suite.add('NEW CODE: + simple string', function () { parser.execute(stringBuffer) }) +suite.add('NEW BUF: + simple string', function () { + parserBuffer.execute(stringBuffer) +}) + // INTEGERS suite.add('\nOLD CODE: + integer', function () { @@ -178,6 +217,10 @@ suite.add('NEW CODE: + integer', function () { parser.execute(integerBuffer) }) +suite.add('NEW STR: + integer', function () { + parserStr.execute(integerBuffer) +}) + // BIG INTEGER suite.add('\nOLD CODE: + big integer', function () { @@ -192,6 +235,10 @@ suite.add('NEW CODE: + big integer', function () { parser.execute(bigIntegerBuffer) }) +suite.add('NEW STR: + big integer', function () { + parserStr.execute(bigIntegerBuffer) +}) + // ARRAYS suite.add('\nOLD CODE: * array', function () { @@ -206,6 +253,10 @@ suite.add('NEW CODE: * array', function () { parser.execute(arrayBuffer) }) +suite.add('NEW BUF: * array', function () { + parserBuffer.execute(arrayBuffer) +}) + // BIG ARRAYS suite.add('\nOLD CODE: * bigArray', function () { @@ -220,6 +271,10 @@ suite.add('NEW CODE: * bigArray', function () { parser.execute(bigArrayBuffer) }) +suite.add('NEW BUF: * bigArray', function () { + parserBuffer.execute(bigArrayBuffer) +}) + // ERRORS suite.add('\nOLD CODE: * error', function () { From 200a8cfc1b45959a43f2c414ea6b0c36371f99ae Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 12:57:47 +0200 Subject: [PATCH 30/38] Improve parsing very big chunks Also free the bufferPool over time again --- lib/parser.js | 71 +++++++++++++++++++++--- test/parsers.spec.js | 128 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 181 insertions(+), 18 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 25ae385..dfd3bae 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -1,8 +1,8 @@ 'use strict' var ReplyError = require('./replyError') -// TODO: Consider shrinking the bufferPool if it's not used a lot by using interval check var bufferPool = new Buffer(64 * 1024) +var interval = null /** * Used for lengths and numbers only, faster perf on arrays / bulks @@ -133,9 +133,10 @@ function parseBulkString (parser) { } var offsetEnd = parser.offset + length if (offsetEnd + 2 > parser.buffer.length) { - parser.bufferCache.push(parser.buffer) - parser.totalChunkSize = parser.buffer.length parser.bigStrSize = offsetEnd + 2 + parser.bigOffset = parser.offset + parser.totalChunkSize = parser.buffer.length + parser.bufferCache.push(parser.buffer) return } @@ -258,10 +259,56 @@ function JavascriptRedisParser (options) { this.offset = 0 this.buffer = null this.bigStrSize = 0 + this.bigOffset = 0 this.totalChunkSize = 0 this.bufferCache = [] } +/** + * Concat a bulk string containing multiple chunks + * @param parser + * @param buffer + * @returns {String} + */ +function concatBigString (parser, buffer) { + var list = parser.bufferCache + var length = parser.bigStrSize + var stringSize = length - parser.bigOffset - 2 + var res = list[0].toString('utf8', parser.bigOffset) + for (var i = 1; i < list.length - 1; i++) { + res += list[i].toString() + } + if (i > 1) { + if (res.length + list[i].length <= stringSize) { + res += list[i].toString() + } else { + res += list[i].toString('utf8', 0, list[i].length - 1) + } + } + var offset = stringSize - res.length + if (offset > 0) { + res += buffer.toString('utf8', 0, offset) + } + parser.offset = offset + 2 + parser.bigOffset = 0 + return res +} + +/** + * Decrease the bufferPool size over time + * @returns {undefined} + */ +function decreaseBufferPool () { + if (bufferPool.length > 96 * 1024) { + // Decrease the bufferPool by 16kb + bufferPool = bufferPool.slice(0, bufferPool.length - 16 * 1024) + // console.log(bufferPool.length) + } else { + clearInterval(interval) + interval = null + } +} + /** * Concat the collected chunks from parser.bufferCache * @param parser @@ -273,6 +320,9 @@ function concat (parser, length) { var pos = 0 if (bufferPool.length < length) { bufferPool = new Buffer(length) + if (interval === null) { + interval = setInterval(decreaseBufferPool, 50) + } } for (var i = 0; i < list.length; i++) { list[i].copy(bufferPool, pos) @@ -289,6 +339,7 @@ function concat (parser, length) { JavascriptRedisParser.prototype.execute = function (buffer) { if (this.buffer === null) { this.buffer = buffer + this.offset = 0 } else if (this.bigStrSize === 0) { var oldLength = this.buffer.length var remainingLength = oldLength - this.offset @@ -301,9 +352,17 @@ JavascriptRedisParser.prototype.execute = function (buffer) { this.buffer.copy(newBuffer, 0, this.offset, oldLength) buffer.copy(newBuffer, remainingLength, 0, buffer.length) this.buffer = newBuffer.slice(0, newLength) + this.offset = 0 } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { - this.bufferCache.push(buffer) - this.buffer = concat(this, this.totalChunkSize + buffer.length) + // The returned type might be Array * (42) and in that case we can't improve the parsing currently + if (this.optionReturnBuffers === false && this.buffer[this.offset] === 36) { + this.returnReply(concatBigString(this, buffer)) + this.buffer = buffer + } else { // This applies for arrays too + this.bufferCache.push(buffer) + this.buffer = concat(this, this.totalChunkSize + buffer.length) + this.offset = 0 + } this.bigStrSize = 0 this.totalChunkSize = 0 this.bufferCache = [] @@ -313,8 +372,6 @@ JavascriptRedisParser.prototype.execute = function (buffer) { return } - this.offset = 0 - while (this.offset < this.buffer.length) { var offset = this.offset var type = this.buffer[this.offset++] diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 4634902..171c3c2 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -40,6 +40,32 @@ describe('parsers', function () { parsers.forEach(function (Parser) { describe(Parser.name, function () { + it('chunks getting to big for the bufferPool', function () { + // This is a edge case. Chunks should not exceed Math.pow(2, 16) bytes + var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars + var bigString = (new Array(Math.pow(2, 17) / lorem.length).join(lorem)) // Math.pow(2, 17) chars long + var replyCount = 0 + function checkReply (reply) { + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError + }) + parser.execute(new Buffer('+test')) + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('\r\n+')) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer(bigString)) + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('\r\n')) + assert.strictEqual(replyCount, 2) + }) + it('handles multi-bulk reply and check context binding', function () { var replyCount = 0 function Abc () {} @@ -430,9 +456,6 @@ describe('parsers', function () { }) it('handle big numbers', function () { - if (Parser.name === 'HiredisReplyParser') { - return this.skip() - } var replyCount = 0 var number = 9007199254740991 // Number.MAX_SAFE_INTEGER function checkReply (reply) { @@ -450,10 +473,58 @@ describe('parsers', function () { assert.strictEqual(replyCount, 2) }) - it('handle big data', function () { - if (Parser.name === 'HiredisReplyParser') { - return this.skip() + it('handle big data with buffers', function (done) { + var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars + var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long + var startBigBuffer = new Buffer('\r\n$' + (128 * 1024) + '\r\n') + var startSecondBigBuffer = new Buffer('\r\n$' + (256 * 1024) + '\r\n') + var chunks = new Array(4) + for (var i = 0; i < 4; i++) { + chunks[i] = new Buffer(bigStringArray.join(' ') + '.') // Math.pow(2, 16) chars long + } + var replyCount = 0 + function checkReply (reply) { + replyCount++ } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError, + returnBuffers: true + }) + parser.execute(new Buffer('+test')) + assert.strictEqual(replyCount, 0) + parser.execute(startBigBuffer) + for (i = 0; i < 2; i++) { + if (Parser.name === 'JavascriptReplyParser') { + assert.strictEqual(parser.bufferCache.length, i + 1) + } + parser.execute(chunks[i]) + } + assert.strictEqual(replyCount, 1) + parser.execute(new Buffer('\r\n')) + assert.strictEqual(replyCount, 2) + parser.execute(new Buffer('+test')) + assert.strictEqual(replyCount, 2) + parser.execute(startSecondBigBuffer) + for (i = 0; i < 4; i++) { + if (Parser.name === 'JavascriptReplyParser') { + assert.strictEqual(parser.bufferCache.length, i + 1) + } + parser.execute(chunks[i]) + } + assert.strictEqual(replyCount, 3) + parser.execute(new Buffer('\r\n')) + assert.strictEqual(replyCount, 4) + // Delay done so the bufferPool is cleared and tested + // If the buffer is not cleared, the coverage is not going to be at 100% + setTimeout(done, 600) + }) + + it('handle big data', function () { var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + @@ -476,7 +547,9 @@ describe('parsers', function () { }) parser.execute(startBigBuffer) for (i = 0; i < 64; i++) { - assert.strictEqual(parser.bufferCache.length, i + 1) + if (Parser.name === 'JavascriptReplyParser') { + assert.strictEqual(parser.bufferCache.length, i + 1) + } parser.execute(chunks[i]) } assert.strictEqual(replyCount, 0) @@ -485,9 +558,6 @@ describe('parsers', function () { }) it('handle big data 2', function () { - if (Parser.name === 'HiredisReplyParser') { - return this.skip() - } var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + @@ -510,13 +580,49 @@ describe('parsers', function () { parser.execute(new Buffer('+test')) parser.execute(startBigBuffer) for (i = 0; i < 64; i++) { - assert.strictEqual(parser.bufferCache.length, i + 1) + if (Parser.name === 'JavascriptReplyParser') { + assert.strictEqual(parser.bufferCache.length, i + 1) + } parser.execute(chunks[i]) } assert.strictEqual(replyCount, 1) parser.execute(new Buffer('\r\n')) assert.strictEqual(replyCount, 2) }) + + it('handle big data 2 with buffers', function () { + var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' + + 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars + var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long + var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n') + var chunks = new Array(64) + for (var i = 0; i < 64; i++) { + chunks[i] = new Buffer(bigStringArray.join(' ') + '.') // Math.pow(2, 16) chars long + } + var replyCount = 0 + function checkReply (reply) { + assert.strictEqual(reply.length, 4 * 1024 * 1024) + replyCount++ + } + var parser = new Parser({ + returnReply: checkReply, + returnError: returnError, + returnFatalError: returnFatalError, + returnBuffers: true + }) + parser.execute(startBigBuffer) + for (i = 0; i < 64; i++) { + if (Parser.name === 'JavascriptReplyParser') { + assert.strictEqual(parser.bufferCache.length, i + 1) + } + parser.execute(chunks[i]) + } + assert.strictEqual(replyCount, 0) + parser.execute(new Buffer('\r\n')) + assert.strictEqual(replyCount, 1) + }) }) }) }) From d3c5cf6ee2e95689f1dcef443164cf85fa76093b Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 12:58:51 +0200 Subject: [PATCH 31/38] Improve options validation --- lib/parser.js | 23 +++++++++++++++++------ test/parsers.spec.js | 32 ++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index dfd3bae..0c4c556 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -224,6 +224,16 @@ function parseType (parser, type) { } } +// All allowed options including their typeof value +var optionTypes = { + returnError: 'function', + returnFatalError: 'function', + returnReply: 'function', + returnBuffers: 'boolean', + stringNumbers: 'boolean', + name: 'string' +} + /** * Javascript Redis Parser * @param options @@ -233,18 +243,19 @@ function JavascriptRedisParser (options) { if (!(this instanceof JavascriptRedisParser)) { return new JavascriptRedisParser(options) } - if ( - !options || - typeof options.returnError !== 'function' || - typeof options.returnReply !== 'function' - ) { + if (!options || !options.returnError || !options.returnReply) { throw new TypeError('Please provide all return functions while initiating the parser') } + for (var key in options) { + if (typeof options[key] !== optionTypes[key]) { + throw new TypeError('The options argument contains unkown properties or properties of a wrong type') + } + } if (options.name === 'hiredis') { /* istanbul ignore next: hiredis is only supported for legacy usage */ try { var Hiredis = require('../test/hiredis') - console.error(new TypeError('Using the hiredis parser is discouraged. Please remove the name option.').stack.replace('Error', 'Warning')) + console.error(new TypeError('Using hiredis is discouraged. Please use the faster JS parser by removing the name option.').stack.replace('Error', 'Warning')) return new Hiredis(options) } catch (e) { console.error(new TypeError('Hiredis is not installed. Please remove the `name` option. The (faster) JS parser is used instead.').stack.replace('Error', 'Warning')) diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 171c3c2..2c78d8e 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -24,15 +24,39 @@ describe('parsers', function () { assert.strictEqual(parser.name, 'hiredis') }) - it('fail for missing options', function () { + it('fail for missing options argument', function () { + assert.throws(function () { + JavascriptParser() + }, function (err) { + assert.strictEqual(err.message, 'Please provide all return functions while initiating the parser') + assert(err instanceof TypeError) + return true + }) + }) + + it('fail for faulty options properties', function () { assert.throws(function () { JavascriptParser({ returnReply: returnReply, - returnBuffers: true, - name: 'hiredis' + returnError: true }) }, function (err) { - assert.strictEqual(err.message, 'Please provide all return functions while initiating the parser') + assert.strictEqual(err.message, 'The options argument contains unkown properties or properties of a wrong type') + assert(err instanceof TypeError) + return true + }) + }) + + it('fail for faulty options properties #2', function () { + assert.throws(function () { + JavascriptParser({ + returnReply: returnReply, + returnError: returnError, + bla: undefined + }) + }, function (err) { + assert.strictEqual(err.message, 'The options argument contains unkown properties or properties of a wrong type') + assert(err instanceof TypeError) return true }) }) From d12bc97663ec558436062cab2e80e2f557200bee Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 13:04:01 +0200 Subject: [PATCH 32/38] Move hiredis back into the lib folder to keep it deprecated --- changelog.md | 9 +++++---- {test => lib}/hiredis.js | 3 --- lib/parser.js | 2 +- test/parsers.spec.js | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) rename {test => lib}/hiredis.js (92%) diff --git a/changelog.md b/changelog.md index 44b9183..82f4521 100644 --- a/changelog.md +++ b/changelog.md @@ -1,20 +1,21 @@ ## v.2.0.0 - 0x May, 2016 The javascript parser got completly rewritten by [Michael Diarmid](https://github.com/Salakar) and [Ruben Bridgewater](https://github.com/BridgeAR) and is now a lot faster than the hiredis parser. -Therefore the hiredis parser was removed and is only used for testing purposes and benchmarking comparison. +Therefore the hiredis parser was deprecated and should only be used for testing purposes and benchmarking comparison. All Errors returned by the parser are from now on of class ReplyError Features +- Improved performance by up to 15x as fast as before +- Improved options validation - Added ReplyError Class - Added parser benchmark +- Switched default parser from hiredis to JS, no matter if hiredis is installed or not Removed -- Dropped support of hiredis -- The `name` option is "removed" - - It is still available for backwards compatibility but it is strongly recommended not to use it +- Deprecated hiredis support ## v.1.3.0 - 27 Mar, 2016 diff --git a/test/hiredis.js b/lib/hiredis.js similarity index 92% rename from test/hiredis.js rename to lib/hiredis.js index 3cf7b53..535c3ef 100644 --- a/test/hiredis.js +++ b/lib/hiredis.js @@ -25,9 +25,6 @@ function parseData (parser) { * @constructor */ function HiredisReplyParser (options) { - if (!(this instanceof HiredisReplyParser)) { - return new HiredisReplyParser(options) - } this.returnError = options.returnError this.returnFatalError = options.returnFatalError || options.returnError this.returnReply = options.returnReply diff --git a/lib/parser.js b/lib/parser.js index 0c4c556..a222e35 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -254,7 +254,7 @@ function JavascriptRedisParser (options) { if (options.name === 'hiredis') { /* istanbul ignore next: hiredis is only supported for legacy usage */ try { - var Hiredis = require('../test/hiredis') + var Hiredis = require('./hiredis') console.error(new TypeError('Using hiredis is discouraged. Please use the faster JS parser by removing the name option.').stack.replace('Error', 'Warning')) return new Hiredis(options) } catch (e) { diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 2c78d8e..1a26150 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -4,7 +4,7 @@ var assert = require('assert') var JavascriptParser = require('../') -var HiredisParser = require('./hiredis') +var HiredisParser = require('../lib/hiredis') var ReplyError = JavascriptParser.ReplyError var parsers = [JavascriptParser, HiredisParser] From decf59273dcd9e420c92c97504eb9bac8a449e83 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 May 2016 13:04:52 +0200 Subject: [PATCH 33/38] Always run istanbul directly --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 09cd44c..003e981 100644 --- a/package.json +++ b/package.json @@ -4,9 +4,9 @@ "description": "Javascript Redis protocol (RESP) parser", "main": "index.js", "scripts": { - "test": "mocha", + "test": "npm run coverage", "benchmark": "node ./benchmark", - "posttest": "standard && npm run coverage && npm run coverage:check", + "posttest": "standard && npm run coverage:check", "coverage": "node ./node_modules/istanbul/lib/cli.js cover --preserve-comments ./node_modules/mocha/bin/_mocha -- -R spec", "coverage:check": "node ./node_modules/istanbul/lib/cli.js check-coverage --branch 100 --statement 100" }, From 9151ae0f326f1854ebed0ec7f911552aeed959e9 Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Tue, 10 May 2016 21:50:21 +0100 Subject: [PATCH 34/38] Update README.md --- README.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 5278ac6..256292a 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A high performance javascript redis parser built for [node_redis](https://github.com/NodeRedis/node_redis) and [ioredis](https://github.com/luin/ioredis). -Generally all [RESP](http://redis.io/topics/protocol) data will be properly parsed by the parser. +All [RESP](http://redis.io/topics/protocol) data is parsed by this parser. ## Install @@ -22,7 +22,7 @@ npm install redis-parser ```js var Parser = require('redis-parser'); -new Parser(options); +var myParser = new Parser(options); ``` ### Possible options @@ -31,7 +31,6 @@ new Parser(options); * `returnError`: *function*; mandatory * `returnFatalError`: *function*; optional, defaults to the returnError function * `returnBuffers`: *boolean*; optional, defaults to false -* `stringNumbers`: *boolean*; optional, defaults to false ### Example @@ -69,7 +68,7 @@ You do not have to use the returnFatalError function. Fatal errors will be retur And if you want to return buffers instead of strings, you can do this by adding the `returnBuffers` option. -If you handle big numbers, you should pass the `stringNumbers` option. That case numbers above 2^53 can be handled properly without reduced precision. +Big numbers that are too large for JS are automatically stringified. ```js // Same functions as in the first example @@ -81,7 +80,6 @@ var parser = new Parser({ returnError: function(err) { lib.returnError(err); }, - stringNumbers: true, // Return all numbers as string instead of a js number returnBuffers: true // All strings are returned as buffer e.g. }); @@ -90,12 +88,11 @@ var parser = new Parser({ ## Protocol errors -To handle protocol errors (this is very unlikely to happen) gracefully you should add the returnFatalError option, reject any still running command (they might have been processed properly but the reply is just wrong), destroy the socket and reconnect. Be aware that while doing this, no new command may be added, so all new commands have to be buffered in the meanwhile. -Otherwise a chunk might still contain partial data of a following command that was already processed properly but answered in the same chunk as the command that resulted in the protocol error. +To handle protocol errors (this is very unlikely to happen) gracefully you should add the returnFatalError option, reject any still running command (they might have been processed properly but the reply is just wrong), destroy the socket and reconnect. Note that while doing this no new command may be added, so all new commands have to be buffered in the meantime, otherwise a chunk might still contain partial data of a following command that was already processed properly but answered in the same chunk as the command that resulted in the protocol error. ## Contribute -The parser is already optimized but there are likely further optimizations possible. +The parser is highly optimized but there may still be further optimizations possible. ``` npm install From 6f24430fbf3ab47c37db121f3bdeca055e689cb6 Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Tue, 10 May 2016 21:51:06 +0100 Subject: [PATCH 35/38] Update README.md --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 256292a..f75d4b6 100644 --- a/README.md +++ b/README.md @@ -5,9 +5,7 @@ # redis-parser -A high performance javascript redis parser built for [node_redis](https://github.com/NodeRedis/node_redis) and [ioredis](https://github.com/luin/ioredis). - -All [RESP](http://redis.io/topics/protocol) data is parsed by this parser. +A high performance javascript redis parser built for [node_redis](https://github.com/NodeRedis/node_redis) and [ioredis](https://github.com/luin/ioredis). Parses all [RESP](http://redis.io/topics/protocol) data. ## Install From 86de5a058c5f12fe81e347b10fdc2f51d726b5c8 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 18 May 2016 15:39:52 +0200 Subject: [PATCH 36/38] Improve input validation error message --- lib/parser.js | 2 +- test/parsers.spec.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index a222e35..ead2695 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -248,7 +248,7 @@ function JavascriptRedisParser (options) { } for (var key in options) { if (typeof options[key] !== optionTypes[key]) { - throw new TypeError('The options argument contains unkown properties or properties of a wrong type') + throw new TypeError('The options argument contains the property "' + key + '" that is either unkown or of a wrong type') } } if (options.name === 'hiredis') { diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 1a26150..7488473 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -41,7 +41,7 @@ describe('parsers', function () { returnError: true }) }, function (err) { - assert.strictEqual(err.message, 'The options argument contains unkown properties or properties of a wrong type') + assert.strictEqual(err.message, 'The options argument contains the property "returnError" that is either unkown or of a wrong type') assert(err instanceof TypeError) return true }) @@ -55,7 +55,7 @@ describe('parsers', function () { bla: undefined }) }, function (err) { - assert.strictEqual(err.message, 'The options argument contains unkown properties or properties of a wrong type') + assert.strictEqual(err.message, 'The options argument contains the property "bla" that is either unkown or of a wrong type') assert(err instanceof TypeError) return true }) From 16d7e7aa4f99bc24a7bba572fc03d8f3f33a7970 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 18 May 2016 15:40:15 +0200 Subject: [PATCH 37/38] Fix bulk string concat --- lib/parser.js | 43 +++++++++++++++++++++---------------------- test/parsers.spec.js | 19 ++++++++++++++----- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index ead2695..fa13339 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -281,27 +281,26 @@ function JavascriptRedisParser (options) { * @param buffer * @returns {String} */ -function concatBigString (parser, buffer) { +function concatBulkString (parser) { var list = parser.bufferCache - var length = parser.bigStrSize - var stringSize = length - parser.bigOffset - 2 - var res = list[0].toString('utf8', parser.bigOffset) - for (var i = 1; i < list.length - 1; i++) { - res += list[i].toString() - } - if (i > 1) { - if (res.length + list[i].length <= stringSize) { - res += list[i].toString() - } else { - res += list[i].toString('utf8', 0, list[i].length - 1) + // The first chunk might contain the whole bulk string including the \r + var end = parser.totalChunkSize - parser.bigStrSize === -1 + var chunks = list.length + if (end) { + parser.offset = 1 + if (chunks === 2) { + return list[0].toString('utf8', parser.bigOffset, list[0].length - 1) } + } else { + parser.offset = parser.bigStrSize - parser.totalChunkSize + chunks++ } - var offset = stringSize - res.length - if (offset > 0) { - res += buffer.toString('utf8', 0, offset) + var res = list[0].toString('utf8', parser.bigOffset) + for (var i = 1; i < chunks - 2; i++) { + // We are only safe to fully add up elements that are neither the first nor any of the last two elements + res += list[i].toString() } - parser.offset = offset + 2 - parser.bigOffset = 0 + res += list[i].toString('utf8', 0, end ? list[i].length - 1 : parser.offset - 2) return res } @@ -313,7 +312,6 @@ function decreaseBufferPool () { if (bufferPool.length > 96 * 1024) { // Decrease the bufferPool by 16kb bufferPool = bufferPool.slice(0, bufferPool.length - 16 * 1024) - // console.log(bufferPool.length) } else { clearInterval(interval) interval = null @@ -326,7 +324,7 @@ function decreaseBufferPool () { * @param length * @returns {Buffer} */ -function concat (parser, length) { +function concatBuffer (parser, length) { var list = parser.bufferCache var pos = 0 if (bufferPool.length < length) { @@ -365,13 +363,14 @@ JavascriptRedisParser.prototype.execute = function (buffer) { this.buffer = newBuffer.slice(0, newLength) this.offset = 0 } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { + this.bufferCache.push(buffer) // The returned type might be Array * (42) and in that case we can't improve the parsing currently if (this.optionReturnBuffers === false && this.buffer[this.offset] === 36) { - this.returnReply(concatBigString(this, buffer)) + this.returnReply(concatBulkString(this)) + this.bigOffset = 0 this.buffer = buffer } else { // This applies for arrays too - this.bufferCache.push(buffer) - this.buffer = concat(this, this.totalChunkSize + buffer.length) + this.buffer = concatBuffer(this, this.totalChunkSize + buffer.length) this.offset = 0 } this.bigStrSize = 0 diff --git a/test/parsers.spec.js b/test/parsers.spec.js index 7488473..de81258 100644 --- a/test/parsers.spec.js +++ b/test/parsers.spec.js @@ -70,9 +70,11 @@ describe('parsers', function () { 'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' + 'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars - var bigString = (new Array(Math.pow(2, 17) / lorem.length).join(lorem)) // Math.pow(2, 17) chars long + var bigString = (new Array(Math.pow(2, 17) / lorem.length + 1).join(lorem)) // Math.pow(2, 17) chars long var replyCount = 0 + var sizes = [4, Math.pow(2, 17)] function checkReply (reply) { + assert.strictEqual(sizes[replyCount], reply.length) replyCount++ } var parser = new Parser({ @@ -200,7 +202,7 @@ describe('parsers', function () { it('should handle \\r and \\n characters properly', function () { // If a string contains \r or \n characters it will always be send as a bulk string var replyCount = 0 - var entries = ['foo\r', 'foo\r\nbar', '\r\nfoo', 'foo\r\n', 'foo'] + var entries = ['foo\r', 'foo\r\nbar', '\r\nfoo', 'foo\r\n', 'foo', 'foobar', 'foo\r', 'äfooöü', 'abc'] function checkReply (reply) { assert.strictEqual(reply, entries[replyCount]) replyCount++ @@ -217,8 +219,16 @@ describe('parsers', function () { assert.strictEqual(replyCount, 4) parser.execute(new Buffer('+foo\r')) assert.strictEqual(replyCount, 4) - parser.execute(new Buffer('\n')) + parser.execute(new Buffer('\n$6\r\nfoobar\r')) assert.strictEqual(replyCount, 5) + parser.execute(new Buffer('\n$4\r\nfoo\r\r\n')) + assert.strictEqual(replyCount, 7) + parser.execute(new Buffer('$9\r\näfo')) + parser.execute(new Buffer('oö')) + parser.execute(new Buffer('ü\r')) + assert.strictEqual(replyCount, 7) + parser.execute(new Buffer('\n+abc\r\n')) + assert.strictEqual(replyCount, 9) }) it('line breaks in the beginning of the last chunk', function () { @@ -271,10 +281,9 @@ describe('parsers', function () { parser.execute(new Buffer( 'abcdefghij\r\n' + '$100\r\nabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij\r\n' + - '$100\r\nabcdefghijabcdefghijabcdefghijabcdefghij' + '$100\r\nabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij' )) assert.strictEqual(replyCount, 3) - parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij')) parser.execute(new Buffer('abcdefghijabcdefghijabcdefghij\r')) assert.strictEqual(replyCount, 3) parser.execute(new Buffer('\n')) From f4fd3404a476759f5d567552498e62eea9b137ff Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 25 May 2016 20:29:53 +0300 Subject: [PATCH 38/38] Refactor some code to reduce code size --- README.md | 2 +- lib/parser.js | 27 +++++++++++---------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index f75d4b6..6843dc3 100644 --- a/README.md +++ b/README.md @@ -90,7 +90,7 @@ To handle protocol errors (this is very unlikely to happen) gracefully you shoul ## Contribute -The parser is highly optimized but there may still be further optimizations possible. +The parser is highly optimized but there may still be further optimizations possible. ``` npm install diff --git a/lib/parser.js b/lib/parser.js index fa13339..8ae4df5 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -65,6 +65,7 @@ function parseStringNumbers (parser) { */ function convertBufferRange (parser, start, end) { // If returnBuffers is active, all return values are returned as buffers besides numbers and errors + parser.offset = end + 2 if (parser.optionReturnBuffers === true) { return parser.buffer.slice(start, end) } @@ -82,10 +83,10 @@ function parseSimpleStringViaOffset (parser) { var start = parser.offset var offset = parser.offset var length = parser.buffer.length + var buffer = parser.buffer while (offset < length) { - if (parser.buffer[offset++] === 10) { // \r\n - parser.offset = offset + if (buffer[offset++] === 10) { // \r\n return convertBufferRange(parser, start, offset - 2) } } @@ -140,10 +141,7 @@ function parseBulkString (parser) { return } - var offsetBegin = parser.offset - parser.offset = offsetEnd + 2 - - return convertBufferRange(parser, offsetBegin, offsetEnd) + return convertBufferRange(parser, parser.offset, offsetEnd) } /** @@ -284,15 +282,14 @@ function JavascriptRedisParser (options) { function concatBulkString (parser) { var list = parser.bufferCache // The first chunk might contain the whole bulk string including the \r - var end = parser.totalChunkSize - parser.bigStrSize === -1 var chunks = list.length - if (end) { - parser.offset = 1 + var offset = parser.bigStrSize - parser.totalChunkSize + parser.offset = offset + if (offset === 1) { if (chunks === 2) { return list[0].toString('utf8', parser.bigOffset, list[0].length - 1) } } else { - parser.offset = parser.bigStrSize - parser.totalChunkSize chunks++ } var res = list[0].toString('utf8', parser.bigOffset) @@ -300,7 +297,7 @@ function concatBulkString (parser) { // We are only safe to fully add up elements that are neither the first nor any of the last two elements res += list[i].toString() } - res += list[i].toString('utf8', 0, end ? list[i].length - 1 : parser.offset - 2) + res += list[i].toString('utf8', 0, offset === 1 ? list[i].length - 1 : offset - 2) return res } @@ -357,17 +354,15 @@ JavascriptRedisParser.prototype.execute = function (buffer) { if (bufferPool.length < newLength) { // We can't rely on the chunk size bufferPool = new Buffer(newLength) } - var newBuffer = bufferPool - this.buffer.copy(newBuffer, 0, this.offset, oldLength) - buffer.copy(newBuffer, remainingLength, 0, buffer.length) - this.buffer = newBuffer.slice(0, newLength) + this.buffer.copy(bufferPool, 0, this.offset, oldLength) + buffer.copy(bufferPool, remainingLength, 0, buffer.length) + this.buffer = bufferPool.slice(0, newLength) this.offset = 0 } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) { this.bufferCache.push(buffer) // The returned type might be Array * (42) and in that case we can't improve the parsing currently if (this.optionReturnBuffers === false && this.buffer[this.offset] === 36) { this.returnReply(concatBulkString(this)) - this.bigOffset = 0 this.buffer = buffer } else { // This applies for arrays too this.buffer = concatBuffer(this, this.totalChunkSize + buffer.length)