Couple the enumeration index of a property to the size of the descriptor array where...

Review URL: https://chromiumcodereview.appspot.com/10692185

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12066 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index 54eb9b8a8f7ca9599f4d0131292022bccd1a887e..996721a5797b2f651483531c7ec789c454737ab6 100644 (file)
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -390,32 +390,25 @@ Handle<DescriptorArray> Genesis::ComputeFunctionInstanceDescriptor(
 
   DescriptorArray::WhitenessWitness witness(*descriptors);
 
-  int index = 0;
-
   {  // Add length.
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionLength));
-    CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
   {  // Add name.
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionName));
-    CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
   {  // Add arguments.
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionArguments));
-    CallbacksDescriptor d(
-        *factory()->arguments_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->arguments_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
   {  // Add caller.
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionCaller));
-    CallbacksDescriptor d(*factory()->caller_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->caller_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
   if (prototypeMode != DONT_ADD_PROTOTYPE) {
     // Add prototype.
@@ -423,9 +416,8 @@ Handle<DescriptorArray> Genesis::ComputeFunctionInstanceDescriptor(
       attribs = static_cast<PropertyAttributes>(attribs & ~READ_ONLY);
     }
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionPrototype));
-    CallbacksDescriptor d(
-        *factory()->prototype_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
+    CallbacksDescriptor d(*factory()->prototype_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
 
   descriptors->Sort(witness);
@@ -542,32 +534,25 @@ Handle<DescriptorArray> Genesis::ComputeStrictFunctionInstanceDescriptor(
 
   DescriptorArray::WhitenessWitness witness(*descriptors);
 
-  int index = 0;
   {  // Add length.
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionLength));
-    CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
   {  // Add name.
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionName));
-    CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
   {  // Add arguments.
     Handle<AccessorPair> arguments(factory()->NewAccessorPair());
-    CallbacksDescriptor d(
-        *factory()->arguments_symbol(), *arguments, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->arguments_symbol(), *arguments, attribs);
+    descriptors->Append(&d, witness);
   }
   {  // Add caller.
     Handle<AccessorPair> caller(factory()->NewAccessorPair());
-    CallbacksDescriptor d(
-        *factory()->caller_symbol(), *caller, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
-    ++index;
+    CallbacksDescriptor d(*factory()->caller_symbol(), *caller, attribs);
+    descriptors->Append(&d, witness);
   }
 
   if (prototypeMode != DONT_ADD_PROTOTYPE) {
@@ -576,9 +561,8 @@ Handle<DescriptorArray> Genesis::ComputeStrictFunctionInstanceDescriptor(
       attribs = static_cast<PropertyAttributes>(attribs | READ_ONLY);
     }
     Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionPrototype));
-    CallbacksDescriptor d(
-        *factory()->prototype_symbol(), *f, attribs, index + 1);
-    descriptors->Set(index, &d, witness);
+    CallbacksDescriptor d(*factory()->prototype_symbol(), *f, attribs);
+    descriptors->Append(&d, witness);
   }
 
   descriptors->Sort(witness);
@@ -967,42 +951,33 @@ bool Genesis::InitializeGlobal(Handle<GlobalObject> inner_global,
     DescriptorArray::WhitenessWitness witness(*descriptors);
     PropertyAttributes final =
         static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY);
-    int index = 0;
     {
       // ECMA-262, section 15.10.7.1.
       FieldDescriptor field(heap->source_symbol(),
                             JSRegExp::kSourceFieldIndex,
-                            final,
-                            index + 1);
-      descriptors->Set(index, &field, witness);
-      ++index;
+                            final);
+      descriptors->Append(&field, witness);
     }
     {
       // ECMA-262, section 15.10.7.2.
       FieldDescriptor field(heap->global_symbol(),
                             JSRegExp::kGlobalFieldIndex,
-                            final,
-                            index + 1);
-      descriptors->Set(index, &field, witness);
-      ++index;
+                            final);
+      descriptors->Append(&field, witness);
     }
     {
       // ECMA-262, section 15.10.7.3.
       FieldDescriptor field(heap->ignore_case_symbol(),
                             JSRegExp::kIgnoreCaseFieldIndex,
-                            final,
-                            index + 1);
-      descriptors->Set(index, &field, witness);
-      ++index;
+                            final);
+      descriptors->Append(&field, witness);
     }
     {
       // ECMA-262, section 15.10.7.4.
       FieldDescriptor field(heap->multiline_symbol(),
                             JSRegExp::kMultilineFieldIndex,
-                            final,
-                            index + 1);
-      descriptors->Set(index, &field, witness);
-      ++index;
+                            final);
+      descriptors->Append(&field, witness);
     }
     {
       // ECMA-262, section 15.10.7.5.
@@ -1010,9 +985,8 @@ bool Genesis::InitializeGlobal(Handle<GlobalObject> inner_global,
           static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE);
       FieldDescriptor field(heap->last_index_symbol(),
                             JSRegExp::kLastIndexFieldIndex,
-                            writable,
-                            index + 1);
-      descriptors->Set(index, &field, witness);
+                            writable);
+      descriptors->Append(&field, witness);
     }
     descriptors->Sort(witness);
 
