Splitting DescriptorArray::CopyInsert into CopyInsert and CopyReplace.

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 1535 matching lines @@
     return AddSlowProperty(name, value, attributes);
   }
 
   DescriptorArray* old_descriptors = map()->instance_descriptors();
   // Compute the new index for new field.
   int index = map()->NextFreePropertyIndex();
 
   // Allocate new instance descriptors with (name, index) added
   FieldDescriptor new_field(name, index, attributes, 0);
   DescriptorArray* new_descriptors;
-  { MaybeObject* maybe_new_descriptors =
-        old_descriptors->CopyInsert(&new_field);
+  { MaybeObject* maybe_new_descriptors = old_descriptors->CopyAdd(&new_field);
     if (!maybe_new_descriptors->To(&new_descriptors)) {
       return maybe_new_descriptors;
     }
   }
 
   // Only allow map transition if the object isn't the global object.
   bool allow_map_transition =
       (isolate->context()->global_context()->object_function()->map() != map());
 
   ASSERT(index < map()->inobject_properties() ||
@@ skipping 40 matching lines @@
 
 
 MaybeObject* JSObject::AddConstantFunctionProperty(
     String* name,
     JSFunction* function,
     PropertyAttributes attributes) {
   // Allocate new instance descriptors with (name, function) added
   ConstantFunctionDescriptor d(name, function, attributes, 0);
   DescriptorArray* new_descriptors;
   { MaybeObject* maybe_new_descriptors =
-        map()->instance_descriptors()->CopyInsert(&d);
+        map()->instance_descriptors()->CopyAdd(&d);
     if (!maybe_new_descriptors->To(&new_descriptors)) {
       return maybe_new_descriptors;
     }
   }
 
   // Allocate a new map for the object.
   Map* new_map;
   { MaybeObject* maybe_new_map = map()->CopyReplaceDescriptors(new_descriptors);
     if (!maybe_new_map->To(&new_map)) return maybe_new_map;
   }
@@ skipping 2958 matching lines @@
   { 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()->CopyInsert(&callbacks_descr2);
+        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;
   }
 
   // step 4: create a new getter/setter pair with a transition to the new map
@@ skipping 1248 matching lines @@
   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(int insertion_index,
+                                          Descriptor* descriptor) {
+  ASSERT(0 <= insertion_index && insertion_index < number_of_descriptors());
+
+  // Ensure the key is a symbol.
+  { MaybeObject* maybe_result = descriptor->KeyToSymbol();
+    if (maybe_result->IsFailure()) return maybe_result;
+  }
+
+  int size = number_of_descriptors();
+
+  DescriptorArray* new_descriptors;
+  { MaybeObject* maybe_result = Allocate(size, MAY_BE_SHARED);
+    if (!maybe_result->To(&new_descriptors)) return maybe_result;
+  }
+
+  descriptor->SetEnumerationIndex(GetDetails(insertion_index).index());

[Michael Starzinger, 2012/07/12 13:17:42]

Can we move this after the WhitenessWitness. It doesn't matter functionally, but
it keeps the code in sync with CopyAdd() a few lines below.

+
+  FixedArray::WhitenessWitness witness(new_descriptors);
+
+  // Copy the descriptors, inserting or replacing a descriptor.

[Michael Starzinger, 2012/07/12 13:17:42]

Drop "inserting or" from the comment.

+  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->Set(insertion_index, descriptor, witness);
+  new_descriptors->SetLastAdded(LastAdded());
+

[Michael Starzinger, 2012/07/12 13:17:42]

Maybe we should add "SLOW_ASSERT(new_descriptors->IsSortedNoDuplicates())" here
as well.

+  return new_descriptors;
+}
+
 
 MaybeObject* DescriptorArray::CopyInsert(Descriptor* descriptor) {
   // Ensure the key is a symbol.
   { MaybeObject* maybe_result = descriptor->KeyToSymbol();
     if (maybe_result->IsFailure()) return maybe_result;
   }
 
-  int new_size = number_of_descriptors();
+  // We replace the key if it is already present.
+  int index = SearchWithCache(descriptor->GetKey());
+  if (index == kNotFound) return CopyAdd(descriptor);
+  return CopyReplace(index, descriptor);
+}
 
-  // If key is in descriptor, we replace it in-place when filtering.
-  // Count a null descriptor for key as inserted, not replaced.
-  int index = SearchWithCache(descriptor->GetKey());
-  const bool replacing = (index != kNotFound);
-  bool keep_enumeration_index = false;
-  if (replacing) {
-    // We are replacing an existing descriptor. We keep the enumeration index
-    // of a visible property.
-    keep_enumeration_index = true;
-  } else {
-    ++new_size;
+
+MaybeObject* DescriptorArray::CopyAdd(Descriptor* descriptor) {
+  // Ensure the key is a symbol.
+  { MaybeObject* maybe_result = descriptor->KeyToSymbol();
+    if (maybe_result->IsFailure()) return maybe_result;
   }
 
+  String* key = descriptor->GetKey();
+  ASSERT(Search(key) == kNotFound);
+
+  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;
   }
 
   FixedArray::WhitenessWitness witness(new_descriptors);
 
-  // Set the enumeration index in the descriptors and set the enumeration index
-  // in the result.
-  if (keep_enumeration_index) {
-    descriptor->SetEnumerationIndex(GetDetails(index).index());
-  } else {
-    descriptor->SetEnumerationIndex(NextEnumerationIndex());
+  descriptor->SetEnumerationIndex(NextEnumerationIndex());
+
+  // Copy the descriptors, inserting or replacing a descriptor.

[Michael Starzinger, 2012/07/12 13:17:42]

Drop "or replacing" from, the comment.

+  int insertion_index = -1;
+  int to = 0;
+  for (int from = 0; from < number_of_descriptors(); ++from) {
+    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;
   }
 

[Michael Starzinger, 2012/07/12 13:17:42]

Remove this empty newline.

-  // Copy the descriptors, inserting or replacing a descriptor.
-  int to_index = 0;
-  int insertion_index = -1;
-  int from_index = 0;
-  while (from_index < number_of_descriptors()) {
-    if (insertion_index < 0 &&
-        InsertionPointFound(GetKey(from_index), descriptor->GetKey())) {
-      insertion_index = to_index++;
-      if (replacing) from_index++;
-    } else {
-      MaybeObject* copy_result =
-          new_descriptors->CopyFrom(to_index++, this, from_index, witness);
-      if (copy_result->IsFailure()) return copy_result;
-      from_index++;
-    }
-  }
-  if (insertion_index < 0) insertion_index = to_index++;
+  if (insertion_index < 0) insertion_index = to++;
 
-  ASSERT(insertion_index < new_descriptors->number_of_descriptors());
+  ASSERT(to == new_descriptors->number_of_descriptors());
+
   new_descriptors->Set(insertion_index, descriptor, witness);
+  new_descriptors->SetLastAdded(insertion_index);
 
-  if (!replacing) {
-    new_descriptors->SetLastAdded(insertion_index);
-  } else {
-    new_descriptors->SetLastAdded(LastAdded());
-  }
-
-  ASSERT(to_index == new_descriptors->number_of_descriptors());
   SLOW_ASSERT(new_descriptors->IsSortedNoDuplicates());
 
   return new_descriptors;
 }
 
 
 MaybeObject* DescriptorArray::Copy(SharedMode shared_mode) {
   // Allocate the new descriptor array.
   int number_of_descriptors = this->number_of_descriptors();
   DescriptorArray* new_descriptors;
@@ skipping 7336 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