Property reconfiguring implemented.

Previous approach for property reconfiguration was to create a free-floating map with generalized representations of all fields. This patch does it right.

When property is reconfigured either by changing its kind (kData <-> kAccessor) or its attributes it implies creation of a new branch in transition tree. If such a branch already existed before reconfiguration then it should be merged with the old (or source) branch of the transition tree. Merging procedure includes all the heavy machinery such as property location changes (kDescriptor -> kField), field representation/field type generalization, map deprecation, etc.

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

Cr-Commit-Position: refs/heads/master@{#26667}

src/lookup.cc

@@ -105,8 +105,10 @@ void LookupIterator::ReconfigureDataProperty(Handle<Object> value,
     PropertyDetails details(attributes, v8::internal::DATA, 0);
     JSObject::SetNormalizedProperty(holder, name(), value, details);
   } else {
-    holder_map_ = Map::ReconfigureDataProperty(holder_map_, descriptor_number(),
-                                               attributes);
+    holder_map_ = Map::ReconfigureExistingProperty(
+        holder_map_, descriptor_number(), i::kData, attributes);
+    holder_map_ =
+        Map::PrepareForDataProperty(holder_map_, descriptor_number(), value);
     JSObject::MigrateToMap(holder, holder_map_);
   }
 

src/objects-inl.h

@@ -58,7 +58,7 @@ PropertyDetails PropertyDetails::AsDeleted() const {
 
 
 int PropertyDetails::field_width_in_words() const {
-  DCHECK(type() == DATA);
+  DCHECK(location() == kField);
   if (!FLAG_unbox_double_fields) return 1;
   if (kDoubleSize == kPointerSize) return 1;
   return representation().IsDouble() ? kDoubleSize / kPointerSize : 1;
@@ -3148,13 +3148,13 @@ PropertyType DescriptorArray::GetType(int descriptor_number) {
 
 
 int DescriptorArray::GetFieldIndex(int descriptor_number) {
-  DCHECK(GetDetails(descriptor_number).type() == DATA);
+  DCHECK(GetDetails(descriptor_number).location() == kField);
   return GetDetails(descriptor_number).field_index();
 }
 
 
 HeapType* DescriptorArray::GetFieldType(int descriptor_number) {
-  DCHECK(GetDetails(descriptor_number).type() == DATA);
+  DCHECK(GetDetails(descriptor_number).location() == kField);
   return HeapType::cast(GetValue(descriptor_number));
 }
 

src/objects-printer.cc

@@ -424,6 +424,7 @@ void Map::MapPrint(std::ostream& os) {  // NOLINT
   if (has_instance_call_handler()) os << " - instance_call_handler\n";
   if (is_access_check_needed()) os << " - access_check_needed\n";
   if (!is_extensible()) os << " - non-extensible\n";
+  if (is_observed()) os << " - observed\n";
   os << " - back pointer: " << Brief(GetBackPointer());
   os << "\n - instance descriptors " << (owns_descriptors() ? "(own) " : "")
      << "#" << NumberOfOwnDescriptors() << ": "

src/objects.h

@@ -5889,23 +5889,15 @@ class Map: public HeapObject {
   static void GeneralizeFieldType(Handle<Map> map, int modify_index,
                                   Representation new_representation,
                                   Handle<HeapType> new_field_type);
-  static Handle<Map> GeneralizeRepresentation(
-      Handle<Map> map,
-      int modify_index,
-      Representation new_representation,
-      Handle<HeapType> new_field_type,
-      StoreMode store_mode);
+  static Handle<Map> ReconfigureProperty(Handle<Map> map, int modify_index,
+                                         PropertyKind new_kind,
+                                         PropertyAttributes new_attributes,
+                                         Representation new_representation,
+                                         Handle<HeapType> new_field_type,
+                                         StoreMode store_mode);
   static Handle<Map> CopyGeneralizeAllRepresentations(
-      Handle<Map> map,
-      int modify_index,
-      StoreMode store_mode,
-      PropertyAttributes attributes,
-      const char* reason);
-  static Handle<Map> CopyGeneralizeAllRepresentations(
-      Handle<Map> map,
-      int modify_index,
-      StoreMode store_mode,
-      const char* reason);
+      Handle<Map> map, int modify_index, StoreMode store_mode,
+      PropertyKind kind, PropertyAttributes attributes, const char* reason);
 
   static Handle<Map> PrepareForDataProperty(Handle<Map> old_map,
                                             int descriptor_number,
@@ -6128,8 +6120,10 @@ class Map: public HeapObject {
   static Handle<Map> TransitionToAccessorProperty(
       Handle<Map> map, Handle<Name> name, AccessorComponent component,
       Handle<Object> accessor, PropertyAttributes attributes);
-  static Handle<Map> ReconfigureDataProperty(Handle<Map> map, int descriptor,
-                                             PropertyAttributes attributes);
+  static Handle<Map> ReconfigureExistingProperty(Handle<Map> map,
+                                                 int descriptor,
+                                                 PropertyKind kind,
+                                                 PropertyAttributes attributes);
 
   inline void AppendDescriptor(Descriptor* desc);
 
@@ -6414,6 +6408,9 @@ class Map: public HeapObject {
                                            Descriptor* descriptor,
                                            int index,
                                            TransitionFlag flag);
+  static MUST_USE_RESULT MaybeHandle<Map> TryReconfigureExistingProperty(
+      Handle<Map> map, int descriptor, PropertyKind kind,
+      PropertyAttributes attributes, const char** reason);
 
   static Handle<Map> CopyNormalized(Handle<Map> map,
                                     PropertyNormalizationMode mode);
@@ -6447,6 +6444,8 @@ class Map: public HeapObject {
                        Representation new_representation,
                        Handle<HeapType> new_type);
 
+  void PrintReconfiguration(FILE* file, int modify_index, PropertyKind kind,
+                            PropertyAttributes attributes);
   void PrintGeneralization(FILE* file,
                            const char* reason,
                            int modify_index,
@@ -10506,6 +10505,14 @@ class AccessorPair: public Struct {
     if (!setter->IsNull()) set_setter(setter);
   }
 
+  bool Equals(AccessorPair* pair) {
+    return (this == pair) || pair->Equals(getter(), setter());
+  }
+
+  bool Equals(Object* getter_value, Object* setter_value) {
+    return (getter() == getter_value) && (setter() == setter_value);
+  }
+
   bool ContainsAccessor() {
     return IsJSAccessor(getter()) || IsJSAccessor(setter());
   }

src/property-details.h

@@ -212,6 +212,15 @@ class PropertyDetails BASE_EMBEDDED {
         | FieldIndexField::encode(field_index);
   }
 
+  PropertyDetails(PropertyAttributes attributes, PropertyKind kind,
+                  PropertyLocation location, Representation representation,
+                  int field_index = 0) {
+    value_ = KindField::encode(kind) | LocationField::encode(location) |
+             AttributesField::encode(attributes) |
+             RepresentationField::encode(EncodeRepresentation(representation)) |
+             FieldIndexField::encode(field_index);
+  }
+
   int pointer() const { return DescriptorPointer::decode(value_); }
 
   PropertyDetails set_pointer(int i) { return PropertyDetails(value_, i); }

src/property.cc

@@ -69,9 +69,9 @@ std::ostream& operator<<(std::ostream& os,
     os << "immutable ";
   }
   os << (details.kind() == kData ? "data" : "accessor");
+  os << ": " << details.representation().Mnemonic();
   if (details.location() == kField) {
-    os << ": " << details.representation().Mnemonic()
-       << ", field_index: " << details.field_index();
+    os << ", field_index: " << details.field_index();
   }
   return os << ", p: " << details.pointer()
             << ", attrs: " << details.attributes() << ")";

test/cctest/cctest.gyp

@@ -137,6 +137,7 @@
         'test-microtask-delivery.cc',
         'test-mark-compact.cc',
         'test-mementos.cc',
+        'test-migrations.cc',
         'test-object-observe.cc',
         'test-ordered-hash-table.cc',
         'test-parsing.cc',