Removed transitions from the accessor pair descriptors.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index de41ce1eb99781b7a1ad8a9745e03d393e0c37b0..7ba64bd1b15b6792abc55c6038682f79582c6c69 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2859,18 +2859,11 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* result,
                                               attributes,
                                               strict_mode);
     case TRANSITION: {
-      Object* transition = result->GetTransitionValue();
+      Map* transition_map = result->GetTransitionTarget();
 
-      if (transition->IsAccessorPair()) {
-        ASSERT(!AccessorPair::cast(transition)->ContainsAccessor());
-        return ConvertDescriptorToField(*name, *value, attributes);
-      }
-
-      Map* transition_map = Map::cast(transition);
       DescriptorArray* descriptors = transition_map->instance_descriptors();
       int descriptor = descriptors->LastAdded();
       PropertyDetails details = descriptors->GetDetails(descriptor);
-      ASSERT(details.type() == FIELD || details.type() == CONSTANT_FUNCTION);
 
       if (details.type() == FIELD) {
         if (attributes == details.attributes()) {
@@ -2881,9 +2874,12 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* result,
                                                field_index);
         }
         return self->ConvertDescriptorToField(*name, *value, attributes);
+      } else if (details.type() == CALLBACKS) {
+        return ConvertDescriptorToField(*name, *value, attributes);
       }
 
-      // Is transition to CONSTANT_FUNCTION.
+      ASSERT(details.type() == CONSTANT_FUNCTION);
+
       Object* constant_function = descriptors->GetValue(descriptor);
       // If the same constant function is being added we can simply
       // transition to the target map.
@@ -2983,18 +2979,11 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
       // Override callback in clone
       return ConvertDescriptorToField(name, value, attributes);
     case TRANSITION: {
-      Object* transition = result.GetTransitionValue();
-
-      if (transition->IsAccessorPair()) {
-        ASSERT(!AccessorPair::cast(transition)->ContainsAccessor());
-        return ConvertDescriptorToField(name, value, attributes);
-      }
+      Map* transition_map = result.GetTransitionTarget();
 
-      Map* transition_map = Map::cast(transition);
       DescriptorArray* descriptors = transition_map->instance_descriptors();
       int descriptor = descriptors->LastAdded();
       PropertyDetails details = descriptors->GetDetails(descriptor);
-      ASSERT(details.type() == FIELD || details.type() == CONSTANT_FUNCTION);
 
       if (details.type() == FIELD) {
         if (attributes == details.attributes()) {
@@ -3005,10 +2994,14 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
                                          field_index);
         }
         return ConvertDescriptorToField(name, value, attributes);
+      } else if (details.type() == CALLBACKS) {
+        return ConvertDescriptorToField(name, value, attributes);
       }
 
-      // Was transition to CONSTANT_FUNCTION. Replace with a map transition to a
-      // new map with a FIELD, even if the value is a function.
+      ASSERT(details.type() == CONSTANT_FUNCTION);
+
+      // Replace transition to CONSTANT FUNCTION with a map transition to a new
+      // map with a FIELD, even if the value is a function.
       return ConvertTransitionToMapTransition(
           result.GetTransitionIndex(), name, value, attributes);
     }
@@ -3294,11 +3287,6 @@ MaybeObject* JSObject::NormalizeProperties(PropertyNormalizationMode mode,
       }
       case CALLBACKS: {
         Object* value = descs->GetCallbacksObject(i);
-        if (value->IsAccessorPair()) {
-          MaybeObject* maybe_copy =
-              AccessorPair::cast(value)->CopyWithoutTransitions();
-          if (!maybe_copy->To(&value)) return maybe_copy;
-        }
         MaybeObject* maybe_dictionary =
             dictionary->Add(descs->GetKey(i), value, details);
         if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary;
@@ -4231,7 +4219,7 @@ void JSObject::LookupCallback(String* name, LookupResult* result) {
        current != heap->null_value() && current->IsJSObject();
        current = JSObject::cast(current)->GetPrototype()) {
     JSObject::cast(current)->LocalLookupRealNamedProperty(name, result);
-    if (result->IsCallbacks()) return;
+    if (result->IsPropertyCallbacks()) return;
   }
   result->NotFound();
 }
