Revert "Reland "Create a fast path to get migration target when updating map""

This reverts commit 6ec90ece.

Reason for revert: causes a lot of Canary crashes (chromium:895208).
GC relies on an the fact that the transition array stays alive while it's owner map
is alive (this is needed in order to properly transfer descriptor array ownership
to the parent map when the map owning a shared descriptor array dies). We need to
rethink a way of caching the migration target shortcut.

Original change's description:
> Reland "Create a fast path to get migration target when updating map"
>
> This is a reland of c285380c
>
> Original change's description:
> > Create a fast path to get migration target when updating map
> >
> > During map updating, store the pointer to new map in the
> > raw_transitions slot of the old map that is deprecated from map
> > transition tree. Thus, we can get the migration target directly
> > instead of TryReplayPropertyTransitions when updating map.
> >
> > This can improve Speedometer2.0 Elm-TodoMVC case by ~5% on ATOM
> > Chromebook and ~9% on big-core Ubuntu.
> >
> > Change-Id: I56f9ce5183bbdd567b964890f623ef0ceed9b7db
> > Reviewed-on: https://chromium-review.googlesource.com/1233433
> > Commit-Queue: Shiyu Zhang <shiyu.zhang@intel.com>
> > Reviewed-by: Igor Sheludko <ishell@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#56303}
>
> Change-Id: Idf0b7716b92a6a15bfe58721c2c34dbd02b31137
> Reviewed-on: https://chromium-review.googlesource.com/c/1270261
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Commit-Queue: Shiyu Zhang <shiyu.zhang@intel.com>
> Cr-Commit-Position: refs/heads/master@{#56588}

TBR=ishell@chromium.org,shiyu.zhang@intel.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Ie7e9b98395b041a1095da549d1cd71d7180a4888
Bug: chromium:895208
Reviewed-on: https://chromium-review.googlesource.com/c/1280223
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56641}

src/map-updater.cc

@@ -170,7 +170,6 @@ Handle<Map> MapUpdater::Update() {
   if (FindTargetMap() == kEnd) return result_map_;
   ConstructNewMap();
   DCHECK_EQ(kEnd, state_);
-  TransitionsAccessor(isolate_, old_map_).SetMigrationTarget(*result_map_);
   return result_map_;
 }
 

src/objects-printer.cc

