Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(550)

Issue 6335004: Optimize JSON stringify by allowing QuoteJSONString to prefix with a comma. (Closed)

Created:
9 years, 11 months ago by sandholm
Modified:
9 years, 6 months ago
Reviewers:
Erik Corry
CC:
v8-dev
Visibility:
Public.

Description

Optimize JSON stringify by allowing QuoteJSONString to prefix with a comma.

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -21 lines) Patch
M src/json.js View 5 chunks +10 lines, -12 lines 4 comments Download
M src/runtime.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime.cc View 6 chunks +30 lines, -9 lines 2 comments Download

Messages

Total messages: 3 (0 generated)
sandholm
9 years, 11 months ago (2011-01-14 14:21:38 UTC) #1
Erik Corry
LGTM http://codereview.chromium.org/6335004/diff/1/src/json.js File src/json.js (right): http://codereview.chromium.org/6335004/diff/1/src/json.js#newcode244 src/json.js:244: var not_first = false; This variable should be ...
9 years, 11 months ago (2011-01-14 14:42:57 UTC) #2
sandholm
9 years, 11 months ago (2011-01-16 21:08:43 UTC) #3
http://codereview.chromium.org/6335004/diff/1/src/json.js
File src/json.js (right):

http://codereview.chromium.org/6335004/diff/1/src/json.js#newcode244
src/json.js:244: var not_first = false;
On 2011/01/14 14:42:57, Erik Corry wrote:
> This variable should be called first and the boolean values should be
inverted.

Done.

http://codereview.chromium.org/6335004/diff/1/src/json.js#newcode247
src/json.js:247: if (not_first) builder.push(%QuoteJSONStringComma(p));
On 2011/01/14 14:42:57, Erik Corry wrote:
> multi-line if statements should use {}

Done.

http://codereview.chromium.org/6335004/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6335004/diff/1/src/runtime.cc#newcode4625
src/runtime.cc:4625: static MaybeObject* SlowQuoteJsonString(Vector<const Char>
characters) {
On 2011/01/14 14:42:57, Erik Corry wrote:
> I think it would be nice to know what the performance hit of having this as a
> normal argument is, before blowing this code up with a factor of 2.

Having this as a normal argument makes the change regress performance on JSON
stringify rather than improve it.

Powered by Google App Engine
This is Rietveld 408576698