Do not follow accessor map transitions when marking descriptor arrays.

src/mark-compact.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1654 matching lines @@
   // that one of these fields contains a pointer to it.
   Object** start_slot = HeapObject::RawField(map,
                                              Map::kPointerFieldsBeginOffset);
 
   Object** end_slot = HeapObject::RawField(map, Map::kPointerFieldsEndOffset);
 
   StaticMarkingVisitor::VisitPointers(map->GetHeap(), start_slot, end_slot);
 }
 
 
+bool MarkCompactCollector::MarkUnmarked(Object* value) {

[Michael Starzinger, 2012/01/24 13:28:30]

The name is kind of misleading, because it not only handles unmarked objects,
but marked ones as well.

My suggestion is rename this to use a different signature and also to move it to
mark-compact-inl.h. Proposed new signature is:

void MarkCompactCollector::MarkObject(HeapObject* obj);

+  HeapObject* object = HeapObject::cast(value);
+  MarkBit mark = Marking::MarkBitFrom(object);
+  bool old_mark = mark.Get();
+  if (!old_mark) SetMark(object, mark);
+  return old_mark;
+}
+
+
+bool MarkCompactCollector::MarkUnmarkedAndPush(Object* value) {

[Michael Starzinger, 2012/01/24 13:28:30]

Boolean return value is never used, can we drop it?

Can we also move this to mark-compact-inl.h and rename it to
MarkObjectAndPush()?

+  bool old_mark = MarkUnmarked(value);
+  if (!old_mark) marking_deque_.PushBlack(HeapObject::cast(value));
+  return old_mark;
+}
+
+
+void MarkCompactCollector::MarkAccessorPairSlot(AccessorPair* accessors,
+                                                int offset) {
+  Object** slot = HeapObject::RawField(accessors, offset);
+  Object* accessor = *slot;
+  if (accessor->IsMap()) return;
+  RecordSlot(slot, slot, accessor);
+  MarkUnmarkedAndPush(accessor);
+}
+
+
 void MarkCompactCollector::MarkDescriptorArray(
     DescriptorArray* descriptors) {
   MarkBit descriptors_mark = Marking::MarkBitFrom(descriptors);
   if (descriptors_mark.Get()) return;
   // Empty descriptor array is marked as a root before any maps are marked.
   ASSERT(descriptors != heap()->empty_descriptor_array());
   SetMark(descriptors, descriptors_mark);
 
   FixedArray* contents = reinterpret_cast<FixedArray*>(
       descriptors->get(DescriptorArray::kContentArrayIndex));
@@ skipping 12 matching lines @@
     // If the pair (value, details) at index i, i+1 is not
     // a transition or null descriptor, mark the value.
     PropertyDetails details(Smi::cast(contents->get(i + 1)));
 
     Object** slot = contents->data_start() + i;
     Object* value = *slot;
     if (!value->IsHeapObject()) continue;
 
     RecordSlot(slot, slot, *slot);
 
-    if (details.IsProperty()) {
-      HeapObject* object = HeapObject::cast(value);
-      MarkBit mark = Marking::MarkBitFrom(HeapObject::cast(object));
-      if (!mark.Get()) {
-        SetMark(HeapObject::cast(object), mark);
-        marking_deque_.PushBlack(object);
-      }
-    } else if (details.type() == ELEMENTS_TRANSITION && value->IsFixedArray()) {
-      // For maps with multiple elements transitions, the transition maps are
-      // stored in a FixedArray. Keep the fixed array alive but not the maps
-      // that it refers to.
-      HeapObject* object = HeapObject::cast(value);
-      MarkBit mark = Marking::MarkBitFrom(HeapObject::cast(object));
-      if (!mark.Get()) {
-        SetMark(HeapObject::cast(object), mark);
-      }
+    switch (details.type()) {
+      case NORMAL:
+      case FIELD:
+      case CONSTANT_FUNCTION:
+      case HANDLER:
+      case INTERCEPTOR:
+        MarkUnmarkedAndPush(value);
+        break;
+      case CALLBACKS:
+        if (!value->IsAccessorPair()) {
+          MarkUnmarkedAndPush(value);
+        } else if (!MarkUnmarked(value)) {
+          AccessorPair* accessors = AccessorPair::cast(value);
+          MarkAccessorPairSlot(accessors, AccessorPair::kGetterOffset);
+          MarkAccessorPairSlot(accessors, AccessorPair::kSetterOffset);
+        }
+        break;
+      case ELEMENTS_TRANSITION:
+        // For maps with multiple elements transitions, the transition maps are
+        // stored in a FixedArray. Keep the fixed array alive but not the maps
+        // that it refers to.
+        if (value->IsFixedArray()) MarkUnmarked(value);
+        break;
+      case MAP_TRANSITION:
+      case CONSTANT_TRANSITION:
+      case NULL_DESCRIPTOR:
+        break;
     }
   }
   // The DescriptorArray descriptors contains a pointer to its contents array,
   // but the contents array is already marked.
   marking_deque_.PushBlack(descriptors);
 }
 
 
 void MarkCompactCollector::CreateBackPointers() {
   HeapObjectIterator iterator(heap()->map_space());
@@ skipping 2180 matching lines @@
   while (buffer != NULL) {
     SlotsBuffer* next_buffer = buffer->next();
     DeallocateBuffer(buffer);
     buffer = next_buffer;
   }
   *buffer_address = NULL;
 }
 
 
 } }  // namespace v8::internal