 Chromium Code Reviews
 Chromium Code Reviews| Index: src/global-handles.cc | 
| diff --git a/src/global-handles.cc b/src/global-handles.cc | 
| index cb3115abfca7db7ce9baec4a29c9da134437c223..cfa9ee5d9e5d8e035b91a3c12625fbd01a2eebda 100644 | 
| --- a/src/global-handles.cc | 
| +++ b/src/global-handles.cc | 
| @@ -36,11 +36,6 @@ namespace v8 { | 
| namespace internal { | 
| -ObjectGroup::~ObjectGroup() { | 
| - if (info_ != NULL) info_->Dispose(); | 
| -} | 
| - | 
| - | 
| class GlobalHandles::Node { | 
| public: | 
| // State transition diagram: | 
| @@ -578,45 +573,74 @@ void GlobalHandles::IterateNewSpaceWeakIndependentRoots(ObjectVisitor* v) { | 
| bool GlobalHandles::IterateObjectGroups(ObjectVisitor* v, | 
| WeakSlotCallbackWithHeap can_skip) { | 
| - int last = 0; | 
| + if (object_groups_.length() == 0) | 
| + return false; | 
| + | 
| + object_groups_.Sort(); | 
| 
Michael Starzinger
2013/04/10 10:54:16
Isn't there a sort call to the retained_object_inf
 
marja
2013/04/10 13:14:51
Yes! Thanks for pointing that out.
 | 
| + | 
| + // During the iteration, some of the elements of object_groups are | 
| + // deleted. This is done by moving surviving elements at the front of the list | 
| + // and deleting from the end. This index tracks where the next surviving | 
| + // element should be moved. | 
| + int surviving_element_ix = 0; | 
| 
Michael Starzinger
2013/04/10 10:54:16
nit: Can we spell out the "index" in these variabl
 
marja
2013/04/10 13:14:51
Done.
 | 
| + int info_ix = 0; // For iterating retained_object_infos_. | 
| + int surviving_info_ix = 0; | 
| + | 
| + UniqueId current_group_id; | 
| + size_t current_group_start = 0; | 
| bool any_group_was_visited = false; | 
| - for (int i = 0; i < object_groups_.length(); i++) { | 
| - ObjectGroup* entry = object_groups_.at(i); | 
| - ASSERT(entry != NULL); | 
| - | 
| - Object*** objects = entry->objects_; | 
| - bool group_should_be_visited = false; | 
| - for (size_t j = 0; j < entry->length_; j++) { | 
| - Object* object = *objects[j]; | 
| - if (object->IsHeapObject()) { | 
| - if (!can_skip(isolate_->heap(), &object)) { | 
| - group_should_be_visited = true; | 
| - break; | 
| + | 
| + for (int i = 0; i <= object_groups_.length(); ++i) { | 
| + if (i == 0) | 
| + current_group_id = object_groups_[i].id; | 
| + if (i == object_groups_.length() || | 
| + current_group_id != object_groups_[i].id) { | 
| + // Group detected: objects in indices [current_group_start, i[. | 
| + bool group_should_be_visited = false; | 
| + for (int j = current_group_start; j < i; ++j) { | 
| + Object* object = *(object_groups_[j].object); | 
| + if (object->IsHeapObject()) { | 
| + if (!can_skip(isolate_->heap(), &object)) { | 
| + group_should_be_visited = true; | 
| + break; | 
| + } | 
| } | 
| } | 
| - } | 
| - | 
| - if (!group_should_be_visited) { | 
| - object_groups_[last++] = entry; | 
| - continue; | 
| - } | 
| - | 
| - // An object in the group requires visiting, so iterate over all | 
| - // objects in the group. | 
| - for (size_t j = 0; j < entry->length_; ++j) { | 
| - Object* object = *objects[j]; | 
| - if (object->IsHeapObject()) { | 
| - v->VisitPointer(&object); | 
| - any_group_was_visited = true; | 
| + if (!group_should_be_visited) { | 
| 
Michael Starzinger
2013/04/10 10:54:16
nit: Can we add an empty newline in front of this
 
marja
2013/04/10 13:14:51
Done.
 | 
| + for (int j = current_group_start; j < i; ++j) | 
| + object_groups_[surviving_element_ix++] = object_groups_[j]; | 
| + } else { | 
| + // An object in the group requires visiting, so iterate over all | 
| + // objects in the group. | 
| + for (int j = current_group_start; j < i; ++j) { | 
| + Object* object = *(object_groups_[j].object); | 
| + if (object->IsHeapObject()) { | 
| + v->VisitPointer(&object); | 
| + any_group_was_visited = true; | 
| + } | 
| + } | 
| + } | 
| + if (info_ix < retained_object_infos_.length() && | 
| 
Michael Starzinger
2013/04/10 10:54:16
Likewise for this if.
 
marja
2013/04/10 13:14:51
Done.
 | 
| + retained_object_infos_[info_ix].id == | 
| + object_groups_[current_group_start].id) { | 
| + // This object group has an associated GroupRetainedObjectInfo. | 
| + if (!group_should_be_visited) { | 
| + retained_object_infos_[surviving_info_ix++] = | 
| + retained_object_infos_[info_ix]; | 
| + } else if (retained_object_infos_[info_ix].info != NULL) { | 
| + retained_object_infos_[info_ix].info->Dispose(); | 
| + retained_object_infos_[info_ix].info = NULL; | 
| + } | 
| + ++info_ix; | 
| + } | 
| + if (i < object_groups_.length()) { | 
| + current_group_id = object_groups_[i].id; | 
| + current_group_start = i; | 
| } | 
| } | 
| - | 
| - // Once the entire group has been iterated over, set the object | 
| - // group to NULL so it won't be processed again. | 
| - entry->Dispose(); | 
| - object_groups_.at(i) = NULL; | 
| } | 
| - object_groups_.Rewind(last); | 
| + object_groups_.Rewind(surviving_element_ix); | 
| + retained_object_infos_.Rewind(surviving_info_ix); | 
| return any_group_was_visited; | 
| } | 
| @@ -824,7 +848,24 @@ void GlobalHandles::AddObjectGroup(Object*** handles, | 
| if (info != NULL) info->Dispose(); | 
| return; | 
| } | 
| - object_groups_.Add(ObjectGroup::New(handles, length, info)); | 
| + for (size_t i = 0; i < length; ++i) { | 
| + object_groups_.Add( | 
| + ObjectGroupConnection(UniqueId((intptr_t)handles[0]), handles[i])); | 
| 
Michael Starzinger
2013/04/10 10:54:16
nit: Use "static_cast<intptr_t>" instead of the C-
 
marja
2013/04/10 13:14:51
Done.
 | 
| + } | 
| + retained_object_infos_.Add( | 
| + GroupRetainedObjectInfo(UniqueId((intptr_t)handles[0]), info)); | 
| 
Michael Starzinger
2013/04/10 10:54:16
Likewise.
 
marja
2013/04/10 13:14:51
Done.
 | 
| +} | 
| + | 
| + | 
| +void GlobalHandles::SetObjectGroupId(Object** handle, | 
| + UniqueId id) { | 
| + object_groups_.Add(ObjectGroupConnection(id, handle)); | 
| +} | 
| + | 
| + | 
| +void GlobalHandles::SetRetainedObjectInfo(UniqueId id, | 
| + RetainedObjectInfo* info) { | 
| + retained_object_infos_.Add(GroupRetainedObjectInfo(id, info)); | 
| } | 
| @@ -843,10 +884,12 @@ void GlobalHandles::AddImplicitReferences(HeapObject** parent, | 
| void GlobalHandles::RemoveObjectGroups() { | 
| - for (int i = 0; i < object_groups_.length(); i++) { | 
| - object_groups_.at(i)->Dispose(); | 
| - } | 
| object_groups_.Clear(); | 
| + for (int i = 0; i < retained_object_infos_.length(); ++i) { | 
| + if (retained_object_infos_[i].info != NULL) | 
| + retained_object_infos_[i].info->Dispose(); | 
| + } | 
| + retained_object_infos_.Clear(); | 
| } |