@@ -4343,7 +4331,7 @@ MaybeObject* JSObject::CreateAccessorPairFor(String* name) {
     // DefinePropertyAccessor below.
     Object* obj = result.GetCallbackObject();
     if (obj->IsAccessorPair()) {
-      return AccessorPair::cast(obj)->CopyWithoutTransitions();
+      return AccessorPair::cast(obj)->Copy();
     }
   }
   return GetHeap()->AllocateAccessorPair();
@@ -4377,9 +4365,9 @@ MaybeObject* JSObject::DefinePropertyAccessor(String* name,
   }
 
   AccessorPair* accessors;
-  { MaybeObject* maybe_accessors = CreateAccessorPairFor(name);
-    if (!maybe_accessors->To(&accessors)) return maybe_accessors;
-  }
+  MaybeObject* maybe_accessors = CreateAccessorPairFor(name);

[Sven Panne, 2012/07/16 13:49:43]

Using a local scope makes it very clear that maybe_accessors is only used for
testing/conversion immediately afterwards. It is a common v8 idiom (a.k.a. Maybe
monad ;-), so I would prefer to leave the scope as it is.

+  if (!maybe_accessors->To(&accessors)) return maybe_accessors;
+
   accessors->SetComponents(getter, setter);
   return SetPropertyCallback(name, accessors, attributes);
 }
@@ -4451,17 +4439,16 @@ MaybeObject* JSObject::SetPropertyCallback(String* name,
                                            Object* structure,
                                            PropertyAttributes attributes) {
   // Normalize object to make this operation simple.
-  { MaybeObject* maybe_ok = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
-    if (maybe_ok->IsFailure()) return maybe_ok;
-  }
+  MaybeObject* maybe_ok = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);

[Sven Panne, 2012/07/16 13:49:43]

See above.

