Eagerly parse expected transitions in JSON.

Eagerly parse expected transitions in JSON.

Committed: https://code.google.com/p/v8/source/detail?r=14191

[Toon Verwaest, 2013-04-08 16:09:11]

PTAL

[Yang, 2013-04-09 15:47:00]

LGTM once comments are addressed.

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

https://codereview.chromium.org/13741010/diff/5001/src/json-parser.h#newcode118
src/json-parser.h:118: c0 == '"' || c0 < 0x20 || c0 == '\\') {
maybe you also need to check for 0x7f, which is also a control character?

https://codereview.chromium.org/13741010/diff/5001/src/json-parser.h#newcode130
src/json-parser.h:130: }
I guess an empty line here would make sense.

https://codereview.chromium.org/13741010/diff/5001/src/json-parser.h#newcode133
src/json-parser.h:133: }
empty line here as well.

https://codereview.chromium.org/13741010/diff/5001/src/json-parser.h#newcode325
src/json-parser.h:325: ZoneList<Handle<Object> > properties(8, zone());
You could reuse one zone list through out the entire JSON parser, so that you
don't have to create new lists all the time. Just Clear() it after use.

https://codereview.chromium.org/13741010/diff/5001/src/json-parser.h#newcode390
src/json-parser.h:390: // If the expected transition failed, parse an internal
string and try
"internalized string"

https://codereview.chromium.org/13741010/diff/5001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/13741010/diff/5001/src/objects-inl.h#newcode1527
src/objects-inl.h:1527: Handle<Map>
JSObject::ExpectedTransitionTarget(Handle<Map> map) {
Maybe add an assert that this can only follow after ExpectedTransitionKey
returned something reasonable.

[Toon Verwaest, 2013-04-09 16:47:59]

Addressed comments, landing.

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/json-parser.h
File src/json-parser.h (right):

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/json-parser.h#n...
src/json-parser.h:118: c0 == '"' || c0 < 0x20 || c0 == '\\') {
On 2013/04/09 15:47:00, Yang wrote:
> maybe you also need to check for 0x7f, which is also a control character?

Irrelevant, as discussed offline.

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/json-parser.h#n...
src/json-parser.h:130: }
On 2013/04/09 15:47:00, Yang wrote:
> I guess an empty line here would make sense.

Done.

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/json-parser.h#n...
src/json-parser.h:133: }
On 2013/04/09 15:47:00, Yang wrote:
> empty line here as well.

Done.

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/json-parser.h#n...
src/json-parser.h:325: ZoneList<Handle<Object> > properties(8, zone());
That's unfortunately not possible since objects are nested...

On 2013/04/09 15:47:00, Yang wrote:
> You could reuse one zone list through out the entire JSON parser, so that you
> don't have to create new lists all the time. Just Clear() it after use.

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/json-parser.h#n...
src/json-parser.h:390: // If the expected transition failed, parse an internal
string and try
On 2013/04/09 15:47:00, Yang wrote:
> "internalized string"

Done.

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/objects-inl.h
File src/objects-inl.h (right):

https://chromiumcodereview.appspot.com/13741010/diff/5001/src/objects-inl.h#n...
src/objects-inl.h:1527: Handle<Map>
JSObject::ExpectedTransitionTarget(Handle<Map> map) {
On 2013/04/09 15:47:00, Yang wrote:
> Maybe add an assert that this can only follow after ExpectedTransitionKey
> returned something reasonable.

Done.

[Toon Verwaest, 2013-04-09 16:49:38]

Committed patchset #3 manually as r14191 (presubmit successful).