querystring: improve parse() performance

These changes improve parse() performance from ~11-30% on all of
the existing querystring benchmarks.

PR-URL: https://github.com/nodejs/node/pull/4675
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

diff --git a/benchmark/querystring/querystring-parse.js b/benchmark/querystring/querystring-parse.js
index 3d88bed..6a4d9f5 100644 (file)
--- a/benchmark/querystring/querystring-parse.js
+++ b/benchmark/querystring/querystring-parse.js
@@ -3,7 +3,7 @@ var querystring = require('querystring');
 var v8 = require('v8');
 
 var bench = common.createBenchmark(main, {
-  type: ['noencode', 'encodemany', 'encodelast'],
+  type: ['noencode', 'encodemany', 'encodelast', 'multivalue'],
   n: [1e6],
 });
 
@@ -14,7 +14,8 @@ function main(conf) {
   var inputs = {
     noencode: 'foo=bar&baz=quux&xyzzy=thud',
     encodemany: '%66%6F%6F=bar&%62%61%7A=quux&xyzzy=%74h%75d',
-    encodelast: 'foo=bar&baz=quux&xyzzy=thu%64'
+    encodelast: 'foo=bar&baz=quux&xyzzy=thu%64',
+    multivalue: 'foo=bar&foo=baz&foo=quux&quuy=quuz'
   };
   var input = inputs[type];
 

diff --git a/lib/querystring.js b/lib/querystring.js
index 5f8df93..b034635 100644 (file)
--- a/lib/querystring.js
+++ b/lib/querystring.js
@@ -209,7 +209,6 @@ QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
     return obj;
   }
 
-  var regexp = /\+/g;
   qs = qs.split(sep);
 
   var maxKeys = 1000;
@@ -230,7 +229,9 @@ QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
 
   var keys = [];
   for (var i = 0; i < len; ++i) {
-    const x = qs[i].replace(regexp, '%20');
+    // replacePlus() is used instead of a regexp because it is ~15-30% faster
+    // with v8 4.7
+    const x = replacePlus(qs[i]);
     const idx = x.indexOf(eq);
     var k, v;
 
@@ -242,10 +243,14 @@ QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
       v = '';
     }
 
+    // Use a key array lookup instead of using hasOwnProperty(), which is slower
     if (keys.indexOf(k) === -1) {
       obj[k] = v;
       keys.push(k);
-    } else if (Array.isArray(obj[k])) {
+    } else if (obj[k] instanceof Array) {
+      // `instanceof Array` is used instead of Array.isArray() because it is
+      // ~15-20% faster with v8 4.7 and is safe to use because we are using it
+      // with values being created within this function
       obj[k].push(v);
     } else {
       obj[k] = [obj[k], v];
@@ -256,6 +261,25 @@ QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
 };
 
 
+function replacePlus(str) {
+  var ret = '';
+  var start = 0;
+  var i = -1;
+  while ((i = str.indexOf('+', i + 1)) !== -1) {
+    ret += str.slice(start, i);
+    ret += '%20';
+    start = i + 1;
+  }
+  if (start === 0)
+    return str;
+  if (start < str.length)
+    ret += str.slice(start);
+  return ret;
+}
+
+
+// v8 does not optimize functions with try-catch blocks, so we isolate them here
+// to minimize the damage
 function decodeStr(s, decoder) {
   try {
     return decoder(s);