[v8-dev] Don't double-allocate mutable heap numbers in the json parser. This removes some stupidity when com… (issue 982793002 by [email protected])

Thu, 05 Mar 2015 06:35:51 -0800 

Reviewers: Yang,

Message:
PTAL

Description:
Don't double-allocate mutable heap numbers in the json parser. This removes some 
stupidity when committing state.

BUG=

Please review this at https://codereview.chromium.org/982793002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+33, -53 lines):
  M src/json-parser.h
  M src/objects.h
  M src/objects.cc
  M src/objects-inl.h


Index: src/json-parser.h
diff --git a/src/json-parser.h b/src/json-parser.h
index 4cf52574b54655a9000651db6391f38bd9cf9450..b9a98d4b604a176011416580ecb8e65cbd7e0434 100644 
--- a/src/json-parser.h
+++ b/src/json-parser.h
@@ -300,6 +300,7 @@ Handle<Object> JsonParser<seq_one_byte>::ParseJsonObject() { 
   Handle<JSObject> json_object =
       factory()->NewJSObject(object_constructor(), pretenure_);
   Handle<Map> map(json_object->map());
+  int descriptor = 0;
   ZoneList<Handle<Object> > properties(8, zone());
   DCHECK_EQ(c0_, '{');
@@ -382,18 +383,15 @@ Handle<Object> JsonParser<seq_one_byte>::ParseJsonObject() { 
         if (value.is_null()) return ReportUnexpectedCharacter();

         if (transitioning) {
-          int descriptor = map->NumberOfOwnDescriptors();
           PropertyDetails details =
               target->instance_descriptors()->GetDetails(descriptor);
Representation expected_representation = details.representation(); 

           if (value->FitsRepresentation(expected_representation)) {
-            if (expected_representation.IsDouble()) {
-              value = Object::NewStorageFor(isolate(), value,
-                                            expected_representation);
-            } else if (expected_representation.IsHeapObject() &&
-                       !target->instance_descriptors()->GetFieldType(
-                           descriptor)->NowContains(value)) {
+            if (expected_representation.IsHeapObject() &&
+                !target->instance_descriptors()
+                     ->GetFieldType(descriptor)
+                     ->NowContains(value)) {
               Handle<HeapType> value_type(value->OptimalType(
                       isolate(), expected_representation));
               Map::GeneralizeFieldType(target, descriptor,
@@ -403,6 +401,7 @@ Handle<Object> JsonParser<seq_one_byte>::ParseJsonObject() { 
                     descriptor)->NowContains(value));
             properties.Add(value, zone());
             map = target;
+            descriptor++;
             continue;
           } else {
             transitioning = false;
@@ -444,30 +443,11 @@ void JsonParser<seq_one_byte>::CommitStateToJsonObject( 
   DCHECK(!json_object->map()->is_dictionary_map());

   DisallowHeapAllocation no_gc;
-  Factory* factory = isolate()->factory();
-  // If the |json_object|'s map is exactly the same as |map| then the
-  // |properties| values correspond to the |map| and nothing more has to be
-  // done. But if the |json_object|'s map is different then we have to
-  // iterate descriptors to ensure that properties still correspond to the
-  // map.
-  bool slow_case = json_object->map() != *map;
-  DescriptorArray* descriptors = NULL;

   int length = properties->length();
-  if (slow_case) {
-    descriptors = json_object->map()->instance_descriptors();
-    DCHECK(json_object->map()->NumberOfOwnDescriptors() == length);
-  }
   for (int i = 0; i < length; i++) {
     Handle<Object> value = (*properties)[i];
-    if (slow_case && value->IsMutableHeapNumber() &&
-        !descriptors->GetDetails(i).representation().IsDouble()) {
- // Turn mutable heap numbers into immutable if the field representation 
-      // is not double.
-      HeapNumber::cast(*value)->set_map(*factory->heap_number_map());
-    }
-    FieldIndex index = FieldIndex::ForPropertyIndex(*map, i);
-    json_object->FastPropertyAtPut(index, *value);
+    json_object->WriteToField(i, *value);
   }
 }

Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index a5b2b352d79174ac87f08b4f6bc2d6e5f349f3a9..2ce845adfce09bb2c07b075c90d7d49f55aee80b 100644 
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -2139,6 +2139,31 @@ void JSObject::FastPropertyAtPut(FieldIndex index, Object* value) { 
 }


+void JSObject::WriteToField(int descriptor, Object* value) {
+  DisallowHeapAllocation no_gc;
+
+  DescriptorArray* desc = map()->instance_descriptors();
+  PropertyDetails details = desc->GetDetails(descriptor);
+
+  DCHECK(details.type() == DATA);
+
+  FieldIndex index = FieldIndex::ForDescriptor(map(), descriptor);
+  if (details.representation().IsDouble()) {
+    // Nothing more to be done.
+    if (value->IsUninitialized()) return;
+    if (IsUnboxedDoubleField(index)) {
+      RawFastDoublePropertyAtPut(index, value->Number());
+    } else {
+      HeapNumber* box = HeapNumber::cast(RawFastPropertyAt(index));
+      DCHECK(box->IsMutableHeapNumber());
+      box->set_value(value->Number());
+    }
+  } else {
+    RawFastPropertyAtPut(index, value);
+  }
+}
+
+
 int JSObject::GetInObjectPropertyOffset(int index) {
   return map()->GetInObjectPropertyOffset(index);
 }
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 717a16a9cbe280125557d42d2191e967d18d5c5d..ebe4a2414b3b8ac721e6ee7f76efa126743eca02 100644 
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4127,31 +4127,6 @@ bool JSObject::TryMigrateInstance(Handle<JSObject> object) { 
 }


-void JSObject::WriteToField(int descriptor, Object* value) {
-  DisallowHeapAllocation no_gc;
-
-  DescriptorArray* desc = map()->instance_descriptors();
-  PropertyDetails details = desc->GetDetails(descriptor);
-
-  DCHECK(details.type() == DATA);
-
-  FieldIndex index = FieldIndex::ForDescriptor(map(), descriptor);
-  if (details.representation().IsDouble()) {
-    // Nothing more to be done.
-    if (value->IsUninitialized()) return;
-    if (IsUnboxedDoubleField(index)) {
-      RawFastDoublePropertyAtPut(index, value->Number());
-    } else {
-      HeapNumber* box = HeapNumber::cast(RawFastPropertyAt(index));
-      DCHECK(box->IsMutableHeapNumber());
-      box->set_value(value->Number());
-    }
-  } else {
-    RawFastPropertyAtPut(index, value);
-  }
-}
-
-
 void JSObject::AddProperty(Handle<JSObject> object, Handle<Name> name,
                            Handle<Object> value,
                            PropertyAttributes attributes) {
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index 8ddcd8b04fc7a8d0cf4171f598cf2ba50ecfc65f..c3344ec98759a28b767e29f5ba440065f210fe21 100644 
--- a/src/objects.h
+++ b/src/objects.h
@@ -2080,7 +2080,7 @@ class JSObject: public JSReceiver {
   inline void FastPropertyAtPut(FieldIndex index, Object* value);
   inline void RawFastPropertyAtPut(FieldIndex index, Object* value);
   inline void RawFastDoublePropertyAtPut(FieldIndex index, double value);
-  void WriteToField(int descriptor, Object* value);
+  inline void WriteToField(int descriptor, Object* value);

   // Access to in object properties.
   inline int GetInObjectPropertyOffset(int index);


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to