JSON stringify collects substrings in one builder array rather than using reg...

src/json.js

 // Copyright 2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 177 matching lines @@
       } else {
         return SerializeObject(value, replacer, stack, indent, gap);
       }
     case "number":
       return $isFinite(value) ? $String(value) : "null";
     case "boolean":
       return value ? "true" : "false";
   }
 }
 
-function BasicSerializeArray(value, stack) {
+
+function BasicSerializeArray(value, stack, builder) {
   if (StackContains(stack, value)) {
     throw MakeTypeError('circular_structure', []);
   }
   stack.push(value);
-  var partial = [];
+  builder.push("[");
   var len = value.length;
   for (var i = 0; i < len; i++) {
-    var strP = BasicJSONSerialize($String(i), value, stack);
-    if (IS_UNDEFINED(strP)) strP = "null";
-    partial.push(strP);
+    var before = builder.length;
+    BasicJSONSerialize($String(i), value, stack, builder);
+    if (before == builder.length) builder.push("null");
+    builder.push(",");

[Lasse Reichstein, 2010/12/06 08:48:45]

An idiom often used in "picket fence loops" like this is 
to start with 
 if (i > 0) builder.push(",");
The overhead of the BasicJSONSerialize call will overshadow the overhead of an
extra test inside the loop, so I doubt it will be visible in the performance.

   }
   stack.pop();
-  return "[" + partial.join() + "]";
+  if (builder.pop() != ",") {

[Erik Corry, 2010/12/05 15:09:06]

This seems a bit indirect.  I think it would be clearer to just say if (len ==
0).  Is this version faster?

[sandholm, 2010/12/06 11:42:31]

I added a comment.
On 2010/12/05 15:09:06, Erik Corry wrote:

+    builder.push("[]");
+  } else {
+    builder.push("]");
+  }
 }
 
-function BasicSerializeObject(value, stack) {
+
+function BasicSerializeObject(value, stack, builder) {
   if (StackContains(stack, value)) {
     throw MakeTypeError('circular_structure', []);
   }
   stack.push(value);
-  var partial = [];
+  builder.push("{");
   for (var p in value) {
     if (ObjectHasOwnProperty.call(value, p)) {
-      var strP = BasicJSONSerialize(p, value, stack);
-      if (!IS_UNDEFINED(strP)) partial.push(%QuoteJSONString(p) + ":" + strP);
+      builder.push(%QuoteJSONString(p), ":");

[Erik Corry, 2010/12/05 15:09:06]

If you didn't already try it I think it would be interesting to experiment with
having QuoteJSONString not add the quotes.  We should be faster at adding the
quotes now and it would open up the possibility of just returning the string
unchanged which should be the common case.  We can save that for another change
though.

[sandholm, 2010/12/06 11:42:31]

I experimented a bit. Will submit another change.
On 2010/12/05 15:09:06, Erik Corry wrote:

+      var before = builder.length;
+      BasicJSONSerialize(p, value, stack, builder);
+      if (before == builder.length) {
+        builder.pop();
+        builder.pop();
+      } else {
+        builder.push(",");
+      }
     }
   }
   stack.pop();
-  return "{" + partial.join() + "}";
+  if (builder.pop() != ",") {

[Erik Corry, 2010/12/05 15:09:06]

Again, I find this convoluted.  A boolean variable that gets set if the for loop
is non-empty would be clearer.

[sandholm, 2010/12/06 11:42:31]

I added a comment
On 2010/12/05 15:09:06, Erik Corry wrote:

+    builder.push("{}");
+  } else {
+    builder.push("}");
+  }
 }
 
-function BasicJSONSerialize(key, holder, stack) {
+
+function BasicJSONSerialize(key, holder, stack, builder) {
   var value = holder[key];
   if (IS_OBJECT(value) && value) {
     var toJSON = value.toJSON;
     if (IS_FUNCTION(toJSON)) value = toJSON.call(value, key);
   }
-  // Unwrap value if necessary
-  if (IS_OBJECT(value)) {
+  if (IS_STRING(value)) {
+    builder.push(%QuoteJSONString(value));
+  } else if (IS_NUMBER(value)) {
+    builder.push(($isFinite(value) ? $String(value) : "null"));

[Erik Corry, 2010/12/05 15:09:06]

I wonder how $String compares speedwise to ("" + value).

[Lasse Reichstein, 2010/12/06 08:48:45]

Use %_NumberToString instead of the generic $String. You know it's a number. (Or
use NonStringToString, but it will just test for being a number and call
%_NumberToString)

[sandholm, 2010/12/06 11:42:31]

Done.

[sandholm, 2010/12/06 11:42:31]

Done.

+  } else if (IS_BOOLEAN(value)) {
+    builder.push((value ? "true" : "false"));
+  } else if (IS_OBJECT(value)) {
+    // Unwrap value if necessary
     if (IS_NUMBER_WRAPPER(value)) {
       value = $Number(value);

[Lasse Reichstein, 2010/12/06 08:48:45]

use %_ValueOf to extract the number.

[sandholm, 2010/12/06 11:42:31]

Done.

+      builder.push(($isFinite(value) ? $String(value) : "null"));      

[Lasse Reichstein, 2010/12/06 08:48:45]

%_NumberToString here too.

[sandholm, 2010/12/06 11:42:31]

Done.

     } else if (IS_STRING_WRAPPER(value)) {
-      value = $String(value);
+      builder.push(%QuoteJSONString($String(value)));
     } else if (IS_BOOLEAN_WRAPPER(value)) {
-      value =  %_ValueOf(value);
+      builder.push((%_ValueOf(value) ? "true" : "false")); 
+    } else { // Regular non-wrapped object

[Erik Corry, 2010/12/05 15:09:06]

There should be 2 spaces before //

[Lasse Reichstein, 2010/12/06 08:48:45]

I would put the comment on the next line instead.

[sandholm, 2010/12/06 11:42:31]

Done.

[sandholm, 2010/12/06 11:42:31]

Done.

+      if (!value) {
+        builder.push("null");
+      } else if (IS_ARRAY(value)) {
+        BasicSerializeArray(value, stack, builder);
+      } else {
+        BasicSerializeObject(value, stack, builder);
+      }
     }
   }
-  switch (typeof value) {
-    case "string":
-      return %QuoteJSONString(value);
-    case "object":
-      if (!value) {
-        return "null";
-      } else if (IS_ARRAY(value)) {
-        return BasicSerializeArray(value, stack);
-      } else {
-        return BasicSerializeObject(value, stack);
-      }
-    case "number":
-      return $isFinite(value) ? $String(value) : "null";
-    case "boolean":
-      return value ? "true" : "false";
-  }
 }
 
 function JSONStringify(value, replacer, space) {
   if (IS_UNDEFINED(replacer) && IS_UNDEFINED(space)) {
-    return BasicJSONSerialize('', {'': value}, []);
+    var builder = [];
+    BasicJSONSerialize('', {'': value}, [], builder);
+    if (builder.length == 0) return;
+    else return %StringBuilderConcat(builder, builder.length, "");
   }
   if (IS_OBJECT(space)) {
     // Unwrap 'space' if it is wrapped
     if (IS_NUMBER_WRAPPER(space)) {
       space = $Number(space);
     } else if (IS_STRING_WRAPPER(space)) {
       space = $String(space);
     }
   }
   var gap;
@@ skipping 16 matching lines @@
 }
 
 function SetupJSON() {
   InstallFunctions($JSON, DONT_ENUM, $Array(
     "parse", JSONParse,
     "stringify", JSONStringify
   ));
 }
 
 SetupJSON();