+  if (maybe_ok->IsFailure()) return maybe_ok;
 
   // For the global object allocate a new map to invalidate the global inline
   // caches which have a global property cell reference directly in the code.
   if (IsGlobalObject()) {
     Map* new_map;
-    { MaybeObject* maybe_new_map = map()->CopyDropDescriptors();
-      if (!maybe_new_map->To(&new_map)) return maybe_new_map;
-    }
+    MaybeObject* maybe_new_map = map()->CopyDropDescriptors();

[Sven Panne, 2012/07/16 13:49:43]

See above.

+    if (!maybe_new_map->To(&new_map)) return maybe_new_map;
+
     set_map(new_map);
     // When running crankshaft, changing the map is not enough. We
     // need to deoptimize all functions that rely on this global
@@ -4471,9 +4458,8 @@ MaybeObject* JSObject::SetPropertyCallback(String* name,
 
   // Update the dictionary with the new CALLBACKS property.
   PropertyDetails details = PropertyDetails(attributes, CALLBACKS);
-  { MaybeObject* maybe_ok = SetNormalizedProperty(name, structure, details);
-    if (maybe_ok->IsFailure()) return maybe_ok;
-  }
+  maybe_ok = SetNormalizedProperty(name, structure, details);

[Sven Panne, 2012/07/16 13:49:43]

See above.

+  if (maybe_ok->IsFailure()) return maybe_ok;
 
   return GetHeap()->undefined_value();
 }
@@ -4525,73 +4511,32 @@ MaybeObject* JSObject::DefineAccessor(String* name,
 }
 
 
-static MaybeObject* CreateFreshAccessor(JSObject* obj,
-                                        String* name,
-                                        AccessorComponent component,
-                                        Object* accessor,
-                                        PropertyAttributes attributes) {
-  // step 1: create a new getter/setter pair with only the accessor in it
-  Heap* heap = obj->GetHeap();
-  AccessorPair* accessors2;
-  { MaybeObject* maybe_accessors2 = heap->AllocateAccessorPair();
-    if (!maybe_accessors2->To(&accessors2)) return maybe_accessors2;
-  }
-  accessors2->set(component, accessor);
-
-  // step 2: create a copy of the descriptors, incl. the new getter/setter pair
-  Map* map1 = obj->map();
-  CallbacksDescriptor callbacks_descr2(name, accessors2, attributes, 0);
-  DescriptorArray* descriptors2;
-  { MaybeObject* maybe_descriptors2 =
-        map1->instance_descriptors()->CopyAdd(&callbacks_descr2);
-    if (!maybe_descriptors2->To(&descriptors2)) return maybe_descriptors2;
-  }
-
-  // step 3: create a new map with the new descriptors
-  Map* map2;
-  { MaybeObject* maybe_map2 = map1->CopyReplaceDescriptors(descriptors2);
-    if (!maybe_map2->To(&map2)) return maybe_map2;
-  }
+static MaybeObject* TryAccessorTransition(JSObject* self,
+                                          Map* transitioned_map,
+                                          String* name,
+                                          AccessorComponent component,
+                                          Object* accessor,
+                                          PropertyAttributes attributes) {
+  DescriptorArray* descs = transitioned_map->instance_descriptors();
+  int number = descs->LastAdded();
+  PropertyDetails details = descs->GetDetails(number);
 
-  // step 4: create a new getter/setter pair with a transition to the new map
-  AccessorPair* accessors1;
-  { MaybeObject* maybe_accessors1 = heap->AllocateAccessorPair();
-    if (!maybe_accessors1->To(&accessors1)) return maybe_accessors1;
-  }
-  accessors1->set(component, map2);
+  // If the transition target was not callbacks, fall back to the slow case.
+  if (details.type() != CALLBACKS) return self->GetHeap()->null_value();
 
-  // step 5: create a copy of the descriptors, incl. the new getter/setter pair
-  // with the transition
-  TransitionArray* new_transitions;
-  { MaybeObject* maybe_new_transitions = map1->AddTransition(name, accessors1);
-    if (!maybe_new_transitions->To(&new_transitions)) {
-      return maybe_new_transitions;
-    }
-  }
+  Object* target_accessor =
+      AccessorPair::cast(descs->GetCallbacksObject(number))->get(component);
+  PropertyAttributes target_attributes = details.attributes();
 
-  // step 6: everything went well so far, so we make our changes visible
-  { MaybeObject* transition_added = map1->set_transitions(new_transitions);
-    if (transition_added->IsFailure()) return transition_added;
+  // Reuse transition if adding same accessor with same attributes.
+  if (target_accessor == accessor && target_attributes == attributes) {
+    self->set_map(transitioned_map);
+    return self;
   }
 
-  map2->SetBackPointer(map1);
-  obj->set_map(map2);
-  return obj;
-}
-
-
-static bool TransitionToSameAccessor(Object* map,
-                                     String* name,
-                                     AccessorComponent component,
-                                     Object* accessor,
-                                     PropertyAttributes attributes ) {
-  Map* transitioned_map = Map::cast(map);
-  DescriptorArray* descs = transitioned_map->instance_descriptors();
-  int number = descs->LastAdded();
-  Object* target_accessor =
-      AccessorPair::cast(descs->GetCallbacksObject(number))->get(component);
-  PropertyAttributes target_attributes = descs->GetDetails(number).attributes();
-  return target_accessor == accessor && target_attributes == attributes;
+  // If either not the same accessor, or not the same attributes, fall back to
+  // the slow case.
+  return self->GetHeap()->null_value();
 }
 
 
@@ -4600,43 +4545,31 @@ static MaybeObject* NewCallbackTransition(JSObject* obj,
                                           AccessorComponent component,
                                           Object* accessor,
                                           PropertyAttributes attributes,
-                                          AccessorPair* accessors2) {
-  // step 1: copy the old getter/setter pair and set the new accessor
-  AccessorPair* accessors3;
-  { MaybeObject* maybe_accessors3 = accessors2->CopyWithoutTransitions();
-    if (!maybe_accessors3->To(&accessors3)) return maybe_accessors3;
-  }
-  accessors3->set(component, accessor);
-
-  // step 2: create a copy of the descriptors, incl. the new getter/setter pair
-  Map* map2 = obj->map();
-  CallbacksDescriptor callbacks_descr3(name, accessors3, attributes, 0);
-  DescriptorArray* descriptors3;
-  { MaybeObject* maybe_descriptors3 =
-        map2->instance_descriptors()->CopyInsert(&callbacks_descr3);
-    if (!maybe_descriptors3->To(&descriptors3)) return maybe_descriptors3;
-  }
+                                          AccessorPair* new_accessors) {
+  // step 1: create a copy of the descriptors, incl. the new getter/setter pair
+  Map* old_map = obj->map();
+  CallbacksDescriptor new_accessors_desc(name, new_accessors, attributes);
+  DescriptorArray* descriptors;
+  MaybeObject* maybe_descriptors =
+      old_map->instance_descriptors()->CopyInsert(&new_accessors_desc);
+  if (!maybe_descriptors->To(&descriptors)) return maybe_descriptors;
 
-  // step 3: create a new map with the new descriptors
-  Map* map3;
-  { MaybeObject* maybe_map3 = map2->CopyReplaceDescriptors(descriptors3);
-    if (!maybe_map3->To(&map3)) return maybe_map3;
-  }
+  // step 2: create a new map with the new descriptors
+  Map* new_map;
+  MaybeObject* maybe_new_map = old_map->CopyReplaceDescriptors(descriptors);
+  if (!maybe_new_map->To(&new_map)) return maybe_new_map;
 
-  // step 4: add a new transition to the new map
-  TransitionArray* new_transitions;
-  { MaybeObject* maybe_transitions = map2->AddTransition(name, accessors2);
-    if (!maybe_transitions->To(&new_transitions)) return maybe_transitions;
-  }
+  // step 3: add a new transition to the new map
+  TransitionArray* transitions;
+  MaybeObject* maybe_transitions = old_map->AddTransition(name, new_map);
+  if (!maybe_transitions->To(&transitions)) return maybe_transitions;
 
-  // step 5: everything went well so far, so we make our changes visible
-  { MaybeObject* transition_added = map2->set_transitions(new_transitions);
-    if (transition_added->IsFailure()) return transition_added;
-  }
+  // step 4: everything went well so far, so we make our changes visible
+  MaybeObject* transition_added = old_map->set_transitions(transitions);
+  if (transition_added->IsFailure()) return transition_added;
 
-  map3->SetBackPointer(map2);
-  obj->set_map(map3);
-  accessors2->set(component, map3);
+  new_map->SetBackPointer(old_map);
+  obj->set_map(new_map);
   return obj;
 }
 
@@ -4649,40 +4582,54 @@ MaybeObject* JSObject::DefineFastAccessor(String* name,
   LookupResult result(GetIsolate());
   LocalLookup(name, &result);
 
-  // If we have a new property, create a fresh accessor plus a transition to it.
-  if (!result.IsFound()) {
-    return CreateFreshAccessor(this, name, component, accessor, attributes);
-  }
+  if (result.IsFound()
+      && !result.IsPropertyCallbacks()
+      && !result.IsTransition()) return GetHeap()->null_value();
 
-  // If the property is not a JavaScript accessor, fall back to the slow case.
-  if (!result.IsCallbacks()) return GetHeap()->null_value();
+  // Return success if the same accessor with the same attributes already exist.
+  AccessorPair* source_accessors = NULL;
+  if (result.IsPropertyCallbacks()) {
+    Object* callback_value = result.GetCallbackObject();
+    if (callback_value->IsAccessorPair()) {
+      source_accessors = AccessorPair::cast(callback_value);
+      Object* entry = source_accessors->get(component);
+      if (entry == accessor && result.GetAttributes() == attributes) {
+        return this;
+      }
+    }
+  }
 
-  Object* callback_value = result.GetCallbackObject();
-  if (!callback_value->IsAccessorPair()) return GetHeap()->null_value();
-  AccessorPair* accessors = AccessorPair::cast(callback_value);
+  // If not, lookup a transition.
+  map()->LookupTransition(this, name, &result);
 
-  // Follow a callback transition, if there is a fitting one.
-  Object* entry = accessors->get(component);
-  if (entry->IsMap() &&
-      TransitionToSameAccessor(entry, name, component, accessor, attributes)) {
-    set_map(Map::cast(entry));
-    return this;
+  // If there is a transition, try to follow it.
+  if (result.IsFound()) {
+    Map* target = result.GetTransitionTarget();
+    return TryAccessorTransition(
+        this, target, name, component, accessor, attributes);
   }
 
-  if (entry == accessor && result.GetAttributes() == attributes) return this;
+  // If there is no transition yet, add a transition to the a new accessor pair
+  // containing the accessor.
+  AccessorPair* accessors;
+  MaybeObject* maybe_accessors;

[Sven Panne, 2012/07/16 13:49:43]

Scope again...

 
-  // Only the other accessor has been set so far, create a new transition.
-  if (entry->IsTheHole()) {
-    return NewCallbackTransition(this,
-                                 name,
-                                 component,
-                                 accessor,
-                                 attributes,
-                                 accessors);
+  // Allocate a new pair if there were no source accessors. Otherwise, copy the
+  // pair and modify the accessor.
+  if (source_accessors != NULL) {
+    maybe_accessors = source_accessors->Copy();
+  } else {
+    maybe_accessors = GetHeap()->AllocateAccessorPair();
   }
+  if (!maybe_accessors->To(&accessors)) return maybe_accessors;
+  accessors->set(component, accessor);
 
-  // Nothing from the above worked, so we have to fall back to the slow case.
-  return GetHeap()->null_value();
+  return NewCallbackTransition(this,
+                               name,
+                               component,
+                               accessor,
+                               attributes,
+                               accessors);
 }
 
 
@@ -5017,39 +4964,17 @@ class IntrusiveMapTransitionIterator {
     // next descriptor by adding 2 to the index. The exceptions are the
     // CALLBACKS entries: An even index means we look at its getter, and an odd
     // index means we look at its setter.
-    int raw_index = Smi::cast(*TransitionArrayHeader())->value();
-    int index = raw_index / 2;
+    int index = Smi::cast(*TransitionArrayHeader())->value();
     int number_of_transitions = transition_array_->number_of_transitions();
     while (index < number_of_transitions) {
-      Object* value = transition_array_->GetValue(index);
-
-      if (value->IsMap()) {
-        *TransitionArrayHeader() = Smi::FromInt(raw_index + 2);
-        return static_cast<Map*>(value);
-      }
-
-      ASSERT(value->IsAccessorPair());
-
-      // We might have a map transition in a getter or in a setter.
-      AccessorPair* accessors = static_cast<AccessorPair*>(value);
-      Object* accessor;
-      if ((raw_index & 1) == 0) {
-        accessor = accessors->setter();
-      } else {
-        ++index;
-        accessor = accessors->getter();
-      }
-      ++raw_index;
-      if (accessor->IsMap()) {
-        *TransitionArrayHeader() = Smi::FromInt(raw_index);
-        return static_cast<Map*>(accessor);
-      }
+      *TransitionArrayHeader() = Smi::FromInt(index + 1);
+      return transition_array_->GetTarget(index);
     }
 
-    if (index == transition_array_->number_of_transitions() &&
+    if (index == number_of_transitions &&
         transition_array_->HasElementsTransition()) {
       Map* elements_transition = transition_array_->elements_transition();
-      *TransitionArrayHeader() = Smi::FromInt(raw_index + 2);
+      *TransitionArrayHeader() = Smi::FromInt(index + 1);
       return elements_transition;
     }
     *TransitionArrayHeader() = transition_array_->GetHeap()->fixed_array_map();
@@ -5786,30 +5711,14 @@ static bool InsertionPointFound(String* key1, String* key2) {
 }
 
 
-void DescriptorArray::CopyFrom(Handle<DescriptorArray> dst,
-                               int dst_index,
-                               Handle<DescriptorArray> src,
+void DescriptorArray::CopyFrom(int dst_index,
+                               DescriptorArray* src,
                                int src_index,
                                const WhitenessWitness& witness) {
-  CALL_HEAP_FUNCTION_VOID(dst->GetIsolate(),
-                          dst->CopyFrom(dst_index, *src, src_index, witness));
-}
-
-
-MaybeObject* DescriptorArray::CopyFrom(int dst_index,
-                                       DescriptorArray* src,
-                                       int src_index,
-                                       const WhitenessWitness& witness) {
   Object* value = src->GetValue(src_index);
   PropertyDetails details = src->GetDetails(src_index);
-  if (details.type() == CALLBACKS && value->IsAccessorPair()) {
-    MaybeObject* maybe_copy =
-        AccessorPair::cast(value)->CopyWithoutTransitions();
-    if (!maybe_copy->To(&value)) return maybe_copy;
-  }
   Descriptor desc(src->GetKey(src_index), value, details);
   Set(dst_index, &desc, witness);
-  return this;
 }
 
 MaybeObject* DescriptorArray::CopyReplace(Descriptor* descriptor,
@@ -5833,9 +5742,7 @@ MaybeObject* DescriptorArray::CopyReplace(Descriptor* descriptor,
   // Copy the descriptors, replacing a descriptor.
   for (int index = 0; index < size; ++index) {
     if (index == insertion_index) continue;
-    MaybeObject* copy_result =
-        new_descriptors->CopyFrom(index, this, index, witness);
-    if (copy_result->IsFailure()) return copy_result;
+    new_descriptors->CopyFrom(index, this, index, witness);
   }
 
   descriptor->SetEnumerationIndex(GetDetails(insertion_index).index());
@@ -5850,9 +5757,8 @@ MaybeObject* DescriptorArray::CopyReplace(Descriptor* descriptor,
 
 MaybeObject* DescriptorArray::CopyInsert(Descriptor* descriptor) {
   // Ensure the key is a symbol.
-  { MaybeObject* maybe_result = descriptor->KeyToSymbol();
-    if (maybe_result->IsFailure()) return maybe_result;
-  }
+  MaybeObject* maybe_result = descriptor->KeyToSymbol();

[Sven Panne, 2012/07/16 13:49:43]

Scope...

+  if (maybe_result->IsFailure()) return maybe_result;
 
   // We replace the key if it is already present.
   int index = SearchWithCache(descriptor->GetKey());
@@ -5863,9 +5769,8 @@ MaybeObject* DescriptorArray::CopyInsert(Descriptor* descriptor) {
 
 MaybeObject* DescriptorArray::CopyAdd(Descriptor* descriptor) {
   // Ensure the key is a symbol.
-  { MaybeObject* maybe_result = descriptor->KeyToSymbol();
-    if (maybe_result->IsFailure()) return maybe_result;
-  }
+  MaybeObject* maybe_result = descriptor->KeyToSymbol();

[Sven Panne, 2012/07/16 13:49:43]

Scope...

+  if (maybe_result->IsFailure()) return maybe_result;
 
   String* key = descriptor->GetKey();
   ASSERT(Search(key) == kNotFound);
@@ -5873,9 +5778,8 @@ MaybeObject* DescriptorArray::CopyAdd(Descriptor* descriptor) {
   int new_size = number_of_descriptors() + 1;
 
   DescriptorArray* new_descriptors;
-  { MaybeObject* maybe_result = Allocate(new_size, MAY_BE_SHARED);
-    if (!maybe_result->To(&new_descriptors)) return maybe_result;
-  }
+  MaybeObject* maybe_descriptors = Allocate(new_size, MAY_BE_SHARED);

[Sven Panne, 2012/07/16 13:49:43]

Scope...

+  if (!maybe_descriptors->To(&new_descriptors)) return maybe_descriptors;
 
   FixedArray::WhitenessWitness witness(new_descriptors);
 
@@ -5886,9 +5790,7 @@ MaybeObject* DescriptorArray::CopyAdd(Descriptor* descriptor) {
     if (insertion_index < 0 && InsertionPointFound(GetKey(from), key)) {
       insertion_index = to++;
     }
-    MaybeObject* copy_result =
-        new_descriptors->CopyFrom(to++, this, from, witness);
-    if (copy_result->IsFailure()) return copy_result;
+    new_descriptors->CopyFrom(to++, this, from, witness);
   }
   if (insertion_index < 0) insertion_index = to++;
 
@@ -5909,18 +5811,14 @@ MaybeObject* DescriptorArray::Copy(SharedMode shared_mode) {
   // Allocate the new descriptor array.
   int number_of_descriptors = this->number_of_descriptors();
   DescriptorArray* new_descriptors;
-  { MaybeObject* maybe_result = Allocate(number_of_descriptors,
-                                         shared_mode);
-    if (!maybe_result->To(&new_descriptors)) return maybe_result;
-  }
+  MaybeObject* maybe_result = Allocate(number_of_descriptors, shared_mode);

[Sven Panne, 2012/07/16 13:49:43]

Scope...

+  if (!maybe_result->To(&new_descriptors)) return maybe_result;
 
   // Copy the content.
   if (number_of_descriptors > 0) {
     FixedArray::WhitenessWitness witness(new_descriptors);
     for (int i = 0; i < number_of_descriptors; i++) {
-      MaybeObject* copy_result =
-          new_descriptors->CopyFrom(i, this, i, witness);
-      if (copy_result->IsFailure()) return copy_result;
+      new_descriptors->CopyFrom(i, this, i, witness);
     }
     new_descriptors->SetLastAdded(LastAdded());
   }
@@ -6020,14 +5918,14 @@ void DescriptorArray::Sort(const WhitenessWitness& witness) {
 }
 
 
-MaybeObject* AccessorPair::CopyWithoutTransitions() {
+MaybeObject* AccessorPair::Copy() {
   Heap* heap = GetHeap();
   AccessorPair* copy;
-  { MaybeObject* maybe_copy = heap->AllocateAccessorPair();
-    if (!maybe_copy->To(&copy)) return maybe_copy;
-  }
-  copy->set_getter(getter()->IsMap() ? heap->the_hole_value() : getter());
-  copy->set_setter(setter()->IsMap() ? heap->the_hole_value() : setter());
+  MaybeObject* maybe_copy = heap->AllocateAccessorPair();

[Sven Panne, 2012/07/16 13:49:43]

Scope...

+  if (!maybe_copy->To(&copy)) return maybe_copy;
+
+  copy->set_getter(getter());
+  copy->set_setter(setter());
   return copy;
 }
 
@@ -7313,40 +7211,6 @@ static bool ClearBackPointer(Heap* heap, Object* target) {
 }
 
 
-static bool ClearAccessorComponent(Heap* heap,
-                                   AccessorPair* accessors,
-                                   AccessorComponent component) {
-  Object* component_value = accessors->get(component);
-  if (!component_value->IsMap()) return true;
-  if (ClearBackPointer(heap, component_value)) {
-    accessors->set(component, heap->the_hole_value());
-    return true;
-  }
-  return false;
-}
-
-
-static bool ClearNonLiveTransition(Heap* heap,
-                                   TransitionArray* t,
-                                   int transition_index) {
-  // If the value is a map, check if the target is live. If not, clear the
-  // transition. 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.
-  Object* value = t->GetValue(transition_index);
-  if (value->IsMap()) {
-    return ClearBackPointer(heap, t->GetValue(transition_index));
-  }
-
-  ASSERT(value->IsAccessorPair());
-
-  AccessorPair* accessors = AccessorPair::cast(value);
-  bool getter = ClearAccessorComponent(heap, accessors, ACCESSOR_GETTER);
-  bool setter = ClearAccessorComponent(heap, accessors, ACCESSOR_SETTER);
-  return getter && setter;
-}
-
-
 // TODO(mstarzinger): This method should be moved into MarkCompactCollector,
 // because it cannot be called from outside the GC and we already have methods
 // depending on the transitions layout in the GC anyways.
@@ -7362,17 +7226,17 @@ void Map::ClearNonLiveTransitions(Heap* heap) {
 
   // Compact all live descriptors to the left.
   for (int i = 0; i < t->number_of_transitions(); ++i) {
-    if (!ClearNonLiveTransition(heap, t, i)) {
+    if (!ClearBackPointer(heap, t->GetTarget(i))) {
       if (i != transition_index) {
         String* key = t->GetKey(i);
-        Object* value = t->GetValue(i);
+        Map* target = t->GetTarget(i);
         t->SetKey(transition_index, key);
-        t->SetValue(transition_index, value);
+        t->SetTarget(transition_index, target);
         MarkCompactCollector* collector = heap->mark_compact_collector();
         Object** key_slot = t->GetKeySlot(transition_index);
         collector->RecordSlot(key_slot, key_slot, key);
-        Object** value_slot = t->GetValueSlot(transition_index);
-        collector->RecordSlot(value_slot, value_slot, value);
+        Object** target_slot = t->GetTargetSlot(transition_index);
+        collector->RecordSlot(target_slot, target_slot, target);
       }
       transition_index++;
     }
@@ -10472,7 +10336,7 @@ bool JSObject::HasRealNamedCallbackProperty(String* key) {
 
   LookupResult result(isolate);
   LocalLookupRealNamedProperty(key, &result);
-  return result.IsCallbacks();
+  return result.IsPropertyCallbacks();
 }
 
 
@@ -12696,15 +12560,15 @@ MaybeObject* StringDictionary::TransformPropertiesToFastFor(
     if (IsKey(k)) {
       Object* value = ValueAt(i);
       // Ensure the key is a symbol before writing into the instance descriptor.
-      Object* key;
-      { MaybeObject* maybe_key = heap->LookupSymbol(String::cast(k));
-        if (!maybe_key->ToObject(&key)) return maybe_key;
-      }
+      String* key;
+      MaybeObject* maybe_key = heap->LookupSymbol(String::cast(k));

[Sven Panne, 2012/07/16 13:49:43]

Scope...

+      if (!maybe_key->To(&key)) return maybe_key;
+
       PropertyDetails details = DetailsAt(i);
       PropertyType type = details.type();
 
       if (value->IsJSFunction() && !heap->InNewSpace(value)) {
-        ConstantFunctionDescriptor d(String::cast(key),
+        ConstantFunctionDescriptor d(key,
                                      JSFunction::cast(value),
                                      details.attributes(),
                                      details.index());
@@ -12718,18 +12582,13 @@ MaybeObject* StringDictionary::TransformPropertiesToFastFor(
           int offset = current_offset - inobject_props;
           FixedArray::cast(fields)->set(offset, value);
         }
-        FieldDescriptor d(String::cast(key),
+        FieldDescriptor d(key,
                           current_offset++,
                           details.attributes(),
                           details.index());
         descriptors->Set(next_descriptor, &d, witness);
       } else if (type == CALLBACKS) {
-        if (value->IsAccessorPair()) {
-          MaybeObject* maybe_copy =
-              AccessorPair::cast(value)->CopyWithoutTransitions();
-          if (!maybe_copy->To(&value)) return maybe_copy;
-        }
-        CallbacksDescriptor d(String::cast(key),
+        CallbacksDescriptor d(key,
                               value,
                               details.attributes(),
                               details.index());
@@ -12745,10 +12604,8 @@ MaybeObject* StringDictionary::TransformPropertiesToFastFor(
   descriptors->Sort(witness);
   // Allocate new map.
   Map* new_map;
-  { MaybeObject* maybe_new_map =
-        obj->map()->CopyReplaceDescriptors(descriptors);
-    if (!maybe_new_map->To(&new_map)) return maybe_new_map;
-  }
+  MaybeObject* maybe_new_map = obj->map()->CopyReplaceDescriptors(descriptors);

[Sven Panne, 2012/07/16 13:49:43]

Scope...

+  if (!maybe_new_map->To(&new_map)) return maybe_new_map;
 
   new_map->set_unused_property_fields(unused_property_fields);