Skip to content

Commit 200a8cf

Browse files
author
Ruben Bridgewater
committed
Improve parsing very big chunks
Also free the bufferPool over time again
1 parent 6f82d62 commit 200a8cf

File tree

2 files changed

+181
-18
lines changed

2 files changed

+181
-18
lines changed

‎lib/parser.js

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
'use strict'
22

33
var ReplyError = require('./replyError')
4-
// TODO: Consider shrinking the bufferPool if it's not used a lot by using interval check
54
var bufferPool = new Buffer(64 * 1024)
5+
var interval = null
66

77
/**
88
* Used for lengths and numbers only, faster perf on arrays / bulks
@@ -133,9 +133,10 @@ function parseBulkString (parser) {
133133
}
134134
var offsetEnd = parser.offset + length
135135
if (offsetEnd + 2 > parser.buffer.length) {
136-
parser.bufferCache.push(parser.buffer)
137-
parser.totalChunkSize = parser.buffer.length
138136
parser.bigStrSize = offsetEnd + 2
137+
parser.bigOffset = parser.offset
138+
parser.totalChunkSize = parser.buffer.length
139+
parser.bufferCache.push(parser.buffer)
139140
return
140141
}
141142

@@ -258,10 +259,56 @@ function JavascriptRedisParser (options) {
258259
this.offset = 0
259260
this.buffer = null
260261
this.bigStrSize = 0
262+
this.bigOffset = 0
261263
this.totalChunkSize = 0
262264
this.bufferCache = []
263265
}
264266

267+
/**
268+
* Concat a bulk string containing multiple chunks
269+
* @param parser
270+
* @param buffer
271+
* @returns {String}
272+
*/
273+
function concatBigString (parser, buffer) {
274+
var list = parser.bufferCache
275+
var length = parser.bigStrSize
276+
var stringSize = length - parser.bigOffset - 2
277+
var res = list[0].toString('utf8', parser.bigOffset)
278+
for (var i = 1; i < list.length - 1; i++) {
279+
res += list[i].toString()
280+
}
281+
if (i > 1) {
282+
if (res.length + list[i].length <= stringSize) {
283+
res += list[i].toString()
284+
} else {
285+
res += list[i].toString('utf8', 0, list[i].length - 1)
286+
}
287+
}
288+
var offset = stringSize - res.length
289+
if (offset > 0) {
290+
res += buffer.toString('utf8', 0, offset)
291+
}
292+
parser.offset = offset + 2
293+
parser.bigOffset = 0
294+
return res
295+
}
296+
297+
/**
298+
* Decrease the bufferPool size over time
299+
* @returns {undefined}
300+
*/
301+
function decreaseBufferPool () {
302+
if (bufferPool.length > 96 * 1024) {
303+
// Decrease the bufferPool by 16kb
304+
bufferPool = bufferPool.slice(0, bufferPool.length - 16 * 1024)
305+
// console.log(bufferPool.length)
306+
} else {
307+
clearInterval(interval)
308+
interval = null
309+
}
310+
}
311+
265312
/**
266313
* Concat the collected chunks from parser.bufferCache
267314
* @param parser
@@ -273,6 +320,9 @@ function concat (parser, length) {
273320
var pos = 0
274321
if (bufferPool.length < length) {
275322
bufferPool = new Buffer(length)
323+
if (interval === null) {
324+
interval = setInterval(decreaseBufferPool, 50)
325+
}
276326
}
277327
for (var i = 0; i < list.length; i++) {
278328
list[i].copy(bufferPool, pos)
@@ -289,6 +339,7 @@ function concat (parser, length) {
289339
JavascriptRedisParser.prototype.execute = function (buffer) {
290340
if (this.buffer === null) {
291341
this.buffer = buffer
342+
this.offset = 0
292343
} else if (this.bigStrSize === 0) {
293344
var oldLength = this.buffer.length
294345
var remainingLength = oldLength - this.offset
@@ -301,9 +352,17 @@ JavascriptRedisParser.prototype.execute = function (buffer) {
301352
this.buffer.copy(newBuffer, 0, this.offset, oldLength)
302353
buffer.copy(newBuffer, remainingLength, 0, buffer.length)
303354
this.buffer = newBuffer.slice(0, newLength)
355+
this.offset = 0
304356
} else if (this.totalChunkSize + buffer.length >= this.bigStrSize) {
305-
this.bufferCache.push(buffer)
306-
this.buffer = concat(this, this.totalChunkSize + buffer.length)
357+
// The returned type might be Array * (42) and in that case we can't improve the parsing currently
358+
if (this.optionReturnBuffers === false && this.buffer[this.offset] === 36) {
359+
this.returnReply(concatBigString(this, buffer))
360+
this.buffer = buffer
361+
} else { // This applies for arrays too
362+
this.bufferCache.push(buffer)
363+
this.buffer = concat(this, this.totalChunkSize + buffer.length)
364+
this.offset = 0
365+
}
307366
this.bigStrSize = 0
308367
this.totalChunkSize = 0
309368
this.bufferCache = []
@@ -313,8 +372,6 @@ JavascriptRedisParser.prototype.execute = function (buffer) {
313372
return
314373
}
315374

316-
this.offset = 0
317-
318375
while (this.offset < this.buffer.length) {
319376
var offset = this.offset
320377
var type = this.buffer[this.offset++]

‎test/parsers.spec.js

Lines changed: 117 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,32 @@ describe('parsers', function () {
4040

4141
parsers.forEach(function (Parser) {
4242
describe(Parser.name, function () {
43+
it('chunks getting to big for the bufferPool', function () {
44+
// This is a edge case. Chunks should not exceed Math.pow(2, 16) bytes
45+
var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' +
46+
'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' +
47+
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' +
48+
'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars
49+
var bigString = (new Array(Math.pow(2, 17) / lorem.length).join(lorem)) // Math.pow(2, 17) chars long
50+
var replyCount = 0
51+
function checkReply (reply) {
52+
replyCount++
53+
}
54+
var parser = new Parser({
55+
returnReply: checkReply,
56+
returnError: returnError,
57+
returnFatalError: returnFatalError
58+
})
59+
parser.execute(new Buffer('+test'))
60+
assert.strictEqual(replyCount, 0)
61+
parser.execute(new Buffer('\r\n+'))
62+
assert.strictEqual(replyCount, 1)
63+
parser.execute(new Buffer(bigString))
64+
assert.strictEqual(replyCount, 1)
65+
parser.execute(new Buffer('\r\n'))
66+
assert.strictEqual(replyCount, 2)
67+
})
68+
4369
it('handles multi-bulk reply and check context binding', function () {
4470
var replyCount = 0
4571
function Abc () {}
@@ -430,9 +456,6 @@ describe('parsers', function () {
430456
})
431457

432458
it('handle big numbers', function () {
433-
if (Parser.name === 'HiredisReplyParser') {
434-
return this.skip()
435-
}
436459
var replyCount = 0
437460
var number = 9007199254740991 // Number.MAX_SAFE_INTEGER
438461
function checkReply (reply) {
@@ -450,10 +473,58 @@ describe('parsers', function () {
450473
assert.strictEqual(replyCount, 2)
451474
})
452475

453-
it('handle big data', function () {
454-
if (Parser.name === 'HiredisReplyParser') {
455-
return this.skip()
476+
it('handle big data with buffers', function (done) {
477+
var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' +
478+
'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' +
479+
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' +
480+
'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars
481+
var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long
482+
var startBigBuffer = new Buffer('\r\n$' + (128 * 1024) + '\r\n')
483+
var startSecondBigBuffer = new Buffer('\r\n$' + (256 * 1024) + '\r\n')
484+
var chunks = new Array(4)
485+
for (var i = 0; i < 4; i++) {
486+
chunks[i] = new Buffer(bigStringArray.join(' ') + '.') // Math.pow(2, 16) chars long
487+
}
488+
var replyCount = 0
489+
function checkReply (reply) {
490+
replyCount++
456491
}
492+
var parser = new Parser({
493+
returnReply: checkReply,
494+
returnError: returnError,
495+
returnFatalError: returnFatalError,
496+
returnBuffers: true
497+
})
498+
parser.execute(new Buffer('+test'))
499+
assert.strictEqual(replyCount, 0)
500+
parser.execute(startBigBuffer)
501+
for (i = 0; i < 2; i++) {
502+
if (Parser.name === 'JavascriptReplyParser') {
503+
assert.strictEqual(parser.bufferCache.length, i + 1)
504+
}
505+
parser.execute(chunks[i])
506+
}
507+
assert.strictEqual(replyCount, 1)
508+
parser.execute(new Buffer('\r\n'))
509+
assert.strictEqual(replyCount, 2)
510+
parser.execute(new Buffer('+test'))
511+
assert.strictEqual(replyCount, 2)
512+
parser.execute(startSecondBigBuffer)
513+
for (i = 0; i < 4; i++) {
514+
if (Parser.name === 'JavascriptReplyParser') {
515+
assert.strictEqual(parser.bufferCache.length, i + 1)
516+
}
517+
parser.execute(chunks[i])
518+
}
519+
assert.strictEqual(replyCount, 3)
520+
parser.execute(new Buffer('\r\n'))
521+
assert.strictEqual(replyCount, 4)
522+
// Delay done so the bufferPool is cleared and tested
523+
// If the buffer is not cleared, the coverage is not going to be at 100%
524+
setTimeout(done, 600)
525+
})
526+
527+
it('handle big data', function () {
457528
var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' +
458529
'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' +
459530
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' +
@@ -476,7 +547,9 @@ describe('parsers', function () {
476547
})
477548
parser.execute(startBigBuffer)
478549
for (i = 0; i < 64; i++) {
479-
assert.strictEqual(parser.bufferCache.length, i + 1)
550+
if (Parser.name === 'JavascriptReplyParser') {
551+
assert.strictEqual(parser.bufferCache.length, i + 1)
552+
}
480553
parser.execute(chunks[i])
481554
}
482555
assert.strictEqual(replyCount, 0)
@@ -485,9 +558,6 @@ describe('parsers', function () {
485558
})
486559

487560
it('handle big data 2', function () {
488-
if (Parser.name === 'HiredisReplyParser') {
489-
return this.skip()
490-
}
491561
var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' +
492562
'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' +
493563
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' +
@@ -510,13 +580,49 @@ describe('parsers', function () {
510580
parser.execute(new Buffer('+test'))
511581
parser.execute(startBigBuffer)
512582
for (i = 0; i < 64; i++) {
513-
assert.strictEqual(parser.bufferCache.length, i + 1)
583+
if (Parser.name === 'JavascriptReplyParser') {
584+
assert.strictEqual(parser.bufferCache.length, i + 1)
585+
}
514586
parser.execute(chunks[i])
515587
}
516588
assert.strictEqual(replyCount, 1)
517589
parser.execute(new Buffer('\r\n'))
518590
assert.strictEqual(replyCount, 2)
519591
})
592+
593+
it('handle big data 2 with buffers', function () {
594+
var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' +
595+
'sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' +
596+
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ' +
597+
'ut aliquip ex ea commodo consequat. Duis aute irure dolor in' // 256 chars
598+
var bigStringArray = (new Array(Math.pow(2, 16) / lorem.length).join(lorem + ' ')).split(' ') // Math.pow(2, 16) chars long
599+
var startBigBuffer = new Buffer('$' + (4 * 1024 * 1024) + '\r\n')
600+
var chunks = new Array(64)
601+
for (var i = 0; i < 64; i++) {
602+
chunks[i] = new Buffer(bigStringArray.join(' ') + '.') // Math.pow(2, 16) chars long
603+
}
604+
var replyCount = 0
605+
function checkReply (reply) {
606+
assert.strictEqual(reply.length, 4 * 1024 * 1024)
607+
replyCount++
608+
}
609+
var parser = new Parser({
610+
returnReply: checkReply,
611+
returnError: returnError,
612+
returnFatalError: returnFatalError,
613+
returnBuffers: true
614+
})
615+
parser.execute(startBigBuffer)
616+
for (i = 0; i < 64; i++) {
617+
if (Parser.name === 'JavascriptReplyParser') {
618+
assert.strictEqual(parser.bufferCache.length, i + 1)
619+
}
620+
parser.execute(chunks[i])
621+
}
622+
assert.strictEqual(replyCount, 0)
623+
parser.execute(new Buffer('\r\n'))
624+
assert.strictEqual(replyCount, 1)
625+
})
520626
})
521627
})
522628
})

0 commit comments

Comments
 (0)