From 36c40ee03dff2b60a14d3520d449b2594c3ac9ff Mon Sep 17 00:00:00 2001 From: Matt Ranney Date: Tue, 7 Dec 2010 00:23:31 -0800 Subject: [PATCH] JavaScript parser passes all tests when returning strings. JS is still way too slow for large mb replies. Hiredis is fast for strings of large replies, but slow for buffers. --- lib/parser/javascript.js | 53 ++++++++++++--------------------- lib/queue.js | 63 ++++++++++++++++++++++++++++++++++++++++ test.js | 4 +-- 3 files changed, 84 insertions(+), 36 deletions(-) create mode 100644 lib/queue.js diff --git a/lib/parser/javascript.js b/lib/parser/javascript.js index 25dd04346d..bb47831648 100644 --- a/lib/parser/javascript.js +++ b/lib/parser/javascript.js @@ -18,10 +18,10 @@ util.inherits(RedisReplyParser, events.EventEmitter); exports.Parser = RedisReplyParser; // Buffer.toString() is quite slow for small strings -function small_toString(buf) { - var tmp = "", i, il; +function small_toString(buf, len) { + var tmp = "", i; - for (i = 0, il = buf.end; i < il; i += 1) { + for (i = 0; i < len; i += 1) { tmp += String.fromCharCode(buf[i]); } @@ -33,7 +33,8 @@ RedisReplyParser.prototype.reset = function () { this.state = "type"; this.return_buffer = new Buffer(16384); // for holding replies, might grow - this.tmp_buffer = new Buffer(128); // for holding size fields + this.return_string = ""; + this.tmp_string = ""; // for holding size fields this.multi_bulk_length = 0; this.multi_bulk_replies = null; @@ -64,22 +65,25 @@ RedisReplyParser.prototype.execute = function (incoming_buf) { case 43: // + this.state = "single line"; this.return_buffer.end = 0; + this.return_string = ""; break; case 42: // * this.state = "multi bulk count"; - this.tmp_buffer.end = 0; + this.tmp_string = ""; break; case 58: // : this.state = "integer line"; this.return_buffer.end = 0; + this.return_string = ""; break; case 36: // $ this.state = "bulk length"; - this.tmp_buffer.end = 0; + this.tmp_string = ""; break; case 45: // - this.state = "error line"; this.return_buffer.end = 0; + this.return_string = ""; break; default: this.state = "unknown type"; @@ -87,7 +91,7 @@ RedisReplyParser.prototype.execute = function (incoming_buf) { break; case "integer line": if (incoming_buf[pos] === 13) { - this.send_reply(+small_toString(this.return_buffer)); + this.send_reply(+small_toString(this.return_buffer, this.return_buffer.end)); this.state = "final lf"; } else { this.return_buffer[this.return_buffer.end] = incoming_buf[pos]; @@ -107,18 +111,10 @@ RedisReplyParser.prototype.execute = function (incoming_buf) { break; case "single line": if (incoming_buf[pos] === 13) { - if (this.return_buffer.end > 10) { - bd_str = this.return_buffer.toString("utf8", 0, this.return_buffer.end); - } else { - bd_str = small_toString(this.return_buffer); - - } - this.send_reply(bd_str); + this.send_reply(this.return_string); this.state = "final lf"; } else { - this.return_buffer[this.return_buffer.end] = incoming_buf[pos]; - this.return_buffer.end += 1; - // TODO - check for return_buffer overflow and then grow, copy, continue, and drink. + this.return_string += String.fromCharCode(incoming_buf[pos]); } pos += 1; break; @@ -126,8 +122,7 @@ RedisReplyParser.prototype.execute = function (incoming_buf) { if (incoming_buf[pos] === 13) { // \r this.state = "multi bulk count lf"; } else { - this.tmp_buffer[this.tmp_buffer.end] = incoming_buf[pos]; - this.tmp_buffer.end += 1; + this.tmp_string += String.fromCharCode(incoming_buf[pos]); } pos += 1; break; @@ -137,7 +132,7 @@ RedisReplyParser.prototype.execute = function (incoming_buf) { this.multi_bulk_nested_length = this.multi_bulk_length; this.multi_bulk_nested_replies = this.multi_bulk_replies; } - this.multi_bulk_length = +small_toString(this.tmp_buffer); + this.multi_bulk_length = +this.tmp_string; this.multi_bulk_replies = []; this.state = "type"; if (this.multi_bulk_length < 0) { @@ -157,14 +152,13 @@ RedisReplyParser.prototype.execute = function (incoming_buf) { if (incoming_buf[pos] === 13) { // \r this.state = "bulk lf"; } else { - this.tmp_buffer[this.tmp_buffer.end] = incoming_buf[pos]; - this.tmp_buffer.end += 1; + this.tmp_string += String.fromCharCode(incoming_buf[pos]); } pos += 1; break; case "bulk lf": if (incoming_buf[pos] === 10) { // \n - this.bulk_length = +small_toString(this.tmp_buffer); + this.bulk_length = +this.tmp_string; if (this.bulk_length === -1) { this.send_reply(null); this.state = "type"; @@ -251,22 +245,13 @@ RedisReplyParser.prototype.send_error = function (reply) { RedisReplyParser.prototype.send_reply = function (reply) { if (this.multi_bulk_length > 0 || this.multi_bulk_nested_length > 0) { if (!this.options.return_buffers && Buffer.isBuffer(reply)) { - if (reply.end > 10) { - this.add_multi_bulk_reply(reply.toString()); - } else { - this.add_multi_bulk_reply(small_toString(reply)); - } + this.add_multi_bulk_reply(reply.toString("utf8")); } else { this.add_multi_bulk_reply(reply); } } else { if (!this.options.return_buffers && Buffer.isBuffer(reply)) { - console.log("converting buffer to string of len " + reply.end + ", " + util.inspect(reply)); - if (reply.length > 10) { - this.emit("reply", reply.toString()); - } else { - this.emit("reply", small_toString(reply)); - } + this.emit("reply", reply.toString("utf8")); } else { this.emit("reply", reply); } diff --git a/lib/queue.js b/lib/queue.js new file mode 100644 index 0000000000..148513dc19 --- /dev/null +++ b/lib/queue.js @@ -0,0 +1,63 @@ +function to_array(args) { + var len = args.length, + arr = new Array(len), i; + + for (i = 0; i < len; i += 1) { + arr[i] = args[i]; + } + + return arr; +} + +// Queue class adapted from Tim Caswell's pattern library +// http://github.com/creationix/pattern/blob/master/lib/pattern/queue.js + +var Queue = function () { + this.tail = []; + this.head = to_array(arguments); + this.offset = 0; +}; + +Queue.prototype.shift = function () { + if (this.offset === this.head.length) { + var tmp = this.head; + tmp.length = 0; + this.head = this.tail; + this.tail = tmp; + this.offset = 0; + if (this.head.length === 0) { + return; + } + } + return this.head[this.offset++]; // sorry, JSLint +}; + +Queue.prototype.push = function (item) { + return this.tail.push(item); +}; + +Queue.prototype.forEach = function (fn, thisv) { + var array = this.head.slice(this.offset), i, il; + + array.push.apply(array, this.tail); + + if (thisv) { + for (i = 0, il = array.length; i < il; i += 1) { + fn.call(thisv, array[i], i, array); + } + } else { + for (i = 0, il = array.length; i < il; i += 1) { + fn(array[i], i, array); + } + } + + return array; +}; + +Object.defineProperty(Queue.prototype, 'length', { + get: function () { + return this.head.length - this.offset + this.tail.length; + } +}); + +exports.Queue = Queue; diff --git a/test.js b/test.js index d41d8800ec..7bc33497df 100644 --- a/test.js +++ b/test.js @@ -14,7 +14,7 @@ var redis = require("./index"), server_info; // Uncomment this to see the wire protocol and other debugging info -redis.debug_mode = true; +redis.debug_mode = false; function buffers_to_strings(arr) { return arr.map(function (val) { @@ -513,7 +513,7 @@ tests.UTF8 = function () { client.set(["utf8test", utf8_sample], require_string("OK", name)); client.get(["utf8test"], function (err, obj) { assert.strictEqual(null, err); - assert.strictEqual(utf8_sample, obj.toString('utf8')); + assert.strictEqual(utf8_sample, obj); next(name); }); };