LVGTM too. Maybe worth applying to the non-basic Stringify too.
http://codereview.chromium.org/5567005/diff/1/src/json.js File src/json.js (right): http://codereview.chromium.org/5567005/diff/1/src/json.js#newcode210 src/json.js:210: builder.push(","); 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. http://codereview.chromium.org/5567005/diff/1/src/json.js#newcode258 src/json.js:258: builder.push(($isFinite(value) ? $String(value) : "null")); 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) http://codereview.chromium.org/5567005/diff/1/src/json.js#newcode264 src/json.js:264: value = $Number(value); use %_ValueOf to extract the number. http://codereview.chromium.org/5567005/diff/1/src/json.js#newcode265 src/json.js:265: builder.push(($isFinite(value) ? $String(value) : "null")); %_NumberToString here too. http://codereview.chromium.org/5567005/diff/1/src/json.js#newcode270 src/json.js:270: } else { // Regular non-wrapped object I would put the comment on the next line instead. http://codereview.chromium.org/5567005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
