chore(jsparser): Refactor bulk string chunk handling

This gives a tiny performance improvement (a few %) in case of chunked bulk data.
It will also minimize the memory usage a tiny bit.

Author: Ruben Bridgewater

lib/parser.js

@@ -385,36 +385,6 @@ JavascriptRedisParser.prototype.setStringNumbers = function (stringNumbers) {
   this.optionStringNumbers = stringNumbers
 }
 
-/**
- * Concat a bulk string containing multiple chunks
- *
- * Notes:
- * 1) The first chunk might contain the whole bulk string including the \r
- * 2) We are only safe to fully add up elements that are neither the first nor any of the last two elements
- *
- * @param parser
- * @param buffer
- * @returns {String}
- */
-function concatBulkString (parser) {
-  var list = parser.bufferCache
-  var chunks = list.length
-  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)
-    }
-    chunks--
-  }
-  var res = decoder.write(list[0].slice(parser.bigOffset))
-  for (var i = 1; i < chunks - 1; i++) {
-    res += decoder.write(list[i])
-  }
-  res += decoder.end(list[i].slice(0, offset - 2))
-  return res
-}
-
 /**
  * Decrease the bufferPool size over time
  * @returns {undefined}
@@ -444,37 +414,86 @@ function decreaseBufferPool () {
 }
 
 /**
- * Concat the collected chunks from parser.bufferCache.
- *
- * Increases the bufferPool size beforehand if necessary.
+ * Check if the requested size fits in the current bufferPool.
+ * If it does not, reset and increase the bufferPool accordingly.
  *
- * @param parser
  * @param length
- * @returns {Buffer}
+ * @returns {undefined}
  */
-function concatBuffer (parser, length) {
-  var list = parser.bufferCache
-  var pos = bufferOffset
-  length -= parser.offset
+function resizeBuffer (length) {
   if (bufferPool.length < length + bufferOffset) {
     var multiplier = length > 1024 * 1024 * 75 ? 2 : 3
-    if (bufferOffset > 1024 * 1024 * 120) {
+    if (bufferOffset > 1024 * 1024 * 111) {
       bufferOffset = 1024 * 1024 * 50
     }
     bufferPool = new Buffer(length * multiplier + bufferOffset)
     bufferOffset = 0
     counter++
-    pos = 0
     if (interval === null) {
       interval = setInterval(decreaseBufferPool, 50)
     }
   }
-  list[0].copy(bufferPool, pos, parser.offset, list[0].length)
-  pos += list[0].length - parser.offset
-  for (var i = 1; i < list.length; i++) {
+}
+
+/**
+ * Concat a bulk string containing multiple chunks
+ *
+ * Notes:
+ * 1) The first chunk might contain the whole bulk string including the \r
+ * 2) We are only safe to fully add up elements that are neither the first nor any of the last two elements
+ *
+ * @param parser
+ * @returns {String}
+ */
+function concatBulkString (parser) {
+  var list = parser.bufferCache
+  var chunks = list.length
+  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)
+    }
+    chunks--
+  }
+  var res = decoder.write(list[0].slice(parser.bigOffset))
+  for (var i = 1; i < chunks - 1; i++) {
+    res += decoder.write(list[i])
+  }
+  res += decoder.end(list[i].slice(0, offset - 2))
+  return res
+}
+
+/**
+ * Concat the collected chunks from parser.bufferCache.
+ *
+ * Increases the bufferPool size beforehand if necessary.
+ *
+ * @param parser
+ * @returns {Buffer}
+ */
+function concatBulkBuffer (parser) {
+  var list = parser.bufferCache
+  var chunks = list.length
+  var length = parser.bigStrSize - parser.bigOffset - 2
+  var offset = parser.bigStrSize - parser.totalChunkSize
+  parser.offset = offset
+  if (offset === 1) {
+    if (chunks === 2) {
+      return list[0].slice(parser.bigOffset, list[0].length - 1)
+    }
+    chunks--
+    offset = list[list.length - 1].length + 1
+  }
+  resizeBuffer(length)
+  var pos = bufferOffset
+  list[0].copy(bufferPool, pos, parser.bigOffset, list[0].length)
+  pos += list[0].length - parser.bigOffset
+  for (var i = 1; i < list.length - 1; i++) {
     list[i].copy(bufferPool, pos)
     pos += list[i].length
   }
+  list[i].copy(bufferPool, pos, 0, offset - 2)
   var buffer = bufferPool.slice(bufferOffset, length + bufferOffset)
   bufferOffset += length
   return buffer
@@ -486,7 +505,6 @@ function concatBuffer (parser, length) {
  * @returns {undefined}
  */
 JavascriptRedisParser.prototype.execute = function execute (buffer) {
-  var arr
   if (this.buffer === null) {
     this.buffer = buffer
     this.offset = 0
@@ -499,32 +517,26 @@ JavascriptRedisParser.prototype.execute = function execute (buffer) {
     this.buffer = newBuffer
     this.offset = 0
     if (this.arrayCache.length) {
-      arr = parseArrayChunks(this)
+      var arr = parseArrayChunks(this)
       if (!arr) {
         return
       }
       this.returnReply(arr)
     }
   } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) {
     this.bufferCache.push(buffer)
-    if (this.optionReturnBuffers === false && !this.arrayCache.length) {
-      this.returnReply(concatBulkString(this))
-      this.buffer = buffer
-    } else {
-      this.buffer = concatBuffer(this, this.totalChunkSize + buffer.length)
-      this.offset = 0
-      if (this.arrayCache.length) {
-        arr = parseArrayChunks(this)
-        if (!arr) {
-          this.bigStrSize = 0
-          this.bufferCache = []
-          return
-        }
-        this.returnReply(arr)
-      }
-    }
+    var tmp = this.optionReturnBuffers ? concatBulkBuffer(this) : concatBulkString(this)
     this.bigStrSize = 0
     this.bufferCache = []
+    this.buffer = buffer
+    if (this.arrayCache.length) {
+      this.arrayCache[0][this.arrayPos[0]++] = tmp
+      tmp = parseArrayChunks(this)
+      if (!tmp) {
+        return
+      }
+    }
+    this.returnReply(tmp)
   } else {
     this.bufferCache.push(buffer)
     this.totalChunkSize += buffer.length

test/parsers.spec.js

@@ -613,9 +613,12 @@ describe('parsers', function () {
 
         parser.execute(new Buffer('+test\r\n'))
         assert.strictEqual(replyCount, 1)
-        parser.execute(new Buffer('$4\r\ntest\r\n'))
+        parser.execute(new Buffer('$4\r\ntest\r'))
+        parser.execute(new Buffer('\n'))
         assert.strictEqual(replyCount, 2)
-        parser.execute(new Buffer('*1\r\n$4\r\ntest\r\n'))
+        parser.execute(new Buffer('*1\r\n$4\r\nte'))
+        parser.execute(new Buffer('st\r'))
+        parser.execute(new Buffer('\n'))
         assert.strictEqual(replyCount, 3)
       })