Skip to content

Commit 37ccf49

Browse files
author
Ruben Bridgewater
committed
Balance between increasing and decreasing the bufferPool
1 parent 2e6b05f commit 37ccf49

File tree

2 files changed

+40
-20
lines changed

2 files changed

+40
-20
lines changed

lib/parser.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ var ReplyError = require('./replyError')
44
var bufferPool = new Buffer(32 * 1024)
55
var bufferOffset = 0
66
var interval = null
7+
var counter = 0
8+
var notDecreased = 0
79

810
/**
911
* Used for lengths and numbers only, faster perf on arrays / bulks
@@ -307,11 +309,19 @@ function concatBulkString (parser) {
307309
* @returns {undefined}
308310
*/
309311
function decreaseBufferPool () {
310-
if (bufferPool.length > 96 * 1024) {
311-
// Decrease the bufferPool by 16kb
312-
bufferPool = bufferPool.slice(0, bufferPool.length - 16 * 1024)
312+
if (bufferPool.length > 50 * 1024) {
313+
// Balance between increasing and decreasing the bufferPool
314+
if (counter === 1 || notDecreased > counter * 2) {
315+
// Decrease the bufferPool by 16kb by removing the first 16kb of the current pool
316+
bufferPool = bufferPool.slice(16 * 1024, bufferPool.length)
317+
} else {
318+
notDecreased++
319+
counter--
320+
}
313321
} else {
314322
clearInterval(interval)
323+
counter = 0
324+
notDecreased = 0
315325
interval = null
316326
}
317327
}
@@ -330,6 +340,7 @@ function concatBuffer (parser, length) {
330340
// Increase the bufferPool size by three times the current needed length
331341
bufferPool = new Buffer(length * 3 + bufferOffset)
332342
bufferOffset = 0
343+
counter++
333344
pos = 0
334345
if (interval === null) {
335346
interval = setInterval(decreaseBufferPool, 30)

test/parsers.spec.js

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,10 @@ describe('parsers', function () {
607607
chunks[i] = new Buffer(bigStringArray.join(' ') + '.') // Math.pow(2, 16) chars long
608608
}
609609
var replyCount = 0
610+
var replies = []
611+
var jsParser = Parser.name === 'JavascriptRedisParser'
610612
function checkReply (reply) {
613+
replies.push(reply)
611614
replyCount++
612615
}
613616
var parser = new Parser({
@@ -620,29 +623,35 @@ describe('parsers', function () {
620623
assert.strictEqual(replyCount, 0)
621624
parser.execute(startBigBuffer)
622625
for (i = 0; i < 2; i++) {
623-
if (Parser.name === 'JavascriptReplyParser') {
626+
if (jsParser) {
624627
assert.strictEqual(parser.bufferCache.length, i + 1)
625628
}
626629
parser.execute(chunks[i])
627630
}
628631
assert.strictEqual(replyCount, 1)
629632
parser.execute(new Buffer('\r\n'))
630633
assert.strictEqual(replyCount, 2)
631-
parser.execute(new Buffer('+test'))
632-
assert.strictEqual(replyCount, 2)
633-
parser.execute(startSecondBigBuffer)
634-
for (i = 0; i < 4; i++) {
635-
if (Parser.name === 'JavascriptReplyParser') {
636-
assert.strictEqual(parser.bufferCache.length, i + 1)
634+
setTimeout(function () {
635+
parser.execute(new Buffer('+test'))
636+
assert.strictEqual(replyCount, 2)
637+
parser.execute(startSecondBigBuffer)
638+
for (i = 0; i < 4; i++) {
639+
if (jsParser) {
640+
assert.strictEqual(parser.bufferCache.length, i + 1)
641+
}
642+
parser.execute(chunks[i])
637643
}
638-
parser.execute(chunks[i])
639-
}
640-
assert.strictEqual(replyCount, 3)
641-
parser.execute(new Buffer('\r\n'))
642-
assert.strictEqual(replyCount, 4)
644+
assert.strictEqual(replyCount, 3)
645+
parser.execute(new Buffer('\r\n'))
646+
assert.strictEqual(replyCount, 4)
647+
}, 200)
643648
// Delay done so the bufferPool is cleared and tested
644-
// If the buffer is not cleared, the coverage is not going to be at 100%
645-
setTimeout(done, 600)
649+
// If the buffer is not cleared, the coverage is not going to be at 100
650+
setTimeout(function () {
651+
var totalBuffer = Buffer.concat(chunks).toString()
652+
assert.strictEqual(replies[3].toString(), totalBuffer)
653+
done()
654+
}, (jsParser ? 1800 : 250))
646655
})
647656

648657
it('handle big data', function () {
@@ -668,7 +677,7 @@ describe('parsers', function () {
668677
})
669678
parser.execute(startBigBuffer)
670679
for (i = 0; i < 64; i++) {
671-
if (Parser.name === 'JavascriptReplyParser') {
680+
if (Parser.name === 'JavascriptRedisParser') {
672681
assert.strictEqual(parser.bufferCache.length, i + 1)
673682
}
674683
parser.execute(chunks[i])
@@ -701,7 +710,7 @@ describe('parsers', function () {
701710
parser.execute(new Buffer('+test'))
702711
parser.execute(startBigBuffer)
703712
for (i = 0; i < 64; i++) {
704-
if (Parser.name === 'JavascriptReplyParser') {
713+
if (Parser.name === 'JavascriptRedisParser') {
705714
assert.strictEqual(parser.bufferCache.length, i + 1)
706715
}
707716
parser.execute(chunks[i])
@@ -735,7 +744,7 @@ describe('parsers', function () {
735744
})
736745
parser.execute(startBigBuffer)
737746
for (i = 0; i < 64; i++) {
738-
if (Parser.name === 'JavascriptReplyParser') {
747+
if (Parser.name === 'JavascriptRedisParser') {
739748
assert.strictEqual(parser.bufferCache.length, i + 1)
740749
}
741750
parser.execute(chunks[i])

0 commit comments

Comments
 (0)