ClearNonLiveTransitions has to hold on to non-map values.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 1544836fe3bac3c4e796f1abd22d9b1a74874eec..035caf90ec3bd687452df298cf355897281101ef 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -7397,17 +7397,14 @@ void String::PrintOn(FILE* file) {
 
 // Clear a possible back pointer in case the transition leads to a dead map.
 // Return true in case a back pointer has been cleared and false otherwise.
-// Set *keep_entry to true when a live map transition has been found.
-static bool ClearBackPointer(Heap* heap, Object* target, bool* keep_entry) {
-  if (!target->IsMap()) return false;
+static bool ClearBackPointer(Heap* heap, Object* target) {
+  ASSERT(target->IsMap());
   Map* map = Map::cast(target);
   if (Marking::MarkBitFrom(map).Get()) {

[Sven Panne, 2012/06/05 10:52:01]

Nit: To be more consistent with the rest of the code, remove the braces and put
the return on the same line.

[Toon Verwaest, 2012/06/05 11:23:28]

Done.

-    *keep_entry = true;
     return false;
-  } else {
-    map->SetBackPointer(heap->undefined_value(), SKIP_WRITE_BARRIER);
-    return true;
   }
+  map->SetBackPointer(heap->undefined_value(), SKIP_WRITE_BARRIER);
+  return true;
 }
 
 
@@ -7427,17 +7424,22 @@ void Map::ClearNonLiveTransitions(Heap* heap) {
     switch (details.type()) {
       case MAP_TRANSITION:
       case CONSTANT_TRANSITION:
-        ClearBackPointer(heap, d->GetValue(i), &keep_entry);
+        keep_entry = !ClearBackPointer(heap, d->GetValue(i));
         break;
       case ELEMENTS_TRANSITION: {
         Object* object = d->GetValue(i);
         if (object->IsMap()) {
-          ClearBackPointer(heap, object, &keep_entry);
+          keep_entry = !ClearBackPointer(heap, object);
         } else {
           FixedArray* array = FixedArray::cast(object);
           for (int j = 0; j < array->length(); ++j) {
-            if (ClearBackPointer(heap, array->get(j), &keep_entry)) {
-              array->set_undefined(j);
+            Object* target = array->get(j);
+            if (target->IsMap()) {
+              if (ClearBackPointer(heap, array->get(j))) {

[Sven Panne, 2012/06/05 10:52:01]

Just use "target", it's cleaner... ;-)

[Toon Verwaest, 2012/06/05 11:23:28]

Done.

+                array->set_undefined(j);
+              } else {
+                keep_entry = true;
+              }
             }
           }
         }
@@ -7447,11 +7449,25 @@ void Map::ClearNonLiveTransitions(Heap* heap) {
         Object* object = d->GetValue(i);
         if (object->IsAccessorPair()) {
           AccessorPair* accessors = AccessorPair::cast(object);
-          if (ClearBackPointer(heap, accessors->getter(), &keep_entry)) {
-            accessors->set_getter(heap->the_hole_value());
+          Object* getter = accessors->getter();
+          Object* setter = accessors->setter();

[Sven Panne, 2012/06/05 10:52:01]

Move this down before "if (setter->IsMap())...", again making everything as
local as possible.

[Toon Verwaest, 2012/06/05 11:23:28]

Done.

+          if (getter->IsMap()) {
+            if (ClearBackPointer(heap, getter)) {
+              accessors->set_getter(heap->the_hole_value());
+            } else {
+              keep_entry = true;
+            }
+          } else if (!getter->IsTheHole()) {
+            keep_entry = true;
           }
-          if (ClearBackPointer(heap, accessors->setter(), &keep_entry)) {
-            accessors->set_setter(heap->the_hole_value());
+          if (setter->IsMap()) {
+            if (ClearBackPointer(heap, setter)) {
+              accessors->set_setter(heap->the_hole_value());
+            } else {
+              keep_entry = true;
+            }
+          } else if (!getter->IsTheHole()) {
+            keep_entry = true;
           }
         } else {
           keep_entry = true;