Improve number parsing further

Author: Ruben Bridgewater

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

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)
 }
 
 /**

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)
+      })
     })
   })
 })