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

Author: Ruben Bridgewater

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

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 () {