Eagerly follow transitions to existing maps while json parsing. (issue 11184006)

9 views
Skip to first unread message

verw...@chromium.org

unread,
Oct 16, 2012, 1:38:13 PM10/16/12
to da...@chromium.org, v8-...@googlegroups.com
Reviewers: danno,

Message:
PTAL.

Description:
Eagerly follow transitions to existing maps while json parsing.


Please review this at https://chromiumcodereview.appspot.com/11184006/

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

Affected files:
M src/json-parser.h
M src/objects.h
M src/objects.cc


Index: src/json-parser.h
diff --git a/src/json-parser.h b/src/json-parser.h
index
8eb4f67f22e1c2a7e6c175d8a0f30e1c5c119062..ed00200e0c42af52464e37f56a64f9aa215d9aec
100644
--- a/src/json-parser.h
+++ b/src/json-parser.h
@@ -289,6 +289,10 @@ Handle<Object> JsonParser<seq_ascii>::ParseJsonValue()
{
// Parse a JSON object. Position must be right at '{'.
template <bool seq_ascii>
Handle<Object> JsonParser<seq_ascii>::ParseJsonObject() {
+ ZoneScope zone_scope(zone(), DELETE_ON_EXIT);
+ ZoneList<Handle<Object> > properties(10, zone());
+ bool transitioning = true;
+ Handle<Object> prototype;
Handle<JSFunction> object_constructor(
isolate()->native_context()->object_function());
Handle<JSObject> json_object =
@@ -309,15 +313,33 @@ Handle<Object>
JsonParser<seq_ascii>::ParseJsonObject() {
if (key->AsArrayIndex(&index)) {
JSObject::SetOwnElement(json_object, index, value, kNonStrictMode);
} else if (key->Equals(isolate()->heap()->Proto_symbol())) {
- SetPrototype(json_object, value);
+ prototype = value;
} else {
- JSObject::SetLocalPropertyIgnoreAttributes(
- json_object, key, value, NONE);
+ if (transitioning) transitioning =
json_object->FieldTransitionTo(*key);
+ if (!transitioning) properties.Add(key, zone());
+ properties.Add(value, zone());
}
} while (MatchSkipWhiteSpace(','));
if (c0_ != '}') {
return ReportUnexpectedCharacter();
}
+ if (properties.length() > 0) {
+ int described_properties =
+ json_object->map()->NumberOfOwnDescriptors();
+ JSObject::InitializeProperties(json_object);
+ int i;
+ for (i = 0; i < described_properties; i++) {
+ Handle<Object> value = properties[i];
+ json_object->FastPropertyAtPut(i, *value);
+ }
+ for (int n = properties.length(); i < n; i += 2) {
+ Handle<String> key = Handle<String>::cast(properties[i]);
+ Handle<Object> value = properties[i+1];
+ JSObject::SetLocalPropertyIgnoreAttributes(
+ json_object, key, value, NONE);
+ }
+ }
+ if (!prototype.is_null()) SetPrototype(json_object, prototype);
}
AdvanceSkipWhitespace();
return json_object;
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
250e009b24f12c24582900bb2a4bcd89b77ab349..c5012becb9cca355972d9f67c690cb8b976ea5ef
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2953,6 +2953,42 @@ MaybeObject*
JSObject::SetPropertyForResult(LookupResult* result,
}


+void JSObject::InitializeProperties(Handle<JSObject> object) {
+ CALL_HEAP_FUNCTION_VOID(
+ object->GetIsolate(),
+ object->InitializeProperties());
+}
+
+
+MaybeObject* JSObject::InitializeProperties() {
+ Map* own_map = map();
+ int size =
+ own_map->NumberOfOwnDescriptors() -
+ own_map->inobject_properties() +
+ own_map->unused_property_fields();
+ if (properties()->length() == size) return this;
+ FixedArray* new_properties;
+ MaybeObject* maybe_properties = properties()->CopySize(size);
+ if (!maybe_properties->To(&new_properties)) return maybe_properties;
+ set_properties(new_properties);
+ return new_properties;
+}
+
+
+bool JSObject::FieldTransitionTo(String* key) {
+ Map* own_map = map();
+ if (!own_map->HasTransitionArray()) return false;
+ TransitionArray* transitions = own_map->transitions();
+ int transition = transitions->Search(key);
+ if (transition == TransitionArray::kNotFound) return false;
+ PropertyDetails target_details =
transitions->GetTargetDetails(transition);
+ if (target_details.type() != FIELD) return false;
+ if (target_details.attributes() != NONE) return false;
+ set_map(transitions->GetTarget(transition));
+ return true;
+}
+
+
// Set a real local property, even if it is READ_ONLY. If the property is
not
// present, add it with attributes NONE. This code is an exact clone of
// SetProperty, with the check for IsReadOnly and the check for a
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
edc85dee0e5c54803432c8e8696c89cc3b2da3f5..b05bd018316f84ea0edf05512dcd932c2bfdf4bf
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -1621,6 +1621,16 @@ class JSObject: public JSReceiver {
Handle<Object> value,
PropertyAttributes attributes);

+ // Try to follow an existing transition to a field with attributes NONE.
The
+ // return value indicates whether the transition was successful. This
function
+ // only updates the map. The property backing store has to be allocated
later.
+ bool FieldTransitionTo(String* key);
+
+ // Allocate the property backing store for an object that only contains
simple
+ // fields.
+ static void InitializeProperties(Handle<JSObject> object);
+ MUST_USE_RESULT MaybeObject* InitializeProperties();
+
// Can cause GC.
MUST_USE_RESULT MaybeObject* SetLocalPropertyIgnoreAttributes(
String* key,


da...@chromium.org

unread,
Oct 17, 2012, 9:54:23 AM10/17/12
to verw...@chromium.org, v8-...@googlegroups.com

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

https://chromiumcodereview.appspot.com/11184006/diff/2003/src/objects-inl.h#newcode1418
src/objects-inl.h:1418: int size =
As discussed, please convert this into something a little more readable.

https://chromiumcodereview.appspot.com/11184006/diff/2003/src/objects-inl.h#newcode1435
src/objects-inl.h:1435: bool JSObject::TryTransitionToField(String* key)
{
This should only be called by handlified code, it should be clear from
API.

https://chromiumcodereview.appspot.com/11184006/diff/2003/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/11184006/diff/2003/src/objects.h#newcode1626
src/objects.h:1626: inline bool TryTransitionToField(String* key);
must be static and use Handle<> just like other Handle APIs

https://chromiumcodereview.appspot.com/11184006/

verw...@chromium.org

unread,
Oct 17, 2012, 10:03:40 AM10/17/12
to da...@chromium.org, v8-...@googlegroups.com
Addressed comments.
On 2012/10/17 13:54:23, danno wrote:
> As discussed, please convert this into something a little more
readable.

Done.

https://chromiumcodereview.appspot.com/11184006/diff/2003/src/objects-inl.h#newcode1435
src/objects-inl.h:1435: bool JSObject::TryTransitionToField(String* key)
{
On 2012/10/17 13:54:23, danno wrote:
> This should only be called by handlified code, it should be clear from
API.

Done.
On 2012/10/17 13:54:23, danno wrote:
> must be static and use Handle<> just like other Handle APIs

Done.

https://chromiumcodereview.appspot.com/11184006/

da...@chromium.org

unread,
Oct 17, 2012, 10:07:49 AM10/17/12
to verw...@chromium.org, v8-...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages