Revert of Replace ad-hoc weakness in transition array with WeakCell. (patchset...

Revert of Replace ad-hoc weakness in transition array with WeakCell. (patchset #5 id:80001 of https://codereview.chromium.org/1157943003/)

Reason for revert:
Breaks descriptor array clearing.

Original issue's description:
> Replace ad-hoc weakness in transition array with WeakCell.
>
> BUG=
>
> Committed: https://crrev.com/885455e99de817f86a0b5df2dc0d932cfc179749
> Cr-Commit-Position: refs/heads/master@{#29083}

TBR=jkummerow@chromium.org,hpayer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

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

src/heap/mark-compact.cc

@@ -2468,39 +2468,25 @@ void MarkCompactCollector::ClearMapTransitions(Map* map, Map* dead_transition) {
 
   int transition_index = 0;
 
-  // This flag will be cleared if we find the live owner in the loop below.
-  bool descriptors_owner_died = true;
+  bool descriptors_owner_died = false;
 
   // Compact all live descriptors to the left.
-  if (TransitionArray::IsFullTransitionArray(transitions)) {
-    for (int i = 0; i < num_transitions; ++i) {
-      WeakCell* target_cell =
-          TransitionArray::cast(transitions)->GetTargetCell(i);
-      if (!target_cell->cleared()) {
-        if (Map::cast(target_cell->value())->instance_descriptors() ==
-            descriptors) {
-          descriptors_owner_died = false;
-        }
-        if (i != transition_index) {
-          TransitionArray* t = TransitionArray::cast(transitions);
-          Name* key = t->GetKey(i);
-          t->SetKey(transition_index, key);
-          Object** key_slot = t->GetKeySlot(transition_index);
-          RecordSlot(key_slot, key_slot, key);
-          WeakCell* target_cell = t->GetTargetCell(i);
-          t->SetTargetCell(transition_index, target_cell);
-          Object** target_slot = t->GetTargetSlot(transition_index);
-          RecordSlot(target_slot, target_slot, target_cell);
-        }
-        transition_index++;
+  for (int i = 0; i < num_transitions; ++i) {
+    Map* target = TransitionArray::GetTarget(transitions, i);
+    if (ClearMapBackPointer(target)) {
+      if (target->instance_descriptors() == descriptors) {
+        descriptors_owner_died = true;
       }
-    }
-  } else if (transitions->IsWeakCell()) {
-    WeakCell* target_cell = WeakCell::cast(transitions);
-    if (!target_cell->cleared()) {
-      if (Map::cast(target_cell->value())->instance_descriptors() ==
-          descriptors) {
-        descriptors_owner_died = false;
+    } else {
+      if (i != transition_index) {
+        DCHECK(TransitionArray::IsFullTransitionArray(transitions));
+        TransitionArray* t = TransitionArray::cast(transitions);
+        Name* key = t->GetKey(i);
+        t->SetKey(transition_index, key);
+        Object** key_slot = t->GetKeySlot(transition_index);
+        RecordSlot(key_slot, key_slot, key);
+        // Target slots do not need to be recorded since maps are not compacted.
+        t->SetTarget(transition_index, t->GetTarget(i));
       }
       transition_index++;
     }

src/heap/objects-visiting-inl.h

@@ -537,6 +537,11 @@ void StaticMarkingVisitor<StaticVisitor>::VisitJSDataView(Map* map,
 template <typename StaticVisitor>
 void StaticMarkingVisitor<StaticVisitor>::MarkMapContents(Heap* heap,
                                                           Map* map) {
+  Object* raw_transitions = map->raw_transitions();
+  if (TransitionArray::IsFullTransitionArray(raw_transitions)) {
+    MarkTransitionArray(heap, TransitionArray::cast(raw_transitions));
+  }
+
   // Since descriptor arrays are potentially shared, ensure that only the
   // descriptors that belong to this map are marked. The first time a
   // non-empty descriptor array is marked, its header is also visited. The slot
@@ -565,6 +570,23 @@ void StaticMarkingVisitor<StaticVisitor>::MarkMapContents(Heap* heap,
 }
 
 
+template <typename StaticVisitor>
+void StaticMarkingVisitor<StaticVisitor>::MarkTransitionArray(
+    Heap* heap, TransitionArray* transitions) {
+  if (!StaticVisitor::MarkObjectWithoutPush(heap, transitions)) return;
+
+  if (transitions->HasPrototypeTransitions()) {
+    StaticVisitor::VisitPointer(heap,
+                                transitions->GetPrototypeTransitionsSlot());
+  }
+
+  int num_transitions = TransitionArray::NumberOfTransitions(transitions);
+  for (int i = 0; i < num_transitions; ++i) {
+    StaticVisitor::VisitPointer(heap, transitions->GetKeySlot(i));
+  }
+}
+
+
 template <typename StaticVisitor>
 void StaticMarkingVisitor<StaticVisitor>::MarkInlinedFunctionsCode(Heap* heap,
                                                                    Code* code) {

src/heap/objects-visiting.h

@@ -438,6 +438,7 @@ class StaticMarkingVisitor : public StaticVisitorBase {
   // Mark pointers in a Map and its TransitionArray together, possibly
   // treating transitions or back pointers weak.
   static void MarkMapContents(Heap* heap, Map* map);
+  static void MarkTransitionArray(Heap* heap, TransitionArray* transitions);
 
   // Code flushing support.
   INLINE(static bool IsFlushable(Heap* heap, JSFunction* function));

src/transitions-inl.h

@@ -47,12 +47,6 @@ Object** TransitionArray::GetKeySlot(int transition_number) {
 }
 
 
-Object** TransitionArray::GetTargetSlot(int transition_number) {
-  DCHECK(transition_number < number_of_transitions());
-  return RawFieldOfElementAt(ToTargetIndex(transition_number));
-}
-
-
 Name* TransitionArray::GetKey(int transition_number) {
   DCHECK(transition_number < number_of_transitions());
   return Name::cast(get(ToKeyIndex(transition_number)));
@@ -77,8 +71,7 @@ void TransitionArray::SetKey(int transition_number, Name* key) {
 
 Map* TransitionArray::GetTarget(int transition_number) {
   DCHECK(transition_number < number_of_transitions());
-  WeakCell* cell = GetTargetCell(transition_number);
-  return Map::cast(cell->value());
+  return Map::cast(get(ToTargetIndex(transition_number)));
 }
 
 
@@ -93,13 +86,7 @@ Map* TransitionArray::GetTarget(Object* raw_transitions,
 }
 
 
-WeakCell* TransitionArray::GetTargetCell(int transition_number) {
-  DCHECK(transition_number < number_of_transitions());
-  return WeakCell::cast(get(ToTargetIndex(transition_number)));
-}
-
-
-void TransitionArray::SetTargetCell(int transition_number, WeakCell* value) {
+void TransitionArray::SetTarget(int transition_number, Map* value) {
   DCHECK(transition_number < number_of_transitions());
   set(ToTargetIndex(transition_number), value);
 }
@@ -171,9 +158,13 @@ PropertyDetails TransitionArray::GetTargetDetails(Name* name, Map* target) {
 }
 
 
-void TransitionArray::Set(int transition_number, Name* key, WeakCell* target) {
-  set(ToKeyIndex(transition_number), key);
-  set(ToTargetIndex(transition_number), target);
+void TransitionArray::NoIncrementalWriteBarrierSet(int transition_number,
+                                                   Name* key,
+                                                   Map* target) {
+  FixedArray::NoIncrementalWriteBarrierSet(
+      this, ToKeyIndex(transition_number), key);
+  FixedArray::NoIncrementalWriteBarrierSet(
+      this, ToTargetIndex(transition_number), target);
 }
 
 

src/transitions.cc

@@ -17,11 +17,11 @@ void TransitionArray::Insert(Handle<Map> map, Handle<Name> name,
                              Handle<Map> target, SimpleTransitionFlag flag) {
   Isolate* isolate = map->GetIsolate();
   target->SetBackPointer(*map);
-  Handle<WeakCell> cell = Map::WeakCellForMap(target);
 
   // If the map doesn't have any transitions at all yet, install the new one.
   if (CanStoreSimpleTransition(map->raw_transitions())) {
     if (flag == SIMPLE_PROPERTY_TRANSITION) {
+      Handle<WeakCell> cell = Map::WeakCellForMap(target);
       ReplaceTransitions(map, *cell);
       return;
     }
@@ -42,6 +42,7 @@ void TransitionArray::Insert(Handle<Map> map, Handle<Name> name,
     if (flag == SIMPLE_PROPERTY_TRANSITION && key->Equals(*name) &&
         old_details.kind() == new_details.kind() &&
         old_details.attributes() == new_details.attributes()) {
+      Handle<WeakCell> cell = Map::WeakCellForMap(target);
       ReplaceTransitions(map, *cell);
       return;
     }
@@ -50,8 +51,8 @@ void TransitionArray::Insert(Handle<Map> map, Handle<Name> name,
     // Re-read existing data; the allocation might have caused it to be cleared.
     if (IsSimpleTransition(map->raw_transitions())) {
       old_target = GetSimpleTransition(map->raw_transitions());
-      result->Set(0, GetSimpleTransitionKey(old_target),
-                  GetSimpleTransitionCell(map->raw_transitions()));
+      result->NoIncrementalWriteBarrierSet(
+          0, GetSimpleTransitionKey(old_target), old_target);
     } else {
       result->SetNumberOfTransitions(0);
     }
@@ -82,7 +83,7 @@ void TransitionArray::Insert(Handle<Map> map, Handle<Name> name,
                             &insertion_index);
     // If an existing entry was found, overwrite it and return.
     if (index != kNotFound) {
-      array->SetTargetCell(index, *cell);
+      array->SetTarget(index, *target);
       return;
     }
 
@@ -95,10 +96,10 @@ void TransitionArray::Insert(Handle<Map> map, Handle<Name> name,
       array->SetNumberOfTransitions(new_nof);
       for (index = number_of_transitions; index > insertion_index; --index) {
         array->SetKey(index, array->GetKey(index - 1));
-        array->SetTargetCell(index, array->GetTargetCell(index - 1));
+        array->SetTarget(index, array->GetTarget(index - 1));
       }
       array->SetKey(index, *name);
-      array->SetTargetCell(index, *cell);
+      array->SetTarget(index, *target);
       SLOW_DCHECK(array->IsSortedNoDuplicates());
       return;
     }
@@ -144,11 +145,11 @@ void TransitionArray::Insert(Handle<Map> map, Handle<Name> name,
 
   DCHECK_NE(kNotFound, insertion_index);
   for (int i = 0; i < insertion_index; ++i) {
-    result->CopyFrom(array, i, i);
+    result->NoIncrementalWriteBarrierCopyFrom(array, i, i);
   }
-  result->Set(insertion_index, *name, *cell);
+  result->NoIncrementalWriteBarrierSet(insertion_index, *name, *target);
   for (int i = insertion_index; i < number_of_transitions; ++i) {
-    result->CopyFrom(array, i, i + 1);
+    result->NoIncrementalWriteBarrierCopyFrom(array, i, i + 1);
   }
 
   SLOW_DCHECK(result->IsSortedNoDuplicates());
@@ -348,10 +349,12 @@ Handle<TransitionArray> TransitionArray::Allocate(Isolate* isolate,
 }
 
 
-void TransitionArray::CopyFrom(TransitionArray* origin, int origin_transition,
-                               int target_transition) {
-  Set(target_transition, origin->GetKey(origin_transition),
-      origin->GetTargetCell(origin_transition));
+void TransitionArray::NoIncrementalWriteBarrierCopyFrom(TransitionArray* origin,
+                                                        int origin_transition,
+                                                        int target_transition) {
+  NoIncrementalWriteBarrierSet(target_transition,
+                               origin->GetKey(origin_transition),
+                               origin->GetTarget(origin_transition));
 }
 
 
@@ -419,9 +422,9 @@ void TransitionArray::EnsureHasFullTransitionArray(Handle<Map> map) {
     result->Shrink(ToKeyIndex(0));
     result->SetNumberOfTransitions(0);
   } else if (nof == 1) {
-    WeakCell* target_cell = GetSimpleTransitionCell(raw_transitions);
-    Name* key = GetSimpleTransitionKey(Map::cast(target_cell->value()));
-    result->Set(0, key, target_cell);
+    Map* target = GetSimpleTransition(raw_transitions);
+    Name* key = GetSimpleTransitionKey(target);
+    result->NoIncrementalWriteBarrierSet(0, key, target);
   }
   ReplaceTransitions(map, *result);
 }

src/transitions.h

@@ -71,11 +71,6 @@ class TransitionArray: public FixedArray {
     DCHECK(raw_transition->IsWeakCell());
     return Map::cast(WeakCell::cast(raw_transition)->value());
   }
-  static inline WeakCell* GetSimpleTransitionCell(Object* raw_transition) {
-    DCHECK(IsSimpleTransition(raw_transition));
-    DCHECK(raw_transition->IsWeakCell());
-    return WeakCell::cast(raw_transition);
-  }
   static inline bool IsFullTransitionArray(Object* raw_transitions) {
     return raw_transitions->IsTransitionArray();
   }
@@ -140,7 +135,6 @@ class TransitionArray: public FixedArray {
   inline Name* GetKey(int transition_number);
   inline void SetKey(int transition_number, Name* value);
   inline Object** GetKeySlot(int transition_number);
-  inline Object** GetTargetSlot(int transition_number);
   int GetSortedKeyIndex(int transition_number) { return transition_number; }
 
   Name* GetSortedKey(int transition_number) {
@@ -149,9 +143,7 @@ class TransitionArray: public FixedArray {
 
   static inline Map* GetTarget(Object* raw_transitions, int transition_number);
   inline Map* GetTarget(int transition_number);
-
-  inline WeakCell* GetTargetCell(int transition_number);
-  inline void SetTargetCell(int transition_number, WeakCell* target);
+  inline void SetTarget(int transition_number, Map* target);
 
   static inline PropertyDetails GetTargetDetails(Name* name, Map* target);
 
@@ -291,11 +283,14 @@ class TransitionArray: public FixedArray {
                                    PropertyKind kind2,
                                    PropertyAttributes attributes2);
 
-  inline void Set(int transition_number, Name* key, WeakCell* target_cell);
+  inline void NoIncrementalWriteBarrierSet(int transition_number,
+                                           Name* key,
+                                           Map* target);
 
   // Copy a single transition from the origin array.
-  inline void CopyFrom(TransitionArray* origin, int origin_transition,
-                       int target_transition);
+  inline void NoIncrementalWriteBarrierCopyFrom(TransitionArray* origin,
+                                                int origin_transition,
+                                                int target_transition);
 
 #ifdef DEBUG
   static void CheckNewTransitionsAreConsistent(Handle<Map> map,