Issue 11238031: Pretenure JSON graph if the input string is larger than 100*1024 chars.

Issue 11238031: Pretenure JSON graph if the input string is larger than 100*1024 chars. (Closed)

Created:
8 years, 2 months ago by Toon Verwaest

Modified:
8 years, 2 months ago

Reviewers:
Michael Starzinger

CC:
v8-dev

Base URL:
https://v8.googlecode.com/svn/branches/bleeding_edge

Visibility:
Public.

More Reviews

Description

Pretenure JSON graph if the input string is larger than 100*1024 chars. Committed: https://code.google.com/p/v8/source/detail?r=12795

Patch Set 3 : Addressed comments #

Created: 8 years, 2 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+24 lines, -13 lines)			Patch
	M	src/json-parser.h	View	1 2	9 chunks	+24 lines, -13 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Michael Starzinger

LGTM (with a few nits). https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h File src/json-parser.h (right): https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h#newcode164 src/json-parser.h:164: PretenureFlag tenure_; Let's call ...

8 years, 2 months ago (2012-10-22 16:44:28 UTC) #2

Toon Verwaest

8 years, 2 months ago (2012-10-23 08:05:57 UTC) #3

Addressed comments.

https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h
File src/json-parser.h (right):

https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h#newcode164
src/json-parser.h:164: PretenureFlag tenure_;
On 2012/10/22 16:44:28, Michael Starzinger wrote:
> Let's call that "pretenure_" to be in line with the rest of the code-base.

Done.

https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h#newcode184
src/json-parser.h:184: static const int kJsonAllocatePretenured = 100 * 1024;
On 2012/10/22 16:44:28, Michael Starzinger wrote:
> Can we move this constant out into the JsonParser class. And I would rename it
> to kPretenureThreshold or something like that.

Done.

https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h#newcode463
src/json-parser.h:463: PretenureFlag tenure);
On 2012/10/22 16:44:28, Michael Starzinger wrote:
> Call the third argument "pretenure".

Done.

https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h#newcode468
src/json-parser.h:468: PretenureFlag tenure) {
On 2012/10/22 16:44:28, Michael Starzinger wrote:
> Likewise.

Done.

https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h#newcode475
src/json-parser.h:475: PretenureFlag tenure) {
On 2012/10/22 16:44:28, Michael Starzinger wrote:
> Likewise.

Done.

https://codereview.chromium.org/11238031/diff/1002/src/json-parser.h#newcode670
src/json-parser.h:670: beg_pos,
Seems fine here.

On 2012/10/22 16:44:28, Michael Starzinger wrote:
> Indentation seems off.

Expand Messages | Collapse Messages