@@ -2434,7 +2434,6 @@ void TransitionsAccessor::PrintTransitions(std::ostream& os) {  // NOLINT
   switch (encoding()) {
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
       return;
     case kWeakRef: {
       Map* target = Map::cast(raw_transitions_->GetHeapObjectAssumeWeak());

src/objects.cc

@@ -4904,45 +4904,6 @@ Handle<Map> Map::ReconfigureElementsKind(Isolate* isolate, Handle<Map> map,
   return mu.ReconfigureElementsKind(new_elements_kind);
 }
 
-namespace {
-
-Map* SearchMigrationTarget(Isolate* isolate, Map* old_map) {
-  DisallowHeapAllocation no_allocation;
-  DisallowDeoptimization no_deoptimization(isolate);
-
-  Map* target = old_map;
-  do {
-    target = TransitionsAccessor(isolate, target, &no_allocation)
-                 .GetMigrationTarget();
-  } while (target != nullptr && target->is_deprecated());
-  if (target == nullptr) return nullptr;
-
-  // TODO(ishell): if this validation ever become a bottleneck consider adding a
-  // bit to the Map telling whether it contains fields whose field types may be
-  // cleared.
-  // TODO(ishell): revisit handling of cleared field types in
-  // TryReplayPropertyTransitions() and consider checking the target map's field
-  // types instead of old_map's types.
-  // Go to slow map updating if the old_map has fast properties with cleared
-  // field types.
-  int old_nof = old_map->NumberOfOwnDescriptors();
-  DescriptorArray* old_descriptors = old_map->instance_descriptors();
-  for (int i = 0; i < old_nof; i++) {
-    PropertyDetails old_details = old_descriptors->GetDetails(i);
-    if (old_details.location() == kField && old_details.kind() == kData) {
-      FieldType* old_type = old_descriptors->GetFieldType(i);
-      if (FieldTypeIsCleared(old_details.representation(), old_type)) {
-        return nullptr;
-      }
-    }
-  }
-
-  SLOW_DCHECK(Map::TryUpdateSlow(isolate, old_map) == target);
-  return target;
-}
-}  // namespace
-
-// TODO(ishell): Move TryUpdate() and friends to MapUpdater
 // static
 MaybeHandle<Map> Map::TryUpdate(Isolate* isolate, Handle<Map> old_map) {
   DisallowHeapAllocation no_allocation;
@@ -4950,22 +4911,6 @@ MaybeHandle<Map> Map::TryUpdate(Isolate* isolate, Handle<Map> old_map) {
 
   if (!old_map->is_deprecated()) return old_map;
 
-  Map* target_map = SearchMigrationTarget(isolate, *old_map);
-  if (target_map != nullptr) {
-    return handle(target_map, isolate);
-  }
-
-  Map* new_map = TryUpdateSlow(isolate, *old_map);
-  if (new_map == nullptr) return MaybeHandle<Map>();
-  TransitionsAccessor(isolate, *old_map, &no_allocation)
-      .SetMigrationTarget(new_map);
-  return handle(new_map, isolate);
-}
-
-Map* Map::TryUpdateSlow(Isolate* isolate, Map* old_map) {
-  DisallowHeapAllocation no_allocation;
-  DisallowDeoptimization no_deoptimization(isolate);
-
   // Check the state of the root map.
   Map* root_map = old_map->FindRootMap(isolate);
   if (root_map->is_deprecated()) {
@@ -4974,21 +4919,23 @@ Map* Map::TryUpdateSlow(Isolate* isolate, Map* old_map) {
     DCHECK(constructor->initial_map()->is_dictionary_map());
     if (constructor->initial_map()->elements_kind() !=
         old_map->elements_kind()) {
-      return nullptr;
+      return MaybeHandle<Map>();
     }
-    return constructor->initial_map();
+    return handle(constructor->initial_map(), constructor->GetIsolate());
   }
-  if (!old_map->EquivalentToForTransition(root_map)) return nullptr;
+  if (!old_map->EquivalentToForTransition(root_map)) return MaybeHandle<Map>();
 
   ElementsKind from_kind = root_map->elements_kind();
   ElementsKind to_kind = old_map->elements_kind();
   if (from_kind != to_kind) {
     // Try to follow existing elements kind transitions.
     root_map = root_map->LookupElementsTransitionMap(isolate, to_kind);
-    if (root_map == nullptr) return nullptr;
+    if (root_map == nullptr) return MaybeHandle<Map>();
     // From here on, use the map with correct elements kind as root map.
   }
-  return root_map->TryReplayPropertyTransitions(isolate, old_map);
+  Map* new_map = root_map->TryReplayPropertyTransitions(isolate, *old_map);
+  if (new_map == nullptr) return MaybeHandle<Map>();
+  return handle(new_map, isolate);
 }
 
 Map* Map::TryReplayPropertyTransitions(Isolate* isolate, Map* old_map) {
@@ -5070,10 +5017,6 @@ Map* Map::TryReplayPropertyTransitions(Isolate* isolate, Map* old_map) {
 // static
 Handle<Map> Map::Update(Isolate* isolate, Handle<Map> map) {
   if (!map->is_deprecated()) return map;
-  Map* target_map = SearchMigrationTarget(isolate, *map);
-  if (target_map != nullptr) {
-    return handle(target_map, isolate);
-  }
   MapUpdater mu(isolate, map);
   return mu.Update();
 }

src/objects/map.h

@@ -633,7 +633,6 @@ class Map : public HeapObject {
   // is found.
   static MaybeHandle<Map> TryUpdate(Isolate* isolate,
                                     Handle<Map> map) V8_WARN_UNUSED_RESULT;
-  static Map* TryUpdateSlow(Isolate* isolate, Map* map) V8_WARN_UNUSED_RESULT;
 
   // Returns a non-deprecated version of the input. This method may deprecate
   // existing maps along the way if encodings conflict. Not for use while

src/transitions-inl.h

@@ -65,7 +65,6 @@ Name* TransitionsAccessor::GetKey(int transition_number) {
   switch (encoding()) {
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
       UNREACHABLE();
       return nullptr;
     case kWeakRef: {
@@ -119,7 +118,6 @@ Map* TransitionsAccessor::GetTarget(int transition_number) {
   switch (encoding()) {
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
       UNREACHABLE();
       return nullptr;
     case kWeakRef:

src/transitions.cc

@@ -21,12 +21,9 @@ void TransitionsAccessor::Initialize() {
   } else if (raw_transitions_->GetHeapObjectIfStrong(&heap_object)) {
     if (heap_object->IsTransitionArray()) {
       encoding_ = kFullTransitionArray;
-    } else if (heap_object->IsPrototypeInfo()) {
-      encoding_ = kPrototypeInfo;
     } else {
-      DCHECK(map_->is_deprecated());
-      DCHECK(heap_object->IsMap());
-      encoding_ = kMigrationTarget;
+      DCHECK(heap_object->IsPrototypeInfo());
+      encoding_ = kPrototypeInfo;
     }
   } else {
     UNREACHABLE();
@@ -51,7 +48,6 @@ bool TransitionsAccessor::HasSimpleTransitionTo(Map* map) {
       return raw_transitions_->GetHeapObjectAssumeWeak() == map;
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
     case kFullTransitionArray:
       return false;
   }
@@ -216,7 +212,6 @@ Map* TransitionsAccessor::SearchTransition(Name* name, PropertyKind kind,
   switch (encoding()) {
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
       return nullptr;
     case kWeakRef: {
       Map* map = Map::cast(raw_transitions_->GetHeapObjectAssumeWeak());
@@ -269,7 +264,6 @@ Handle<String> TransitionsAccessor::ExpectedTransitionKey() {
   switch (encoding()) {
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
     case kFullTransitionArray:
       return Handle<String>::null();
     case kWeakRef: {
@@ -436,7 +430,6 @@ int TransitionsAccessor::NumberOfTransitions() {
   switch (encoding()) {
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
       return 0;
     case kWeakRef:
       return 1;
@@ -447,26 +440,6 @@ int TransitionsAccessor::NumberOfTransitions() {
   return 0;  // Make GCC happy.
 }
 
-void TransitionsAccessor::SetMigrationTarget(Map* migration_target) {
-  DCHECK(map_->is_deprecated());
-  if (encoding() == kFullTransitionArray) {
-    // Transition arrays are not shared. When one is replaced, it should not
-    // keep referenced objects alive, so we zap it.
-    // When there is another reference to the array somewhere (e.g. a handle),
-    // not zapping turns from a waste of memory into a source of crashes.
-    transitions()->Zap(isolate_);
-  }
-  map_->set_raw_transitions(MaybeObject::FromObject(migration_target));
-  MarkNeedsReload();
-}
-
-Map* TransitionsAccessor::GetMigrationTarget() {
-  if (encoding() == kMigrationTarget) {
-    return map_->raw_transitions()->cast<Map>();
-  }
-  return nullptr;
-}
-
 void TransitionArray::Zap(Isolate* isolate) {
   MemsetPointer(
       data_start() + kPrototypeTransitionsIndex,
@@ -501,8 +474,7 @@ void TransitionsAccessor::SetPrototypeTransitions(
 
 void TransitionsAccessor::EnsureHasFullTransitionArray() {
   if (encoding() == kFullTransitionArray) return;
-  int nof =
-      (encoding() == kUninitialized || encoding() == kMigrationTarget) ? 0 : 1;
+  int nof = encoding() == kUninitialized ? 0 : 1;
   Handle<TransitionArray> result = isolate_->factory()->NewTransitionArray(nof);
   Reload();  // Reload after possible GC.
   if (nof == 1) {
@@ -525,7 +497,6 @@ void TransitionsAccessor::TraverseTransitionTreeInternal(
   switch (encoding()) {
     case kPrototypeInfo:
     case kUninitialized:
-    case kMigrationTarget:
       break;
     case kWeakRef: {
       Map* simple_target =

src/transitions.h

@@ -106,14 +106,6 @@ class TransitionsAccessor {
   void PutPrototypeTransition(Handle<Object> prototype, Handle<Map> target_map);
   Handle<Map> GetPrototypeTransition(Handle<Object> prototype);
 
-  // During the first-time Map::Update and Map::TryUpdate, the migration target
-  // map could be cached in the raw_transitions slot of the old map that is
-  // deprecated from the map transition tree. The next time old map is updated,
-  // we will check this cache slot as a shortcut to get the migration target
-  // map.
-  void SetMigrationTarget(Map* migration_target);
-  Map* GetMigrationTarget();
-
 #if DEBUG || OBJECT_PRINT
   void PrintTransitions(std::ostream& os);
   static void PrintOneTransition(std::ostream& os, Name* key, Map* target);
@@ -133,7 +125,6 @@ class TransitionsAccessor {
   enum Encoding {
     kPrototypeInfo,
     kUninitialized,
-    kMigrationTarget,
     kWeakRef,
     kFullTransitionArray,
   };

test/cctest/test-field-type-tracking.cc

@@ -78,14 +78,6 @@ static Handle<AccessorPair> CreateAccessorPair(bool with_getter,
   return pair;
 }
 
-// Check cached migration target map after Map::Update() and Map::TryUpdate()
-static void CheckMigrationTarget(Isolate* isolate, Map* old_map, Map* new_map) {
-  Map* target = TransitionsAccessor(isolate, handle(old_map, isolate))
-                    .GetMigrationTarget();
-  if (!target) return;
-  CHECK_EQ(new_map, target);
-  CHECK_EQ(Map::TryUpdateSlow(isolate, old_map), target);
-}
 
 class Expectations {
   static const int MAX_PROPERTIES = 10;
@@ -715,7 +707,6 @@ static void TestGeneralizeField(int detach_property_at_index,
   // Update all deprecated maps and check that they are now the same.
   Handle<Map> updated_map = Map::Update(isolate, map);
   CHECK_EQ(*new_map, *updated_map);
-  CheckMigrationTarget(isolate, *map, *updated_map);
 }
 
 static void TestGeneralizeField(const CRFTData& from, const CRFTData& to,
@@ -976,11 +967,9 @@ TEST(GeneralizeFieldWithAccessorProperties) {
   // Update all deprecated maps and check that they are now the same.
   Handle<Map> updated_map = Map::Update(isolate, map);
   CHECK_EQ(*active_map, *updated_map);
-  CheckMigrationTarget(isolate, *map, *updated_map);
   for (int i = 0; i < kPropCount; i++) {
     updated_map = Map::Update(isolate, maps[i]);
     CHECK_EQ(*active_map, *updated_map);
-    CheckMigrationTarget(isolate, *maps[i], *updated_map);
   }
 }
 
@@ -1071,7 +1060,6 @@ static void TestReconfigureDataFieldAttribute_GeneralizeField(
   // Update deprecated |map|, it should become |new_map|.
   Handle<Map> updated_map = Map::Update(isolate, map);
   CHECK_EQ(*new_map, *updated_map);
-  CheckMigrationTarget(isolate, *map, *updated_map);
 }
 
 // This test ensures that trivial field generalization (from HeapObject to
@@ -1382,7 +1370,6 @@ struct CheckDeprecated {
     // Update deprecated |map|, it should become |new_map|.
     Handle<Map> updated_map = Map::Update(isolate, map);
     CHECK_EQ(*new_map, *updated_map);
-    CheckMigrationTarget(isolate, *map, *updated_map);
   }
 };
 
@@ -1841,7 +1828,6 @@ static void TestReconfigureElementsKind_GeneralizeField(
   // Update deprecated |map|, it should become |new_map|.
   Handle<Map> updated_map = Map::Update(isolate, map);
   CHECK_EQ(*new_map, *updated_map);
-  CheckMigrationTarget(isolate, *map, *updated_map);
 
   // Ensure Map::FindElementsKindTransitionedMap() is able to find the
   // transitioned map.
@@ -2353,11 +2339,9 @@ static void TestGeneralizeFieldWithSpecialTransition(TestConfig& config,
   // Update all deprecated maps and check that they are now the same.
   Handle<Map> updated_map = Map::Update(isolate, map);
   CHECK_EQ(*active_map, *updated_map);
-  CheckMigrationTarget(isolate, *map, *updated_map);
   for (int i = 0; i < kPropCount; i++) {
     updated_map = Map::Update(isolate, maps[i]);
     CHECK_EQ(*active_map, *updated_map);
-    CheckMigrationTarget(isolate, *maps[i], *updated_map);
   }
 }
 
@@ -2639,7 +2623,6 @@ struct FieldGeneralizationChecker {
     CHECK_NE(*map1, *map2);
     Handle<Map> updated_map = Map::Update(isolate, map1);
     CHECK_EQ(*map2, *updated_map);
-    CheckMigrationTarget(isolate, *map1, *updated_map);
 
     expectations2.SetDataField(descriptor_, attributes_, constness_,
                                representation_, heap_type_);