Revert "Reland^3 "[runtime] Amortize descriptor array growing for fast-mode prototypes""

This reverts commit 65630850.

Reason for revert: due to failures

Original change's description:
> Reland^3 "[runtime] Amortize descriptor array growing for fast-mode prototypes"
> 
> This CL:
>  - stops tracking transitions for fast maps that are known to be detached
>  - reuses descriptor arrays when transitioning detached maps to avoid O(n^2) performance and garbage creation
> 
> Fix in reland: check whether the map of the back pointer is the metamap rather than reading the map of the constructor-or-backpointer slot. If the slot contains a constructor, it's possible that the object transitions while the concurrent marker is reading the map (from which it's reading the instance type); and it's possible that the transitioned map isn't set up yet fully when we read the instance type. An acquire load for the constructor-or-backpointer map would also fix it by serializing stores, but is more expensive. Checking the metamap is faster.
> 
> Original commit message:
> > This avoids an O(n^2) algorithm that creates an equal amount of garbage.
> > Even though the actual final descriptor array might be a little bigger,
> > it reduces peak memory usage by allocating less.
> 
> TBR=ulan@chromium.org,ishell@chromium.org
> 
> Change-Id: I57000949debdee2b69dd41e0c5975b3e8a34c6f4
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: b:148346655, v8:10339
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2173363
> Commit-Queue: Toon Verwaest <verwaest@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#67499}

TBR=ulan@chromium.org,clemensb@chromium.org,ishell@chromium.org,verwaest@chromium.org

Change-Id: Ie7018912f591d397c8acede9b31fbf269d225fe4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:148346655, v8:10339
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2174299Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67500}

src/diagnostics/objects-debug.cc

@@ -439,12 +439,8 @@ void Map::MapVerify(Isolate* isolate) {
     if (GetBackPointer().IsUndefined(isolate)) {
       // Root maps must not have descriptors in the descriptor array that do not
       // belong to the map.
-      int nof = instance_descriptors().number_of_descriptors();
-      if (IsDetached(isolate)) {
-        CHECK(base::IsInRange(NumberOfOwnDescriptors(), 0, nof));
-      } else {
-        CHECK_EQ(NumberOfOwnDescriptors(), nof);
-      }
+      CHECK_EQ(NumberOfOwnDescriptors(),
+               instance_descriptors().number_of_descriptors());
     } else {
       // If there is a parent map it must be non-stable.
       Map parent = Map::cast(GetBackPointer());

src/heap/marking-visitor-inl.h

@@ -407,14 +407,7 @@ int MarkingVisitorBase<ConcreteVisitor, MarkingState>::VisitMap(Map meta_map,
     DescriptorArray descriptors = map.synchronized_instance_descriptors();
     size += MarkDescriptorArrayBlack(map, descriptors);
     int number_of_own_descriptors = map.NumberOfOwnDescriptors();
-    if (map.IsDetached(heap_->isolate())) {
-      // Mark all descriptors in detached maps. Descriptor arrays are reused as
-      // objects transition between maps, and its possible that a newer map dies
-      // before an older map. Marking all descriptors through each map
-      // guarantees that we don't end up with dangling references that we don't
-      // clear since there are no explicit transitions.
-      VisitDescriptors(descriptors, descriptors.number_of_descriptors());
-    } else if (number_of_own_descriptors) {
+    if (number_of_own_descriptors) {
       // It is possible that the concurrent marker observes the
       // number_of_own_descriptors out of sync with the descriptors. In that
       // case the marking write barrier for the descriptor array will ensure

src/json/json-parser.cc

@@ -838,7 +838,7 @@ MaybeHandle<Object> JsonParser<Char>::ParseJsonValue() {
             Map maybe_feedback = JSObject::cast(*element_stack.back()).map();
             // Don't consume feedback from objects with a map that's detached
             // from the transition tree.
-            if (!maybe_feedback.IsDetached(isolate_)) {
+            if (!maybe_feedback.GetBackPointer().IsUndefined(isolate_)) {
               feedback = handle(maybe_feedback, isolate_);
               if (feedback->is_deprecated()) {
                 feedback = Map::Update(isolate_, feedback);

src/objects/map-inl.h

@@ -123,12 +123,6 @@ bool Map::CanHaveFastTransitionableElementsKind() const {
   return CanHaveFastTransitionableElementsKind(instance_type());
 }
 
-bool Map::IsDetached(Isolate* isolate) const {
-  if (is_prototype_map()) return true;
-  return instance_type() == JS_OBJECT_TYPE && NumberOfOwnDescriptors() > 0 &&
-         GetBackPointer().IsUndefined(isolate);
-}
-
 // static
 void Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
     Isolate* isolate, InstanceType instance_type,
@@ -721,10 +715,7 @@ void Map::AppendDescriptor(Isolate* isolate, Descriptor* desc) {
 
 DEF_GETTER(Map, GetBackPointer, HeapObject) {
   Object object = constructor_or_backpointer(isolate);
-  // This is the equivalent of IsMap() but avoids reading the instance type so
-  // it can be used concurrently without acquire load.
-  if (HeapObject::cast(object).map(isolate) ==
-      GetReadOnlyRoots(isolate).meta_map()) {
+  if (object.IsMap(isolate)) {
     return Map::cast(object);
   }
   // Can't use ReadOnlyRoots(isolate) as this isolate could be produced by

src/objects/map.cc

@@ -658,7 +658,7 @@ Map Map::FindRootMap(Isolate* isolate) const {
     if (back.IsUndefined(isolate)) {
       // Initial map must not contain descriptors in the descriptors array
       // that do not belong to the map.
-      DCHECK_LE(result.NumberOfOwnDescriptors(),
+      DCHECK_EQ(result.NumberOfOwnDescriptors(),
                 result.instance_descriptors().number_of_descriptors());
       return result;
     }
@@ -1221,7 +1221,7 @@ Map Map::FindElementsKindTransitionedMap(Isolate* isolate,
   DisallowHeapAllocation no_allocation;
   DisallowDeoptimization no_deoptimization(isolate);
 
-  if (IsDetached(isolate)) return Map();
+  if (is_prototype_map()) return Map();
 
   ElementsKind kind = elements_kind();
   bool packed = IsFastPackedElementsKind(kind);
@@ -1354,7 +1354,7 @@ static Handle<Map> AddMissingElementsTransitions(Isolate* isolate,
 
   ElementsKind kind = map->elements_kind();
   TransitionFlag flag;
-  if (map->IsDetached(isolate)) {
+  if (map->is_prototype_map()) {
     flag = OMIT_TRANSITION;
   } else {
     flag = INSERT_TRANSITION;
@@ -1721,14 +1721,14 @@ void Map::ConnectTransition(Isolate* isolate, Handle<Map> parent,
                  child->may_have_interesting_symbols());
   if (!parent->GetBackPointer().IsUndefined(isolate)) {
     parent->set_owns_descriptors(false);
-  } else if (!parent->IsDetached(isolate)) {
+  } else {
     // |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());
   }
-  if (parent->IsDetached(isolate)) {
-    DCHECK(child->IsDetached(isolate));
+  if (parent->is_prototype_map()) {
+    DCHECK(child->is_prototype_map());
     if (FLAG_trace_maps) {
       LOG(isolate, MapEvent("Transition", parent, child, "prototype", name));
     }
@@ -1755,9 +1755,7 @@ Handle<Map> Map::CopyReplaceDescriptors(
     result->set_may_have_interesting_symbols(true);
   }
 
-  if (map->is_prototype_map()) {
-    result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
-  } else {
+  if (!map->is_prototype_map()) {
     if (flag == INSERT_TRANSITION &&
         TransitionsAccessor(isolate, map).CanHaveMoreTransitions()) {
       result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
@@ -1768,11 +1766,19 @@ Handle<Map> Map::CopyReplaceDescriptors(
       descriptors->GeneralizeAllFields();
       result->InitializeDescriptors(isolate, *descriptors,
                                     LayoutDescriptor::FastPointerLayout());
+      // If we were trying to insert a transition but failed because there are
+      // too many transitions already, mark the object as a prototype to avoid
+      // tracking transitions from the detached map.
+      if (flag == INSERT_TRANSITION) {
+        result->set_is_prototype_map(true);
+      }
     }
+  } else {
+    result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
   }
   if (FLAG_trace_maps &&
       // Mirror conditions above that did not call ConnectTransition().
-      (map->IsDetached(isolate) ||
+      (map->is_prototype_map() ||
        !(flag == INSERT_TRANSITION &&
          TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) {
     LOG(isolate, MapEvent("ReplaceDescriptors", map, result, reason,
@@ -1954,7 +1960,7 @@ Handle<Map> Map::AsLanguageMode(Isolate* isolate, Handle<Map> initial_map,
 }
 
 Handle<Map> Map::CopyForElementsTransition(Isolate* isolate, Handle<Map> map) {
-  DCHECK(!map->IsDetached(isolate));
+  DCHECK(!map->is_prototype_map());
   Handle<Map> new_map = CopyDropDescriptors(isolate, map);
 
   if (map->owns_descriptors()) {
@@ -2155,7 +2161,7 @@ Handle<Map> Map::TransitionToDataProperty(Isolate* isolate, Handle<Map> map,
                                           StoreOrigin store_origin) {
   RuntimeCallTimerScope stats_scope(
       isolate,
-      map->IsDetached(isolate)
+      map->is_prototype_map()
           ? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty
           : RuntimeCallCounterId::kMap_TransitionToDataProperty);
 
@@ -2269,7 +2275,7 @@ Handle<Map> Map::TransitionToAccessorProperty(Isolate* isolate, Handle<Map> map,
                                               PropertyAttributes attributes) {
   RuntimeCallTimerScope stats_scope(
       isolate,
-      map->IsDetached(isolate)
+      map->is_prototype_map()
           ? RuntimeCallCounterId::kPrototypeMap_TransitionToAccessorProperty
           : RuntimeCallCounterId::kMap_TransitionToAccessorProperty);
 
@@ -2384,64 +2390,19 @@ Handle<Map> Map::CopyAddDescriptor(Isolate* isolate, Handle<Map> map,
     return ShareDescriptor(isolate, map, descriptors, descriptor);
   }
 
-  Handle<DescriptorArray> new_descriptors;
-  Handle<LayoutDescriptor> new_layout_descriptor;
-  if (map->IsDetached(isolate)) {
-    if (descriptors->number_of_slack_descriptors() == 0) {
-      int old_size = descriptors->number_of_descriptors();
-      if (old_size == 0) {
-        new_descriptors = DescriptorArray::Allocate(isolate, 0, 1);
-      } else {
-        int slack = SlackForArraySize(old_size, kMaxNumberOfDescriptors);
-        EnsureDescriptorSlack(isolate, map, slack);
-        new_descriptors = handle(map->instance_descriptors(), isolate);
-      }
-    } else {
-      new_descriptors = descriptors;
-    }
-    new_layout_descriptor =
-        FLAG_unbox_double_fields
-            ? LayoutDescriptor::ShareAppend(isolate, map,
-                                            descriptor->GetDetails())
-            : handle(LayoutDescriptor::FastPointerLayout(), isolate);
-  } else {
-    int nof = map->NumberOfOwnDescriptors();
-    new_descriptors = DescriptorArray::CopyUpTo(isolate, descriptors, nof, 1);
-    new_layout_descriptor =
-        FLAG_unbox_double_fields
-            ? LayoutDescriptor::New(isolate, map, new_descriptors, nof + 1)
-            : handle(LayoutDescriptor::FastPointerLayout(), isolate);
-  }
-
-  Handle<Map> result = CopyDropDescriptors(isolate, map);
-
-  if (descriptor->GetKey()->IsInterestingSymbol()) {
-    result->set_may_have_interesting_symbols(true);
-  }
-
-  {
-    DisallowHeapAllocation no_gc;
-    new_descriptors->Append(descriptor);
-    result->InitializeDescriptors(isolate, *new_descriptors,
-                                  *new_layout_descriptor);
-  }
-
-  if (flag == INSERT_TRANSITION &&
-      TransitionsAccessor(isolate, map).CanHaveMoreTransitions()) {
-    ConnectTransition(isolate, map, result, descriptor->GetKey(),
-                      SIMPLE_PROPERTY_TRANSITION);
-  }
+  int nof = map->NumberOfOwnDescriptors();
+  Handle<DescriptorArray> new_descriptors =
+      DescriptorArray::CopyUpTo(isolate, descriptors, nof, 1);
+  new_descriptors->Append(descriptor);
 
-  if (FLAG_trace_maps &&
-      // Mirror conditions above that did not call ConnectTransition().
-      (map->IsDetached(isolate) ||
-       !(flag == INSERT_TRANSITION &&
-         TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) {
-    LOG(isolate, MapEvent("ReplaceDescriptors", map, result,
-                          "CopyAddDescriptor", descriptor->GetKey()));
-  }
+  Handle<LayoutDescriptor> new_layout_descriptor =
+      FLAG_unbox_double_fields
+          ? LayoutDescriptor::New(isolate, map, new_descriptors, nof + 1)
+          : handle(LayoutDescriptor::FastPointerLayout(), isolate);
 
-  return result;
+  return CopyReplaceDescriptors(
+      isolate, map, new_descriptors, new_layout_descriptor, flag,
+      descriptor->GetKey(), "CopyAddDescriptor", SIMPLE_PROPERTY_TRANSITION);
 }
 
 Handle<Map> Map::CopyInsertDescriptor(Isolate* isolate, Handle<Map> map,

src/objects/map.h

@@ -422,11 +422,6 @@ class Map : public HeapObject {
   inline bool has_sealed_elements() const;
   inline bool has_frozen_elements() const;
 
-  // Weakly checks whether a map is detached from all transition trees. If this
-  // returns true, the map is guaranteed to be detached. If it returns false,
-  // there is no guarantee it is attached.
-  inline bool IsDetached(Isolate* isolate) const;
-
   // Returns true if the current map doesn't have DICTIONARY_ELEMENTS but if a
   // map with DICTIONARY_ELEMENTS was found in the prototype chain.
   bool DictionaryElementsInPrototypeChainOnly(Isolate* isolate);