[runtime] Don't track transitions for certainly detached maps

Previously such maps were marked as prototype, but that has bad
performance / memory characteristics if objects are used as
dictionaries.

Bug: b:148346655, v8:10339
Change-Id: I287c5664c8b7799a084669aaaffe3affcf73e95f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2179322Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67537}

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.GetBackPointer().IsUndefined(isolate_)) {
+            if (!maybe_feedback.IsDetached(isolate_)) {
               feedback = handle(maybe_feedback, isolate_);
               if (feedback->is_deprecated()) {
                 feedback = Map::Update(isolate_, feedback);

src/objects/map-inl.h

@@ -123,6 +123,12 @@ 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,
@@ -715,7 +721,10 @@ void Map::AppendDescriptor(Isolate* isolate, Descriptor* desc) {
 
 DEF_GETTER(Map, GetBackPointer, HeapObject) {
   Object object = constructor_or_backpointer(isolate);
-  if (object.IsMap(isolate)) {
+  // This is the equivalent of IsMap() but avoids reading the instance type so
+  // it can be used concurrently without acquire load.
+  if (object.IsHeapObject() && HeapObject::cast(object).map(isolate) ==
+                                   GetReadOnlyRoots(isolate).meta_map()) {
     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_EQ(result.NumberOfOwnDescriptors(),
+      DCHECK_LE(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 (is_prototype_map()) return Map();
+  if (IsDetached(isolate)) 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->is_prototype_map()) {
+  if (map->IsDetached(isolate)) {
     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 {
+  } else if (!parent->IsDetached(isolate)) {
     // |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->is_prototype_map()) {
-    DCHECK(child->is_prototype_map());
+  if (parent->IsDetached(isolate)) {
+    DCHECK(child->IsDetached(isolate));
     if (FLAG_trace_maps) {
       LOG(isolate, MapEvent("Transition", parent, child, "prototype", name));
     }
@@ -1755,7 +1755,9 @@ Handle<Map> Map::CopyReplaceDescriptors(
     result->set_may_have_interesting_symbols(true);
   }
 
-  if (!map->is_prototype_map()) {
+  if (map->is_prototype_map()) {
+    result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
+  } else {
     if (flag == INSERT_TRANSITION &&
         TransitionsAccessor(isolate, map).CanHaveMoreTransitions()) {
       result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
@@ -1766,19 +1768,11 @@ 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->is_prototype_map() ||
+      (map->IsDetached(isolate) ||
        !(flag == INSERT_TRANSITION &&
          TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) {
     LOG(isolate, MapEvent("ReplaceDescriptors", map, result, reason,
@@ -1960,7 +1954,7 @@ Handle<Map> Map::AsLanguageMode(Isolate* isolate, Handle<Map> initial_map,
 }
 
 Handle<Map> Map::CopyForElementsTransition(Isolate* isolate, Handle<Map> map) {
-  DCHECK(!map->is_prototype_map());
+  DCHECK(!map->IsDetached(isolate));
   Handle<Map> new_map = CopyDropDescriptors(isolate, map);
 
   if (map->owns_descriptors()) {
@@ -2161,7 +2155,7 @@ Handle<Map> Map::TransitionToDataProperty(Isolate* isolate, Handle<Map> map,
                                           StoreOrigin store_origin) {
   RuntimeCallTimerScope stats_scope(
       isolate,
-      map->is_prototype_map()
+      map->IsDetached(isolate)
           ? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty
           : RuntimeCallCounterId::kMap_TransitionToDataProperty);
 
@@ -2275,7 +2269,7 @@ Handle<Map> Map::TransitionToAccessorProperty(Isolate* isolate, Handle<Map> map,
                                               PropertyAttributes attributes) {
   RuntimeCallTimerScope stats_scope(
       isolate,
-      map->is_prototype_map()
+      map->IsDetached(isolate)
           ? RuntimeCallCounterId::kPrototypeMap_TransitionToAccessorProperty
           : RuntimeCallCounterId::kMap_TransitionToAccessorProperty);
 

src/objects/map.h

@@ -422,6 +422,11 @@ 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);