From 7719981078dc090e136c736302d8f41f39fa800d Mon Sep 17 00:00:00 2001 From: "svenpanne@chromium.org" Date: Wed, 25 Jan 2012 14:22:59 +0000 Subject: [PATCH] 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 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10501 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/mark-compact.cc | 158 +++++++++++++++++++++++--------------------- src/mark-compact.h | 2 + 2 files changed, 83 insertions(+), 77 deletions(-) diff --git a/src/mark-compact.cc b/src/mark-compact.cc index 16e51468cb..3b0415f2f7 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(); + ClearNonLivePrototypeTransitions(map); + ClearNonLiveMapTransitions(map, map_mark); + } +} - 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++; + +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(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); } - if (new_number_of_transitions != number_of_transitions) { - map->SetNumberOfProtoTransitions(new_number_of_transitions); - } + Object** slot = HeapObject::RawField(current, Map::kPrototypeOffset); + *slot = real_prototype; + if (current_is_alive) RecordSlot(slot, slot, real_prototype); - // 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(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(next); - } + current = reinterpret_cast(next); + current_is_alive = Marking::MarkBitFrom(current).Get(); } } diff --git a/src/mark-compact.h b/src/mark-compact.h index 93f3fd2548..a911b49d2e 100644 --- a/src/mark-compact.h +++ b/src/mark-compact.h @@ -696,6 +696,8 @@ class MarkCompactCollector { // 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