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.
tkpage · Dec 7, 2010 · 36c40ee · 36c40ee
1 parent b907364
commit 36c40ee
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 36 deletions.
diff --git 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,30 +65,33 @@ 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";
             }
             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,27 +111,18 @@ 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;
         case "multi bulk count":
             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
@@ -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
@@ -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);
     });
 };