Revision: 10501
Author:   [email protected]
Date:     Wed Jan 25 06:22:59 2012
Log:      Refactoring only: Extracted 2 methods from ClearNonLiveTransitions

This simple refactoring makes it very clear that clearing non-live transitions actually consists of 2 quite separate things. Things would even be nicer if the prototype transitions were represented by a separate data structure instead of
reusing FixedArray in an interesting way once again.

As an additional bonus, this CL makes it possible to read each of the methods in
question on a 30" screen without scrolling!

Review URL: https://chromiumcodereview.appspot.com/9169045
http://code.google.com/p/v8/source/detail?r=10501

Modified:
 /branches/bleeding_edge/src/mark-compact.cc
 /branches/bleeding_edge/src/mark-compact.h

=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Wed Jan 25 00:50:55 2012
+++ /branches/bleeding_edge/src/mark-compact.cc Wed Jan 25 06:22:59 2012
@@ -2310,88 +2310,92 @@
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++;
-      }
-    }
-
-    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);
-    }
-
-    // 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);
-
-      // 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);
-    }
+    ClearNonLivePrototypeTransitions(map);
+    ClearNonLiveMapTransitions(map, map_mark);
+  }
+}
+
+
+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);
+  }
+
+  // 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);
+  }
+}
+
+
+void MarkCompactCollector::ClearNonLiveMapTransitions(Map* map,
+                                                      MarkBit map_mark) {
+  // 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());
+  }
+
+ // Follow back pointers, setting them to prototype, clearing map transitions
+  // when necessary.
+  Map* current = map;
+  bool current_is_alive = map_mark.Get();
+  bool on_dead_path = !current_is_alive;
+  while (current->IsMap()) {
+    Object* next = current->prototype();
+    // There should never be a dead map above a live map.
+    ASSERT(on_dead_path || current_is_alive);
+
+ // 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 && current_is_alive) {
+      on_dead_path = false;
+      current->ClearNonLiveTransitions(heap(), real_prototype);
+    }
+
+    Object** slot = HeapObject::RawField(current, Map::kPrototypeOffset);
+    *slot = real_prototype;
+    if (current_is_alive) RecordSlot(slot, slot, real_prototype);
+
+    current = reinterpret_cast<Map*>(next);
+    current_is_alive = Marking::MarkBitFrom(current).Get();
   }
 }

=======================================
--- /branches/bleeding_edge/src/mark-compact.h  Wed Jan 25 00:50:55 2012
+++ /branches/bleeding_edge/src/mark-compact.h  Wed Jan 25 06:22:59 2012
@@ -696,6 +696,8 @@
   // Map transitions from a live map to a dead map must be killed.
   // We replace them with a null descriptor, with the same key.
   void ClearNonLiveTransitions();
+  void ClearNonLivePrototypeTransitions(Map* map);
+  void ClearNonLiveMapTransitions(Map* map, MarkBit map_mark);

   // Marking detaches initial maps from SharedFunctionInfo objects
   // to make this reference weak. We need to reattach initial maps

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to