@@ -1158,26 +1132,21 @@ bool Genesis::InitializeGlobal(Handle<GlobalObject> inner_global,
     // Create the descriptor array for the arguments object.
     Handle<DescriptorArray> descriptors = factory->NewDescriptorArray(3);
     DescriptorArray::WhitenessWitness witness(*descriptors);
-    int index = 0;
     {  // length
-      FieldDescriptor d(*factory->length_symbol(), 0, DONT_ENUM, index + 1);
-      descriptors->Set(index, &d, witness);
-      ++index;
+      FieldDescriptor d(*factory->length_symbol(), 0, DONT_ENUM);
+      descriptors->Append(&d, witness);
     }
     {  // callee
       CallbacksDescriptor d(*factory->callee_symbol(),
                             *callee,
-                            attributes,
-                            index + 1);
-      descriptors->Set(index, &d, witness);
-      ++index;
+                            attributes);
+      descriptors->Append(&d, witness);
     }
     {  // caller
       CallbacksDescriptor d(*factory->caller_symbol(),
                             *caller,
-                            attributes,
-                            index + 1);
-      descriptors->Set(index, &d, witness);
+                            attributes);
+      descriptors->Append(&d, witness);
     }
     descriptors->Sort(witness);
 
@@ -1775,22 +1744,20 @@ bool Genesis::InstallNatives() {
         reresult_descriptors->CopyFrom(0, *array_descriptors, old, witness);
     if (copy_result->IsFailure()) return false;
 
-    int index = 1;
+    reresult_descriptors->SetLastAdded(0);
+
     {
       FieldDescriptor index_field(heap()->index_symbol(),
                                   JSRegExpResult::kIndexIndex,
-                                  NONE,
-                                  index + 1);
-      reresult_descriptors->Set(index, &index_field, witness);
-      ++index;
+                                  NONE);
+      reresult_descriptors->Append(&index_field, witness);
     }
 
     {
       FieldDescriptor input_field(heap()->input_symbol(),
                                   JSRegExpResult::kInputIndex,
-                                  NONE,
-                                  index + 1);
-      reresult_descriptors->Set(index, &input_field, witness);
+                                  NONE);
+      reresult_descriptors->Append(&input_field, witness);
     }
     reresult_descriptors->Sort(witness);
 

diff --git a/src/factory.cc b/src/factory.cc
index 7b915371616863782650b973e22d331f4c0806e6..f56db70f14f895d310dc8ebb033ef7e64c3a2d8f 100644 (file)
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -893,7 +893,7 @@ MUST_USE_RESULT static inline MaybeObject* DoCopyAdd(
     String* key,
     Object* value,
     PropertyAttributes attributes) {
-  CallbacksDescriptor desc(key, value, attributes, 0);
+  CallbacksDescriptor desc(key, value, attributes);
   MaybeObject* obj = array->CopyAdd(&desc);
   return obj;
 }
@@ -930,15 +930,16 @@ Handle<DescriptorArray> Factory::CopyAppendCallbackDescriptors(
   DescriptorArray::WhitenessWitness witness(*result);
 
   // Copy the descriptors from the array.
-  for (int i = 0; i < descriptor_count; i++) {
-    DescriptorArray::CopyFrom(result, i, array, i, witness);
+  if (0 < descriptor_count) {
+    result->SetLastAdded(array->LastAdded());
+    for (int i = 0; i < descriptor_count; i++) {
+      DescriptorArray::CopyFrom(result, i, array, i, witness);
+    }
   }
 
   // Fill in new callback descriptors.  Process the callbacks from
   // back to front so that the last callback with a given name takes
   // precedence over previously added callbacks with that name.
-  int added_descriptor_count = descriptor_count;
-  int next_enum = array->NextEnumerationIndex();
   for (int i = nof_callbacks - 1; i >= 0; i--) {
     Handle<AccessorInfo> entry =
         Handle<AccessorInfo>(AccessorInfo::cast(callbacks.get(i)));
@@ -946,26 +947,26 @@ Handle<DescriptorArray> Factory::CopyAppendCallbackDescriptors(
     Handle<String> key =
         SymbolFromString(Handle<String>(String::cast(entry->name())));
     // Check if a descriptor with this name already exists before writing.
-    if (LinearSearch(*result, EXPECT_UNSORTED, *key, added_descriptor_count) ==
+    if (LinearSearch(*result,
+                     EXPECT_UNSORTED,
+                     *key,
+                     result->NumberOfSetDescriptors()) ==
         DescriptorArray::kNotFound) {
-      CallbacksDescriptor desc(*key,
-                               *entry,
-                               entry->property_attributes(),
-                               next_enum++);
-      result->Set(added_descriptor_count, &desc, witness);
-      added_descriptor_count++;
+      CallbacksDescriptor desc(*key, *entry, entry->property_attributes());
+      result->Append(&desc, witness);
     }
   }
 
+  int new_number_of_descriptors = result->NumberOfSetDescriptors();
   // Return the old descriptor array if there were no new elements.
-  if (added_descriptor_count == descriptor_count) return array;
+  if (new_number_of_descriptors == descriptor_count) return array;
 
   // If duplicates were detected, allocate a result of the right size
   // and transfer the elements.
-  if (added_descriptor_count < result->length()) {
+  if (new_number_of_descriptors < result->length()) {
     Handle<DescriptorArray> new_result =
-        NewDescriptorArray(added_descriptor_count);
-    for (int i = 0; i < added_descriptor_count; i++) {
+        NewDescriptorArray(new_number_of_descriptors);
+    for (int i = 0; i < new_number_of_descriptors; i++) {
       DescriptorArray::CopyFrom(new_result, i, result, i, witness);
     }
     result = new_result;
@@ -973,7 +974,6 @@ Handle<DescriptorArray> Factory::CopyAppendCallbackDescriptors(
 
   // Sort the result before returning.
   result->Sort(witness);
-  ASSERT(result->NextEnumerationIndex() == next_enum);
   return result;
 }
 

diff --git a/src/objects-inl.h b/src/objects-inl.h
index fbb0183815083b93fe410b498c968372f7f71df4..4a7f8a4a602842bb049c1b32e94945e64a9cfa8e 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -2095,6 +2095,7 @@ void DescriptorArray::Set(int descriptor_number,
                           const WhitenessWitness&) {
   // Range check.
   ASSERT(descriptor_number < number_of_descriptors());
+  ASSERT(desc->GetDetails().index() <= number_of_descriptors());
   ASSERT(desc->GetDetails().index() > 0);
 
   NoIncrementalWriteBarrierSet(this,
@@ -2109,6 +2110,16 @@ void DescriptorArray::Set(int descriptor_number,
 }
 
 
+void DescriptorArray::Append(Descriptor* desc,
+                             const WhitenessWitness& witness) {
+  int descriptor_number = NumberOfSetDescriptors();
+  int enumeration_index = descriptor_number + 1;
+  desc->SetEnumerationIndex(enumeration_index);
+  Set(descriptor_number, desc, witness);
+  SetLastAdded(descriptor_number);
+}
+
+
 void DescriptorArray::NoIncrementalWriteBarrierSwapDescriptors(
     int first, int second) {
   NoIncrementalWriteBarrierSwap(this, ToKeyIndex(first), ToKeyIndex(second));

diff --git a/src/objects.cc b/src/objects.cc
index ba407b8c6ef17267676c96ecb52fc9ac8b749eec..67bb67a7e26cba1883d80dd3aa7a350af7ddc952 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -5764,7 +5764,7 @@ MaybeObject* DescriptorArray::Allocate(int number_of_descriptors,
     if (!maybe_array->To(&result)) return maybe_array;
   }
 
-  result->set(kLastAddedIndex, Smi::FromInt(-1));
+  result->set(kLastAddedIndex, Smi::FromInt(kNoneAdded));
   result->set(kTransitionsIndex, Smi::FromInt(0));
   return result;
 }
@@ -5907,7 +5907,8 @@ MaybeObject* DescriptorArray::CopyAdd(Descriptor* descriptor) {
 
   ASSERT(to == new_descriptors->number_of_descriptors());
 
-  descriptor->SetEnumerationIndex(NextEnumerationIndex());
+  ASSERT(new_size == NextEnumerationIndex());
+  descriptor->SetEnumerationIndex(new_size);
   new_descriptors->Set(insertion_index, descriptor, witness);
   new_descriptors->SetLastAdded(insertion_index);
 
@@ -5950,6 +5951,9 @@ void DescriptorArray::SortUnchecked(const WhitenessWitness& witness) {
   // Nothing to sort.
   if (len == 0) return;
 
+  ASSERT(LastAdded() == kNoneAdded ||
+         GetDetails(LastAdded()).index() == number_of_descriptors());
+
   // Bottom-up max-heap construction.
   // Index of the last node with children
   const int max_parent_index = (len / 2) - 1;
@@ -5997,18 +6001,29 @@ void DescriptorArray::SortUnchecked(const WhitenessWitness& witness) {
     }
   }
 
-  int last_enum_index = -1;
-  int last_added = -1;
+#ifdef DEBUG
+  // Ensure that all enumeration indexes between 1 and length occur uniquely in
+  // the descriptor array.
+  for (int i = 1; i <= len; ++i) {
+    int j;
+    for (j = 0; j < len; ++j) {
+      if (GetDetails(j).index() == i) break;
+    }
+    ASSERT(j != len);
+    for (j++; j < len; ++j) {
+      ASSERT(GetDetails(j).index() != i);
+    }
+  }
+#endif
+
   for (int i = 0; i < len; ++i) {
-    int current_enum = GetDetails(i).index();
-    if (current_enum > last_enum_index) {
-      last_added = i;
-      last_enum_index = current_enum;
+    if (GetDetails(i).index() == len) {
+      SetLastAdded(i);
+      return;
     }
   }
-  SetLastAdded(last_added);
 
-  ASSERT(LastAdded() != -1);
+  UNREACHABLE();
 }
 
 

diff --git a/src/objects.h b/src/objects.h
index 28eb4f3096412d59fc276b75327199793a32072a..412820bcbca0afac932c5bf7ad81521cc58724ac 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -2475,6 +2475,7 @@ class DescriptorArray: public FixedArray {
   }
 
   inline int number_of_entries() { return number_of_descriptors(); }
+  inline int NextEnumerationIndex() { return number_of_descriptors() + 1; }
 
   int LastAdded() {
     ASSERT(!IsEmpty());
@@ -2487,19 +2488,18 @@ class DescriptorArray: public FixedArray {
     }
   }
 
-  int NextEnumerationIndex() {
-    if (number_of_descriptors() == 0) {
-      return PropertyDetails::kInitialIndex;
-    }
-    return GetDetails(LastAdded()).index() + 1;
-  }
-
   // Set index of the last added descriptor and flush any enum cache.
   void SetLastAdded(int index) {
     ASSERT(!IsEmpty() || index > 0);
     set(kLastAddedIndex, Smi::FromInt(index));
   }
 
+  int NumberOfSetDescriptors() {
+    ASSERT(!IsEmpty());
+    if (LastAdded() == kNoneAdded) return 0;
+    return GetDetails(LastAdded()).index();
+  }
+
   bool HasEnumCache() {
     return !IsEmpty() && !get(kLastAddedIndex)->IsSmi();
   }
@@ -2546,6 +2546,11 @@ class DescriptorArray: public FixedArray {
   inline void Set(int descriptor_number,
                   Descriptor* desc,
                   const WhitenessWitness&);
+  // Append automatically sets the enumeration index. This should only be used
+  // to add descriptors in bulk at the end, followed by sorting the descriptor
+  // array.
+  inline void Append(Descriptor* desc,
+                     const WhitenessWitness&);
 
   // Transfer a complete descriptor from the src descriptor array to the dst
   // one, dropping map transitions in CALLBACKS.
@@ -2613,6 +2618,9 @@ class DescriptorArray: public FixedArray {
   // Constant for denoting key was not found.
   static const int kNotFound = -1;
 
+  // Constant for denoting that the LastAdded field was not yet set.
+  static const int kNoneAdded = -1;
+
   static const int kBackPointerStorageIndex = 0;
   static const int kLastAddedIndex = 1;
   static const int kTransitionsIndex = 2;

diff --git a/src/property.h b/src/property.h
index f783dc2e9a03dd6967fa7bdca4759df88cfb1066..4ff0b2b6658d796d7749f617796ef92f4debf622 100644 (file)
--- a/src/property.h
+++ b/src/property.h
@@ -106,7 +106,7 @@ class FieldDescriptor: public Descriptor {
   FieldDescriptor(String* key,
                   int field_index,
                   PropertyAttributes attributes,
-                  int index)
+                  int index = 0)
       : Descriptor(key, Smi::FromInt(field_index), attributes, FIELD, index) {}
 };
 
@@ -126,7 +126,7 @@ class CallbacksDescriptor:  public Descriptor {
   CallbacksDescriptor(String* key,
                       Object* foreign,
                       PropertyAttributes attributes,
-                      int index)
+                      int index = 0)
       : Descriptor(key, foreign, attributes, CALLBACKS, index) {}
 };