Properly share descriptor arrays

... and remove too restrictive checks.

Bug: chromium:1025468, chromium:1027498
Change-Id: I1558d66ef88d1481530479969c0fb81fb6ff808c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1932373Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65153}

src/diagnostics/objects-debug.cc

@@ -442,18 +442,30 @@ void Map::MapVerify(Isolate* isolate) {
     CHECK(native_context().IsNativeContext());
   } else {
     if (GetBackPointer().IsUndefined(isolate)) {
-      // Root maps must keep the ownership and there must be no descriptors
-      // in the descriptors array that do not belong to the map.
-      CHECK(owns_descriptors() || is_prototype_map());
+      // Root maps must not have descriptors in the descriptor array that do not
+      // belong to the map.
       CHECK_EQ(NumberOfOwnDescriptors(),
                instance_descriptors().number_of_descriptors());
-      if (!is_prototype_map()) {
-        // There must be no slack in root maps' descriptors array.
-        CHECK_EQ(0, instance_descriptors().number_of_slack_descriptors());
-      }
     } else {
       // If there is a parent map it must be non-stable.
-      CHECK(!Map::cast(GetBackPointer()).is_stable());
+      Map parent = Map::cast(GetBackPointer());
+      CHECK(!parent.is_stable());
+      DescriptorArray descriptors = instance_descriptors();
+      if (descriptors == parent.instance_descriptors()) {
+        if (NumberOfOwnDescriptors() == parent.NumberOfOwnDescriptors() + 1) {
+          // Descriptors sharing through property transitions takes over
+          // ownership from the parent map.
+          CHECK(!parent.owns_descriptors());
+        } else {
+          CHECK_EQ(NumberOfOwnDescriptors(), parent.NumberOfOwnDescriptors());
+          // Descriptors sharing through special transitions takes over
+          // ownership from the parent map unless it uses the canonical empty
+          // descriptor array.
+          CHECK_IMPLIES(
+              descriptors != ReadOnlyRoots(isolate).empty_descriptor_array(),
+              !parent.owns_descriptors());
+        }
+      }
     }
   }
   SLOW_DCHECK(instance_descriptors().IsSortedNoDuplicates());

src/objects/map.cc

@@ -634,9 +634,8 @@ Map Map::FindRootMap(Isolate* isolate) const {
   while (true) {
     Object back = result.GetBackPointer(isolate);
     if (back.IsUndefined(isolate)) {
-      // Initial map always owns descriptors and doesn't have unused entries
-      // in the descriptor array.
-      DCHECK(result.owns_descriptors());
+      // Initial map must not contain descriptors in the descriptors array
+      // that do not belong to the map.
       DCHECK_EQ(result.NumberOfOwnDescriptors(),
                 result.instance_descriptors().number_of_descriptors());
       return result;
@@ -1572,9 +1571,8 @@ void EnsureInitialMap(Isolate* isolate, Handle<Map> map) {
          *map == *isolate->async_function_with_home_object_map() ||
          *map == *isolate->async_function_with_name_and_home_object_map());
 #endif
-  // Initial maps must always own their descriptors and it's descriptor array
-  // does not contain descriptors that do not belong to the map.
-  DCHECK(map->owns_descriptors());
+  // Initial maps must not contain descriptors in the descriptors array
+  // that do not belong to the map.
   DCHECK_EQ(map->NumberOfOwnDescriptors(),
             map->instance_descriptors().number_of_descriptors());
 }
@@ -1592,6 +1590,11 @@ Handle<Map> Map::CopyInitialMap(Isolate* isolate, Handle<Map> map,
                                 int instance_size, int inobject_properties,
                                 int unused_property_fields) {
   EnsureInitialMap(isolate, map);
+  // Initial map must not contain descriptors in the descriptors array
+  // that do not belong to the map.
+  DCHECK_EQ(map->NumberOfOwnDescriptors(),
+            map->instance_descriptors().number_of_descriptors());
+
   Handle<Map> result =
       RawCopy(isolate, map, instance_size, inobject_properties);
 
@@ -1600,9 +1603,10 @@ Handle<Map> Map::CopyInitialMap(Isolate* isolate, Handle<Map> map,
 
   int number_of_own_descriptors = map->NumberOfOwnDescriptors();
   if (number_of_own_descriptors > 0) {
-    // The copy will use the same descriptors array.
-    result->UpdateDescriptors(isolate, map->instance_descriptors(),
-                              map->GetLayoutDescriptor(),
+    // The copy will use the same descriptors array without ownership.
+    DescriptorArray descriptors = map->instance_descriptors();
+    result->set_owns_descriptors(false);
+    result->UpdateDescriptors(isolate, descriptors, map->GetLayoutDescriptor(),
                               number_of_own_descriptors);
 
     DCHECK_EQ(result->NumberOfFields(),
@@ -1682,9 +1686,8 @@ void Map::ConnectTransition(Isolate* isolate, Handle<Map> parent,
   if (!parent->GetBackPointer().IsUndefined(isolate)) {
     parent->set_owns_descriptors(false);
   } else {
-    // |parent| is initial map and it must keep the ownership, there must be no
-    // descriptors in the descriptors array that do not belong to the map.
-    DCHECK(parent->owns_descriptors());
+    // |parent| is initial map and it must not contain descriptors in the
+    // descriptors array that do not belong to the map.
     DCHECK_EQ(parent->NumberOfOwnDescriptors(),
               parent->instance_descriptors().number_of_descriptors());
   }
@@ -1928,6 +1931,7 @@ Handle<Map> Map::CopyForElementsTransition(Isolate* isolate, Handle<Map> map) {
     // In case the map owned its own descriptors, share the descriptors and
     // transfer ownership to the new map.
     // The properties did not change, so reuse descriptors.
+    map->set_owns_descriptors(false);
     new_map->InitializeDescriptors(isolate, map->instance_descriptors(),
                                    map->GetLayoutDescriptor());
   } else {