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

Issue 7077004: JSON.stringify improvement. Fast case in C++ for string arrays. (Closed)

Created:
9 years, 7 months ago by sandholm
Modified:
9 years, 7 months ago
Reviewers:
Lasse Reichstein, Rico
CC:
v8-dev
Visibility:
Public.

Description

JSON.stringify improvement. Fast case in C++ for string arrays.

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 8

Patch Set 3 : '' #

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -46 lines) Patch
M src/json.js View 1 2 3 1 chunk +15 lines, -10 lines 0 comments Download
M src/runtime.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime.cc View 1 2 3 6 chunks +141 lines, -36 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
sandholm
9 years, 7 months ago (2011-05-26 10:51:10 UTC) #1
Lasse Reichstein
LGTM http://codereview.chromium.org/7077004/diff/3001/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/7077004/diff/3001/src/runtime.cc#newcode3538 src/runtime.cc:3538: ASSERT(2 <= radix && radix <= 36); ASSERT ...
9 years, 7 months ago (2011-05-26 11:24:43 UTC) #2
sandholm
9 years, 7 months ago (2011-05-26 11:41:36 UTC) #3
http://codereview.chromium.org/7077004/diff/3001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7077004/diff/3001/src/runtime.cc#newcode3538
src/runtime.cc:3538: ASSERT(2 <= radix && radix <= 36);
On 2011/05/26 11:24:43, Lasse Reichstein wrote:
> ASSERT -> RUNTIME_ASSERT

Done.

http://codereview.chromium.org/7077004/diff/3001/src/runtime.cc#newcode3548
src/runtime.cc:3548: }
On 2011/05/26 11:24:43, Lasse Reichstein wrote:
> Maybe we should have special case for < radix^2 too, for cached two-byte
> strings.

Good idea (for another CL).

http://codereview.chromium.org/7077004/diff/3001/src/runtime.cc#newcode5102
src/runtime.cc:5102: SinkChar* write_cursor,
On 2011/05/26 11:24:43, Lasse Reichstein wrote:
> indentation.

Done.

http://codereview.chromium.org/7077004/diff/3001/src/runtime.cc#newcode5241
src/runtime.cc:5241: }
On 2011/05/26 11:24:43, Lasse Reichstein wrote:
> I dare you to put an
>  AssertNoAllocation no_gc;
> here.

Done.

Powered by Google App Engine
This is Rietveld 408576698