Move property addition code from JSObject to Map

BUG=
R=ishell@chromium.org

Review URL: https://codereview.chromium.org/238543005

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20791 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/objects.cc b/src/objects.cc
index 9cca2cab2e90c544868b5ec89beadbabcf70371b..7094de13f1a350e1d1e6ee1c0164a902248baf52 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1823,48 +1823,98 @@ String* JSReceiver::constructor_name() {
 }
 
 
-void JSObject::AddFastProperty(Handle<JSObject> object,
-                               Handle<Name> name,
-                               Handle<Object> value,
-                               PropertyAttributes attributes,
-                               StoreFromKeyed store_mode,
-                               ValueType value_type,
-                               TransitionFlag flag) {
-  ASSERT(!object->IsJSGlobalProxy());
+MaybeHandle<Map> Map::CopyWithField(Handle<Map> map,
+                                    Handle<Name> name,
+                                    Handle<HeapType> type,
+                                    PropertyAttributes attributes,
+                                    Representation representation,
+                                    TransitionFlag flag) {
   ASSERT(DescriptorArray::kNotFound ==
-         object->map()->instance_descriptors()->Search(
-             *name, object->map()->NumberOfOwnDescriptors()));
+         map->instance_descriptors()->Search(
+             *name, map->NumberOfOwnDescriptors()));
+
+  // Ensure the descriptor array does not get too big.
+  if (map->NumberOfOwnDescriptors() >= kMaxNumberOfDescriptors) {
+    return MaybeHandle<Map>();
+  }
 
   // Normalize the object if the name is an actual name (not the
   // hidden strings) and is not a real identifier.
   // Normalize the object if it will have too many fast properties.
-  Isolate* isolate = object->GetIsolate();
-  if (!name->IsCacheable(isolate) ||
-      object->TooManyFastProperties(store_mode)) {
-    NormalizeProperties(object, CLEAR_INOBJECT_PROPERTIES, 0);
-    AddSlowProperty(object, name, value, attributes);
-    return;
-  }
-
-  // Allocate new instance descriptors with (name, index) added
-  if (object->IsJSContextExtensionObject()) value_type = FORCE_TAGGED;
-  Representation representation = value->OptimalRepresentation(value_type);
+  Isolate* isolate = map->GetIsolate();
+  if (!name->IsCacheable(isolate)) return MaybeHandle<Map>();
 
   // Compute the new index for new field.
-  int index = object->map()->NextFreePropertyIndex();
+  int index = map->NextFreePropertyIndex();
+
+  if (map->instance_type() == JS_CONTEXT_EXTENSION_OBJECT_TYPE) {
+    representation = Representation::Tagged();
+    type = HeapType::Any(isolate);
+  }
 
-  Handle<HeapType> type = value->OptimalType(isolate, representation);
   FieldDescriptor new_field_desc(name, index, type, attributes, representation);
-  Handle<Map> new_map = Map::CopyAddDescriptor(
-      handle(object->map()), &new_field_desc, flag);
+  Handle<Map> new_map = Map::CopyAddDescriptor(map, &new_field_desc, flag);
   int unused_property_fields = new_map->unused_property_fields() - 1;
   if (unused_property_fields < 0) {
     unused_property_fields += JSObject::kFieldsAdded;
   }
   new_map->set_unused_property_fields(unused_property_fields);
+  return new_map;
+}
+
+
+MaybeHandle<Map> Map::CopyWithConstant(Handle<Map> map,
+                                       Handle<Name> name,
+                                       Handle<Object> constant,
+                                       PropertyAttributes attributes,
+                                       TransitionFlag flag) {
+  // Ensure the descriptor array does not get too big.
+  if (map->NumberOfOwnDescriptors() >= kMaxNumberOfDescriptors) {
+    return MaybeHandle<Map>();
+  }
+
+  // Allocate new instance descriptors with (name, constant) added.
+  ConstantDescriptor new_constant_desc(name, constant, attributes);
+  return Map::CopyAddDescriptor(map, &new_constant_desc, flag);
+}
+
+
+void JSObject::AddFastProperty(Handle<JSObject> object,
+                               Handle<Name> name,
+                               Handle<Object> value,
+                               PropertyAttributes attributes,
+                               StoreFromKeyed store_mode,
+                               ValueType value_type,
+                               TransitionFlag flag) {
+  ASSERT(!object->IsJSGlobalProxy());
+
+  MaybeHandle<Map> maybe_map;
+  if (value->IsJSFunction()) {
+    maybe_map = Map::CopyWithConstant(
+        handle(object->map()), name, value, attributes, flag);
+  } else if (!object->TooManyFastProperties(store_mode)) {
+    Isolate* isolate = object->GetIsolate();
+    Representation representation = value->OptimalRepresentation(value_type);
+    maybe_map = Map::CopyWithField(
+        handle(object->map(), isolate), name,
+        value->OptimalType(isolate, representation),
+        attributes, representation, flag);
+  }
+
+  Handle<Map> new_map;
+  if (!maybe_map.ToHandle(&new_map)) {
+    NormalizeProperties(object, CLEAR_INOBJECT_PROPERTIES, 0);
+    return;
+  }
 
   JSObject::MigrateToMap(object, new_map);
 
+  PropertyDetails details = new_map->GetLastDescriptorDetails();
+  if (details.type() != FIELD) return;
+
+  Representation representation = details.representation();
+  int index = details.field_index();
+
   if (representation.IsDouble()) {
     // Nothing more to be done.
     if (value->IsUninitialized()) return;
@@ -1876,24 +1926,6 @@ void JSObject::AddFastProperty(Handle<JSObject> object,
 }
 
 
-void JSObject::AddConstantProperty(Handle<JSObject> object,
-                                   Handle<Name> name,
-                                   Handle<Object> constant,
-                                   PropertyAttributes attributes,
-                                   TransitionFlag initial_flag) {
-  ASSERT(!object->IsGlobalObject());
-  // Don't add transitions to special properties with non-trivial attributes.
-  TransitionFlag flag = attributes != NONE ? OMIT_TRANSITION : initial_flag;
-
-  // Allocate new instance descriptors with (name, constant) added.
-  ConstantDescriptor new_constant_desc(name, constant, attributes);
-  Handle<Map> new_map = Map::CopyAddDescriptor(
-      handle(object->map()), &new_constant_desc, flag);
-
-  JSObject::MigrateToMap(object, new_map);
-}
-
-
 void JSObject::AddSlowProperty(Handle<JSObject> object,
                                Handle<Name> name,
                                Handle<Object> value,
@@ -1958,25 +1990,11 @@ MaybeHandle<Object> JSObject::AddProperty(
   }
 
   if (object->HasFastProperties()) {
-    // Ensure the descriptor array does not get too big.
-    if (object->map()->NumberOfOwnDescriptors() <= kMaxNumberOfDescriptors) {
-      // TODO(verwaest): Support other constants.
-      // if (mode == ALLOW_AS_CONSTANT &&
-      //     !value->IsTheHole() &&
-      //     !value->IsConsString()) {
-      if (value->IsJSFunction()) {
-        AddConstantProperty(object, name, value, attributes, transition_flag);
-      } else {
-        AddFastProperty(object, name, value, attributes, store_mode,
-                        value_type, transition_flag);
-      }
-    } else {
-      // Normalize the object to prevent very large instance descriptors.
-      // This eliminates unwanted N^2 allocation and lookup behavior.
-      NormalizeProperties(object, CLEAR_INOBJECT_PROPERTIES, 0);
-      AddSlowProperty(object, name, value, attributes);
-    }
-  } else {
+    AddFastProperty(object, name, value, attributes, store_mode,
+                    value_type, transition_flag);
+  }
+
+  if (!object->HasFastProperties()) {
     AddSlowProperty(object, name, value, attributes);
   }
 
@@ -6616,15 +6634,6 @@ static bool TryAccessorTransition(Handle<JSObject> self,
 }
 
 
-static Handle<Map> CopyInsertDescriptor(Handle<Map> map,
-                                        Handle<Name> name,
-                                        Handle<AccessorPair> accessors,
-                                        PropertyAttributes attributes) {
-  CallbacksDescriptor new_accessors_desc(name, accessors, attributes);
-  return Map::CopyInsertDescriptor(map, &new_accessors_desc, INSERT_TRANSITION);
-}
-
-
 bool JSObject::DefineFastAccessor(Handle<JSObject> object,
                                   Handle<Name> name,
                                   AccessorComponent component,
@@ -6689,8 +6698,11 @@ bool JSObject::DefineFastAccessor(Handle<JSObject> object,
       ? AccessorPair::Copy(Handle<AccessorPair>(source_accessors))
       : isolate->factory()->NewAccessorPair();
   accessors->set(component, *accessor);
-  Handle<Map> new_map = CopyInsertDescriptor(Handle<Map>(object->map()),
-                                             name, accessors, attributes);
+
+  CallbacksDescriptor new_accessors_desc(name, accessors, attributes);
+  Handle<Map> new_map = Map::CopyInsertDescriptor(
+      handle(object->map()), &new_accessors_desc, INSERT_TRANSITION);
+
   JSObject::MigrateToMap(object, new_map);
   return true;
 }

diff --git a/src/objects.h b/src/objects.h
index 71bd97a625aa544cd341da5abd134e8c1cac048f..985472bc9cecc25328e0d99669a124d8bd77fbaa 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -2891,18 +2891,6 @@ class JSObject: public JSReceiver {
       StoreMode mode = ALLOW_AS_CONSTANT,
       TransitionFlag flag = INSERT_TRANSITION);
 
-  // Add a constant function property to a fast-case object.
-  // This leaves a CONSTANT_TRANSITION in the old map, and
-  // if it is called on a second object with this map, a
-  // normal property is added instead, with a map transition.
-  // This avoids the creation of many maps with the same constant
-  // function, all orphaned.
-  static void AddConstantProperty(Handle<JSObject> object,
-                                  Handle<Name> name,
-                                  Handle<Object> constant,
-                                  PropertyAttributes attributes,
-                                  TransitionFlag flag);
-
   // Add a property to a fast-case object.
   static void AddFastProperty(Handle<JSObject> object,
                               Handle<Name> name,
@@ -6416,9 +6404,6 @@ class Map: public HeapObject {
       Handle<DescriptorArray> descriptors,
       TransitionFlag flag,
       SimpleTransitionFlag simple_flag = FULL_TRANSITION);
-  static Handle<Map> CopyAddDescriptor(Handle<Map> map,
-                                       Descriptor* descriptor,
-                                       TransitionFlag flag);
   static Handle<Map> CopyInsertDescriptor(Handle<Map> map,
                                           Descriptor* descriptor,
                                           TransitionFlag flag);
@@ -6428,6 +6413,21 @@ class Map: public HeapObject {
                                            int index,
                                            TransitionFlag flag);
 
+  MUST_USE_RESULT static MaybeHandle<Map> CopyWithField(
+      Handle<Map> map,
+      Handle<Name> name,
+      Handle<HeapType> type,
+      PropertyAttributes attributes,
+      Representation representation,
+      TransitionFlag flag);
+
+  MUST_USE_RESULT static MaybeHandle<Map> CopyWithConstant(
+      Handle<Map> map,
+      Handle<Name> name,
+      Handle<Object> constant,
+      PropertyAttributes attributes,
+      TransitionFlag flag);
+
   static Handle<Map> AsElementsKind(Handle<Map> map, ElementsKind kind);
 
   static Handle<Map> CopyAsElementsKind(Handle<Map> map,
@@ -6678,6 +6678,9 @@ class Map: public HeapObject {
       Handle<Map> map,
       int new_descriptor,
       Handle<DescriptorArray> descriptors);
+  static Handle<Map> CopyAddDescriptor(Handle<Map> map,
+                                       Descriptor* descriptor,
+                                       TransitionFlag flag);
 
   // Zaps the contents of backing data structures. Note that the
   // heap verifier (i.e. VerifyMarkingVisitor) relies on zapping of objects

diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 2fe601e7840dfba9a38c4df79e1b36e46154a5bf..28ffa05e56a66d4075e648cede0dc4a4c844c942 100644 (file)
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -2633,14 +2633,15 @@ TEST(Regress1465) {
   v8::HandleScope scope(CcTest::isolate());
   static const int transitions_count = 256;
 
+  CompileRun("function F() {}");
   {
     AlwaysAllocateScope always_allocate(CcTest::i_isolate());
     for (int i = 0; i < transitions_count; i++) {
       EmbeddedVector<char, 64> buffer;
-      OS::SNPrintF(buffer, "var o = new Object; o.prop%d = %d;", i, i);
+      OS::SNPrintF(buffer, "var o = new F; o.prop%d = %d;", i, i);
       CompileRun(buffer.start());
     }
-    CompileRun("var root = new Object;");
+    CompileRun("var root = new F;");
   }
 
   Handle<JSObject> root =
@@ -2669,7 +2670,7 @@ static void AddTransitions(int transitions_count) {
   AlwaysAllocateScope always_allocate(CcTest::i_isolate());
   for (int i = 0; i < transitions_count; i++) {
     EmbeddedVector<char, 64> buffer;
-    OS::SNPrintF(buffer, "var o = new Object; o.prop%d = %d;", i, i);
+    OS::SNPrintF(buffer, "var o = new F; o.prop%d = %d;", i, i);
     CompileRun(buffer.start());
   }
 }
@@ -2702,8 +2703,9 @@ TEST(TransitionArrayShrinksDuringAllocToZero) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
   static const int transitions_count = 10;
+  CompileRun("function F() { }");
   AddTransitions(transitions_count);
-  CompileRun("var root = new Object;");
+  CompileRun("var root = new F;");
   Handle<JSObject> root = GetByName("root");
 
   // Count number of live transitions before marking.
@@ -2711,8 +2713,8 @@ TEST(TransitionArrayShrinksDuringAllocToZero) {
   CHECK_EQ(transitions_count, transitions_before);
 
   // Get rid of o
-  CompileRun("o = new Object;"
-             "root = new Object");
+  CompileRun("o = new F;"
+             "root = new F");
   root = GetByName("root");
   AddPropertyTo(2, root, "funny");
 
@@ -2730,8 +2732,9 @@ TEST(TransitionArrayShrinksDuringAllocToOne) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
   static const int transitions_count = 10;
+  CompileRun("function F() {}");
   AddTransitions(transitions_count);
-  CompileRun("var root = new Object;");
+  CompileRun("var root = new F;");
   Handle<JSObject> root = GetByName("root");
 
   // Count number of live transitions before marking.
@@ -2755,8 +2758,9 @@ TEST(TransitionArrayShrinksDuringAllocToOnePropertyFound) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
   static const int transitions_count = 10;
+  CompileRun("function F() {}");
   AddTransitions(transitions_count);
-  CompileRun("var root = new Object;");
+  CompileRun("var root = new F;");
   Handle<JSObject> root = GetByName("root");
 
   // Count number of live transitions before marking.
@@ -2780,16 +2784,17 @@ TEST(TransitionArraySimpleToFull) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
   static const int transitions_count = 1;
+  CompileRun("function F() {}");
   AddTransitions(transitions_count);
-  CompileRun("var root = new Object;");
+  CompileRun("var root = new F;");
   Handle<JSObject> root = GetByName("root");
 
   // Count number of live transitions before marking.
   int transitions_before = CountMapTransitions(root->map());
   CHECK_EQ(transitions_count, transitions_before);
 
-  CompileRun("o = new Object;"
-             "root = new Object");
+  CompileRun("o = new F;"
+             "root = new F");
   root = GetByName("root");
   ASSERT(root->map()->transitions()->IsSimpleTransition());
   AddPropertyTo(2, root, "happy");