ClearNonLiveTransitions has to hold on to non-map values.

src/objects.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 7379 matching lines @@
 void String::PrintOn(FILE* file) {
   int length = this->length();
   for (int i = 0; i < length; i++) {
     fprintf(file, "%c", Get(i));
   }
 }
 
 
 // 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;
 }
 
 
 void Map::ClearNonLiveTransitions(Heap* heap) {
   DescriptorArray* d = DescriptorArray::cast(
       *RawField(this, Map::kInstanceDescriptorsOrBitField3Offset));
   if (d->IsEmpty()) return;
   Smi* NullDescriptorDetails =
     PropertyDetails(NONE, NULL_DESCRIPTOR).AsSmi();
   for (int i = 0; i < d->number_of_descriptors(); ++i) {
     // If the pair (value, details) is a map transition, check if the target is
     // live. If not, null the descriptor. Also drop the back pointer for that
     // map transition, so that this map is not reached again by following a back
     // pointer from that non-live map.
     bool keep_entry = false;

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

Move the initialization to the CALLBACKS case, this is more local.

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

Done.

     PropertyDetails details(d->GetDetails(i));
     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;
+              }
             }
           }
         }
         break;
       }
       case CALLBACKS: {
         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;
         }
         break;
       }
       case NORMAL:
       case FIELD:
       case CONSTANT_FUNCTION:
       case HANDLER:
@@ skipping 5753 matching lines @@
   set_year(Smi::FromInt(year), SKIP_WRITE_BARRIER);
   set_month(Smi::FromInt(month), SKIP_WRITE_BARRIER);
   set_day(Smi::FromInt(day), SKIP_WRITE_BARRIER);
   set_weekday(Smi::FromInt(weekday), SKIP_WRITE_BARRIER);
   set_hour(Smi::FromInt(hour), SKIP_WRITE_BARRIER);
   set_min(Smi::FromInt(min), SKIP_WRITE_BARRIER);
   set_sec(Smi::FromInt(sec), SKIP_WRITE_BARRIER);
 }
 
 } }  // namespace v8::internal