Refactoring only: Extracted 2 methods from ClearNonLiveTransitions

src/mark-compact.cc

Index: src/mark-compact.cc
diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index 16e51468cb81390bcb1aeb8b5c89064e392e2fc2..153b32d8e75654089ddf88b98e10918e51cca1e0 100644
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -2310,88 +2310,92 @@ void MarkCompactCollector::ClearNonLiveTransitions() {
       map->unchecked_constructor()->unchecked_shared()->AttachInitialMap(map);
     }
 
-    // Clear dead prototype transitions.
-    int number_of_transitions = map->NumberOfProtoTransitions();
-    FixedArray* prototype_transitions = map->prototype_transitions();
-
-    int new_number_of_transitions = 0;
-    const int header = Map::kProtoTransitionHeaderSize;
-    const int proto_offset =
-        header + Map::kProtoTransitionPrototypeOffset;
-    const int map_offset = header + Map::kProtoTransitionMapOffset;
-    const int step = Map::kProtoTransitionElementsPerEntry;
-    for (int i = 0; i < number_of_transitions; i++) {
-      Object* prototype = prototype_transitions->get(proto_offset + i * step);
-      Object* cached_map = prototype_transitions->get(map_offset + i * step);
-      if (IsMarked(prototype) && IsMarked(cached_map)) {
-        int proto_index = proto_offset + new_number_of_transitions * step;
-        int map_index = map_offset + new_number_of_transitions * step;
-        if (new_number_of_transitions != i) {
-          prototype_transitions->set_unchecked(
-              heap_,
-              proto_index,
-              prototype,
-              UPDATE_WRITE_BARRIER);
-          prototype_transitions->set_unchecked(
-              heap_,
-              map_index,
-              cached_map,
-              SKIP_WRITE_BARRIER);
-        }
-        Object** slot =
-            HeapObject::RawField(prototype_transitions,
-                                 FixedArray::OffsetOfElementAt(proto_index));
-        RecordSlot(slot, slot, prototype);
-        new_number_of_transitions++;
+    ClearNonLivePrototypeTransitions(map);
+    ClearNonLiveMapTransitions(map, map_mark.Get());
+  }
+}
+
+
+void MarkCompactCollector::ClearNonLivePrototypeTransitions(Map* map) {
+  int number_of_transitions = map->NumberOfProtoTransitions();
+  FixedArray* prototype_transitions = map->prototype_transitions();
+
+  int new_number_of_transitions = 0;
+  const int header = Map::kProtoTransitionHeaderSize;
+  const int proto_offset = header + Map::kProtoTransitionPrototypeOffset;
+  const int map_offset = header + Map::kProtoTransitionMapOffset;
+  const int step = Map::kProtoTransitionElementsPerEntry;
+  for (int i = 0; i < number_of_transitions; i++) {
+    Object* prototype = prototype_transitions->get(proto_offset + i * step);
+    Object* cached_map = prototype_transitions->get(map_offset + i * step);
+    if (IsMarked(prototype) && IsMarked(cached_map)) {
+      int proto_index = proto_offset + new_number_of_transitions * step;
+      int map_index = map_offset + new_number_of_transitions * step;
+      if (new_number_of_transitions != i) {
+        prototype_transitions->set_unchecked(
+            heap_,
+            proto_index,
+            prototype,
+            UPDATE_WRITE_BARRIER);
+        prototype_transitions->set_unchecked(
+            heap_,
+            map_index,
+            cached_map,
+            SKIP_WRITE_BARRIER);
       }
+      Object** slot =
+          HeapObject::RawField(prototype_transitions,
+                               FixedArray::OffsetOfElementAt(proto_index));
+      RecordSlot(slot, slot, prototype);
+      new_number_of_transitions++;
     }
+  }
 
-    if (new_number_of_transitions != number_of_transitions) {
-      map->SetNumberOfProtoTransitions(new_number_of_transitions);
-    }
+  if (new_number_of_transitions != number_of_transitions) {
+    map->SetNumberOfProtoTransitions(new_number_of_transitions);
+  }
 
-    // Fill slots that became free with undefined value.
-    for (int i = new_number_of_transitions * step;
-         i < number_of_transitions * step;
-         i++) {
-      prototype_transitions->set_undefined(heap_, header + i);
-    }
+  // Fill slots that became free with undefined value.
+  for (int i = new_number_of_transitions * step;
+       i < number_of_transitions * step;
+       i++) {
+    prototype_transitions->set_undefined(heap_, header + i);
+  }
+}
 
-    // Follow the chain of back pointers to find the prototype.
-    Map* current = map;
-    while (current->IsMap()) {
-      current = reinterpret_cast<Map*>(current->prototype());
-      ASSERT(current->IsHeapObject());
-    }
-    Object* real_prototype = current;
 
-    // Follow back pointers, setting them to prototype,
-    // clearing map transitions when necessary.
-    current = map;
-    bool on_dead_path = !map_mark.Get();
-    Object* next;
-    while (current->IsMap()) {
-      next = current->prototype();
-      // There should never be a dead map above a live map.
-      MarkBit current_mark = Marking::MarkBitFrom(current);
-      bool is_alive = current_mark.Get();
-      ASSERT(on_dead_path || is_alive);
+void MarkCompactCollector::ClearNonLiveMapTransitions(Map* map, bool marked) {

[Michael Starzinger, 2012/01/25 13:15:47]

I don't like boolean flags, they are very confusing. Can we instead pass the
map's MarkBit (i.e. map_mark)? Thereby you can reuse that as the initial
current_mark value as well and it is clear what that argument means.

+  // Follow the chain of back pointers to find the prototype.
+  Map* real_prototype = map;
+  while (real_prototype->IsMap()) {
+    real_prototype = reinterpret_cast<Map*>(real_prototype->prototype());
+    ASSERT(real_prototype->IsHeapObject());
+  }
 
-      // A live map above a dead map indicates a dead transition.
-      // This test will always be false on the first iteration.
-      if (on_dead_path && is_alive) {
-        on_dead_path = false;
-        current->ClearNonLiveTransitions(heap(), real_prototype);
-      }
-      *HeapObject::RawField(current, Map::kPrototypeOffset) =
-          real_prototype;
+  // Follow back pointers, setting them to prototype, clearing map transitions
+  // when necessary.
+  Map* current = map;
+  bool on_dead_path = !marked;
+  while (current->IsMap()) {
+    Object* next = current->prototype();
+    // There should never be a dead map above a live map.
+    MarkBit current_mark = Marking::MarkBitFrom(current);

[Michael Starzinger, 2012/01/25 13:15:47]

Like the above comment said: Initialize outside of loop and update at end of
loop after line 2398.

+    bool is_alive = current_mark.Get();
+    ASSERT(on_dead_path || is_alive);
 
-      if (is_alive) {
-        Object** slot = HeapObject::RawField(current, Map::kPrototypeOffset);
-        RecordSlot(slot, slot, real_prototype);
-      }
-      current = reinterpret_cast<Map*>(next);
+    // A live map above a dead map indicates a dead transition.
+    // This test will always be false on the first iteration.
+    if (on_dead_path && is_alive) {
+      on_dead_path = false;
+      current->ClearNonLiveTransitions(heap(), real_prototype);
+    }
+    *HeapObject::RawField(current, Map::kPrototypeOffset) = real_prototype;
+
+    if (is_alive) {
+      Object** slot = HeapObject::RawField(current, Map::kPrototypeOffset);
+      RecordSlot(slot, slot, real_prototype);
     }
+    current = reinterpret_cast<Map*>(next);
   }
 }