fix(jsparser): minor memory consumtion overhead

Chunked data might have used more bufferPool memory than required

Author: Ruben Bridgewater

benchmark/index.js

@@ -68,9 +68,10 @@ for (i = 0; i < bigArraySize; i++) {
   // A chunk has a maximum size of 2^16 bytes.
   size = 65000 + i
   if (i % 2) {
-    bigArrayChunks.push(new Buffer('\r\n$' + size + '\r\n' + Array(size + 1).join('a') + '\r\n:' + (Math.random() * 1000000 | 0)))
+    // The "x" in the beginning is important to prevent benchmark manipulation due to a minor jsparser optimization
+    bigArrayChunks.push(new Buffer('x\r\n$' + size + '\r\n' + Array(size + 1).join('a') + '\r\n:' + (Math.random() * 1000000 | 0)))
   } else {
-    bigArrayChunks.push(new Buffer('\r\n+this is some short text about nothing\r\n:' + size + '\r\n$' + size + '\r\n' + Array(size + 1).join('b')))
+    bigArrayChunks.push(new Buffer('\r\n+this is some short text about nothing\r\n:' + size + '\r\n$' + size + '\r\n' + Array(size).join('b')))
   }
 }
 bigArrayChunks.push(new Buffer('\r\n'))

changelog.md

@@ -1,3 +1,9 @@
+## v.2.4.1 - 05 Feb, 2017
+
+Bugfixes
+
+-  Fixed minimal memory consumtion overhead for chunked buffers
+
 ## v.2.4.0 - 25 Jan, 2017
 
 Features

lib/parser.js

@@ -450,11 +450,12 @@ function concatBulkString (parser) {
   var chunks = list.length
   var offset = parser.bigStrSize - parser.totalChunkSize
   parser.offset = offset
-  if (offset === 1) {
+  if (offset <= 2) {
     if (chunks === 2) {
-      return list[0].toString('utf8', parser.bigOffset, list[0].length - 1)
+      return list[0].toString('utf8', parser.bigOffset, list[0].length + offset - 2)
     }
     chunks--
+    offset = list[list.length - 2].length + offset
   }
   var res = decoder.write(list[0].slice(parser.bigOffset))
   for (var i = 1; i < chunks - 1; i++) {
@@ -478,25 +479,24 @@ function concatBulkBuffer (parser) {
   var length = parser.bigStrSize - parser.bigOffset - 2
   var offset = parser.bigStrSize - parser.totalChunkSize
   parser.offset = offset
-  if (offset === 1) {
+  if (offset <= 2) {
     if (chunks === 2) {
-      return list[0].slice(parser.bigOffset, list[0].length - 1)
+      return list[0].slice(parser.bigOffset, list[0].length + offset - 2)
     }
     chunks--
-    offset = list[list.length - 1].length + 1
+    offset = list[list.length - 2].length + offset
   }
   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
+  var start = bufferOffset
+  list[0].copy(bufferPool, start, parser.bigOffset, list[0].length)
+  bufferOffset += list[0].length - parser.bigOffset
+  for (var i = 1; i < chunks - 1; i++) {
+    list[i].copy(bufferPool, bufferOffset)
+    bufferOffset += list[i].length
   }
-  list[i].copy(bufferPool, pos, 0, offset - 2)
-  var buffer = bufferPool.slice(bufferOffset, length + bufferOffset)
-  bufferOffset += length
-  return buffer
+  list[i].copy(bufferPool, bufferOffset, 0, offset - 2)
+  bufferOffset += offset - 2
+  return bufferPool.slice(start, bufferOffset)
 }
 
 /**

test/parsers.spec.js

@@ -574,7 +574,6 @@ describe('parsers', function () {
         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'))