Remove the descriptor stream abstractions.

The abstractions have led to bugs because it looks like descriptor
streams are GC safe but they are not.

I have moved the descriptor stream helper functions to descriptor
arrays and I find most of the code just as readable now as it was
before.
Review URL: http://codereview.chromium.org/149458

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

src/bootstrapper.cc

@@ -1373,43 +1373,35 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
   if (from->HasFastProperties()) {
     Handle<DescriptorArray> descs =
         Handle<DescriptorArray>(from->map()->instance_descriptors());
-    int offset = 0;
-    while (true) {
-      // Iterating through the descriptors is not gc safe so we have to
-      // store the value in a handle and create a new stream for each entry.
-      DescriptorReader stream(*descs, offset);
-      if (stream.eos()) break;
-      // We have to read out the next offset before we do anything that may
-      // cause a gc, since the DescriptorReader is not gc safe.
-      offset = stream.next_position();
-      PropertyDetails details = stream.GetDetails();
+    for (int i = 0; i < descs->number_of_descriptors(); i++) {
+      PropertyDetails details = PropertyDetails(descs->GetDetails(i));
       switch (details.type()) {
         case FIELD: {
           HandleScope inner;
-          Handle<String> key = Handle<String>(stream.GetKey());
-          int index = stream.GetFieldIndex();
+          Handle<String> key = Handle<String>(descs->GetKey(i));
+          int index = descs->GetFieldIndex(i);
           Handle<Object> value = Handle<Object>(from->FastPropertyAt(index));
           SetProperty(to, key, value, details.attributes());
           break;
         }
         case CONSTANT_FUNCTION: {
           HandleScope inner;
-          Handle<String> key = Handle<String>(stream.GetKey());
+          Handle<String> key = Handle<String>(descs->GetKey(i));
           Handle<JSFunction> fun =
-              Handle<JSFunction>(stream.GetConstantFunction());
+              Handle<JSFunction>(descs->GetConstantFunction(i));
           SetProperty(to, key, fun, details.attributes());
           break;
         }
         case CALLBACKS: {
           LookupResult result;
-          to->LocalLookup(stream.GetKey(), &result);
+          to->LocalLookup(descs->GetKey(i), &result);
           // If the property is already there we skip it
           if (result.IsValid()) continue;
           HandleScope inner;
           Handle<DescriptorArray> inst_descs =
               Handle<DescriptorArray>(to->map()->instance_descriptors());
-          Handle<String> key = Handle<String>(stream.GetKey());
-          Handle<Object> entry = Handle<Object>(stream.GetCallbacksObject());
+          Handle<String> key = Handle<String>(descs->GetKey(i));
+          Handle<Object> entry = Handle<Object>(descs->GetCallbacksObject(i));
           inst_descs = Factory::CopyAppendProxyDescriptor(inst_descs,
                                                           key,
                                                           entry,

src/factory.cc

@@ -570,13 +570,9 @@ Handle<DescriptorArray> Factory::CopyAppendCallbackDescriptors(
   int descriptor_count = 0;
 
   // Copy the descriptors from the array.
-  {
-    DescriptorWriter w(*result);
-    for (DescriptorReader r(*array); !r.eos(); r.advance()) {
-      if (!r.IsNullDescriptor()) {
-        w.WriteFrom(&r);
-      }
-      descriptor_count++;
+  for (int i = 0; i < array->number_of_descriptors(); i++) {
+    if (array->GetType(i) != NULL_DESCRIPTOR) {
+      result->CopyFrom(descriptor_count++, *array, i);
     }
   }
 
@@ -596,9 +592,6 @@ Handle<DescriptorArray> Factory::CopyAppendCallbackDescriptors(
     if (result->LinearSearch(*key, descriptor_count) ==
         DescriptorArray::kNotFound) {
       CallbacksDescriptor desc(*key, *entry, entry->property_attributes());
-      // We do not use a DescriptorWriter because SymbolFromString can
-      // allocate. A DescriptorWriter holds a raw pointer and is
-      // therefore not GC safe.
       result->Set(descriptor_count, &desc);
       descriptor_count++;
     } else {
@@ -609,13 +602,11 @@ Handle<DescriptorArray> Factory::CopyAppendCallbackDescriptors(
   // If duplicates were detected, allocate a result of the right size
   // and transfer the elements.
   if (duplicates > 0) {
+    int number_of_descriptors = result->number_of_descriptors() - duplicates;
     Handle<DescriptorArray> new_result =
-        NewDescriptorArray(result->number_of_descriptors() - duplicates);
-    DescriptorWriter w(*new_result);
-    DescriptorReader r(*result);
-    while (!w.eos()) {
-      w.WriteFrom(&r);
-      r.advance();
+        NewDescriptorArray(number_of_descriptors);
+    for (int i = 0; i < number_of_descriptors; i++) {
+      new_result->CopyFrom(i, *result, i);
     }
     result = new_result;
   }

src/handles.cc

@@ -289,10 +289,11 @@ Handle<Object> GetHiddenProperties(Handle<JSObject> obj,
     // hidden symbols hash code is zero (and no other string has hash
     // code zero) it will always occupy the first entry if present.
     DescriptorArray* descriptors = obj->map()->instance_descriptors();
-    DescriptorReader r(descriptors, 0);  // Explicitly position reader at zero.
-    if (!r.eos() && (r.GetKey() == *key) && r.IsProperty()) {
-      ASSERT(r.type() == FIELD);
-      return Handle<Object>(obj->FastPropertyAt(r.GetFieldIndex()));
+    if ((descriptors->number_of_descriptors() > 0) &&
+        (descriptors->GetKey(0) == *key) &&
+        descriptors->IsProperty(0)) {
+      ASSERT(descriptors->GetType(0) == FIELD);
+      return Handle<Object>(obj->FastPropertyAt(descriptors->GetFieldIndex(0)));
     }
   }
 
@@ -588,12 +589,13 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object) {
     int num_enum = object->NumberOfEnumProperties();
     Handle<FixedArray> storage = Factory::NewFixedArray(num_enum);
     Handle<FixedArray> sort_array = Factory::NewFixedArray(num_enum);
-    for (DescriptorReader r(object->map()->instance_descriptors());
-         !r.eos();
-         r.advance()) {
-      if (r.IsProperty() && !r.IsDontEnum()) {
-        (*storage)->set(index, r.GetKey());
-        (*sort_array)->set(index, Smi::FromInt(r.GetDetails().index()));
+    Handle<DescriptorArray> descs =
+        Handle<DescriptorArray>(object->map()->instance_descriptors());
+    for (int i = 0; i < descs->number_of_descriptors(); i++) {
+      if (descs->IsProperty(i) && !descs->IsDontEnum(i)) {
+        (*storage)->set(index, descs->GetKey(i));
+        PropertyDetails details(descs->GetDetails(i));
+        (*sort_array)->set(index, Smi::FromInt(details.index()));
         index++;
       }
     }

src/objects-debug.cc

@@ -271,29 +271,38 @@ void ByteArray::ByteArrayVerify() {
 
 void JSObject::PrintProperties() {
   if (HasFastProperties()) {
-    for (DescriptorReader r(map()->instance_descriptors());
-         !r.eos();
-         r.advance()) {
+    DescriptorArray* descs = map()->instance_descriptors();
+    for (int i = 0; i < descs->number_of_descriptors(); i++) {
       PrintF("   ");
-      r.GetKey()->StringPrint();
+      descs->GetKey(i)->StringPrint();
       PrintF(": ");
-      if (r.type() == FIELD) {
-        FastPropertyAt(r.GetFieldIndex())->ShortPrint();
-        PrintF(" (field at offset %d)\n", r.GetFieldIndex());
-      } else if (r.type() ==  CONSTANT_FUNCTION) {
-        r.GetConstantFunction()->ShortPrint();
+      switch (descs->GetType(i)) {
+        case FIELD: {
+          int index = descs->GetFieldIndex(i);
+          FastPropertyAt(index)->ShortPrint();
+          PrintF(" (field at offset %d)\n", index);
+          break;
+        }
+        case CONSTANT_FUNCTION:
+          descs->GetConstantFunction(i)->ShortPrint();
           PrintF(" (constant function)\n");
-      } else if (r.type() == CALLBACKS) {
-        r.GetCallbacksObject()->ShortPrint();
+          break;
+        case CALLBACKS:
+          descs->GetCallbacksObject(i)->ShortPrint();
           PrintF(" (callback)\n");
-      } else if (r.type() == MAP_TRANSITION) {
+          break;
+        case MAP_TRANSITION:
           PrintF(" (map transition)\n");
-      } else if (r.type() == CONSTANT_TRANSITION) {
+          break;
+        case CONSTANT_TRANSITION:
           PrintF(" (constant transition)\n");
-      } else if (r.type() == NULL_DESCRIPTOR) {
+          break;
+        case NULL_DESCRIPTOR:
           PrintF(" (null descriptor)\n");
-      } else {
+          break;
+        default:
           UNREACHABLE();
+          break;
       }
     }
   } else {
@@ -1062,11 +1071,10 @@ void JSObject::SpillInformation::Print() {
 
 void DescriptorArray::PrintDescriptors() {
   PrintF("Descriptor array  %d\n", number_of_descriptors());
-  int number = 0;
-  for (DescriptorReader r(this); !r.eos(); r.advance()) {
+  for (int i = 0; i < number_of_descriptors(); i++) {
+    PrintF(" %d: ", i);
     Descriptor desc;
-    r.Get(&desc);
-    PrintF(" %d: ", number++);
+    Get(i, &desc);
     desc.Print();
   }
   PrintF("\n");
@@ -1076,14 +1084,14 @@ void DescriptorArray::PrintDescriptors() {
 bool DescriptorArray::IsSortedNoDuplicates() {
   String* current_key = NULL;
   uint32_t current = 0;
-  for (DescriptorReader r(this); !r.eos(); r.advance()) {
-    String* key = r.GetKey();
+  for (int i = 0; i < number_of_descriptors(); i++) {
+    String* key = GetKey(i);
     if (key == current_key) {
       PrintDescriptors();
       return false;
     }
     current_key = key;
-    uint32_t hash = r.GetKey()->Hash();
+    uint32_t hash = GetKey(i)->Hash();
     if (hash < current) {
       PrintDescriptors();
       return false;

src/objects-inl.h

@@ -1350,6 +1350,56 @@ Smi* DescriptorArray::GetDetails(int descriptor_number) {
 }
 
 
+PropertyType DescriptorArray::GetType(int descriptor_number) {
+  ASSERT(descriptor_number < number_of_descriptors());
+  return PropertyDetails(GetDetails(descriptor_number)).type();
+}
+
+
+int DescriptorArray::GetFieldIndex(int descriptor_number) {
+  return Descriptor::IndexFromValue(GetValue(descriptor_number));
+}
+
+
+JSFunction* DescriptorArray::GetConstantFunction(int descriptor_number) {
+  return JSFunction::cast(GetValue(descriptor_number));
+}
+
+
+Object* DescriptorArray::GetCallbacksObject(int descriptor_number) {
+  ASSERT(GetType(descriptor_number) == CALLBACKS);
+  return GetValue(descriptor_number);
+}
+
+
+AccessorDescriptor* DescriptorArray::GetCallbacks(int descriptor_number) {
+  ASSERT(GetType(descriptor_number) == CALLBACKS);
+  Proxy* p = Proxy::cast(GetCallbacksObject(descriptor_number));
+  return reinterpret_cast<AccessorDescriptor*>(p->proxy());
+}
+
+
+bool DescriptorArray::IsProperty(int descriptor_number) {
+  return GetType(descriptor_number) < FIRST_PHANTOM_PROPERTY_TYPE;
+}
+
+
+bool DescriptorArray::IsTransition(int descriptor_number) {
+  PropertyType t = GetType(descriptor_number);
+  return t == MAP_TRANSITION || t == CONSTANT_TRANSITION;
+}
+
+
+bool DescriptorArray::IsNullDescriptor(int descriptor_number) {
+  return GetType(descriptor_number) == NULL_DESCRIPTOR;
+}
+
+
+bool DescriptorArray::IsDontEnum(int descriptor_number) {
+  return PropertyDetails(GetDetails(descriptor_number)).IsDontEnum();
+}
+
+
 void DescriptorArray::Get(int descriptor_number, Descriptor* desc) {
   desc->Init(GetKey(descriptor_number),
              GetValue(descriptor_number),
@@ -1373,6 +1423,13 @@ void DescriptorArray::Set(int descriptor_number, Descriptor* desc) {
 }
 
 
+void DescriptorArray::CopyFrom(int index, DescriptorArray* src, int src_index) {
+  Descriptor desc;
+  src->Get(src_index, &desc);
+  Set(index, &desc);
+}
+
+
 void DescriptorArray::Swap(int first, int second) {
   fast_swap(this, ToKeyIndex(first), ToKeyIndex(second));
   FixedArray* content_array = GetContentArray();

src/objects.h

@@ -1850,15 +1850,28 @@ class DescriptorArray: public FixedArray {
   // using the supplied storage for the small "bridge".
   void SetEnumCache(FixedArray* bridge_storage, FixedArray* new_cache);
 
-  // Accessors for fetching instance descriptor at descriptor number..
+  // Accessors for fetching instance descriptor at descriptor number.
   inline String* GetKey(int descriptor_number);
   inline Object* GetValue(int descriptor_number);
   inline Smi* GetDetails(int descriptor_number);
+  inline PropertyType GetType(int descriptor_number);
+  inline int GetFieldIndex(int descriptor_number);
+  inline JSFunction* GetConstantFunction(int descriptor_number);
+  inline Object* GetCallbacksObject(int descriptor_number);
+  inline AccessorDescriptor* GetCallbacks(int descriptor_number);
+  inline bool IsProperty(int descriptor_number);
+  inline bool IsTransition(int descriptor_number);
+  inline bool IsNullDescriptor(int descriptor_number);
+  inline bool IsDontEnum(int descriptor_number);
 
   // Accessor for complete descriptor.
   inline void Get(int descriptor_number, Descriptor* desc);
   inline void Set(int descriptor_number, Descriptor* desc);
 
+  // Transfer complete descriptor from another descriptor array to
+  // this one.
+  inline void CopyFrom(int index, DescriptorArray* src, int src_index);
+
   // Copy the descriptor array, insert a new descriptor and optionally
   // remove map transitions.  If the descriptor is already present, it is
   // replaced.  If a replaced descriptor is a real property (not a transition

src/property.cc

@@ -31,20 +31,6 @@ namespace v8 {
 namespace internal {
 
 
-void DescriptorWriter::Write(Descriptor* desc) {
-  ASSERT(desc->key_->IsSymbol());
-  descriptors_->Set(pos_, desc);
-  advance();
-}
-
-
-void DescriptorWriter::WriteFrom(DescriptorReader* reader) {
-  Descriptor desc;
-  reader->Get(&desc);
-  Write(&desc);
-}
-
-
 #ifdef DEBUG
 void LookupResult::Print() {
   if (!IsValid()) {

src/property.h

@@ -95,8 +95,6 @@ class Descriptor BASE_EMBEDDED {
         value_(value),
         details_(attributes, type, index) { }
 
-  friend class DescriptorWriter;
-  friend class DescriptorReader;
   friend class DescriptorArray;
 };
 
@@ -324,92 +322,6 @@ class LookupResult BASE_EMBEDDED {
 };
 
 
-// The DescriptorStream is an abstraction for iterating over a map's
-// instance descriptors.
-class DescriptorStream BASE_EMBEDDED {
- public:
-  explicit DescriptorStream(DescriptorArray* descriptors, int pos) {
-    descriptors_ = descriptors;
-    pos_ = pos;
-    limit_ = descriptors_->number_of_descriptors();
-  }
-
-  // Tells whether we have reached the end of the steam.
-  bool eos() { return pos_ >= limit_; }
-
-  int next_position() { return pos_ + 1; }
-  void advance() { pos_ = next_position(); }
-
- protected:
-  DescriptorArray* descriptors_;
-  int pos_;   // Current position.
-  int limit_;  // Limit for position.
-};
-
-
-class DescriptorReader: public DescriptorStream {
- public:
-  explicit DescriptorReader(DescriptorArray* descriptors, int pos = 0)
-      : DescriptorStream(descriptors, pos) {}
-
-  String* GetKey() { return descriptors_->GetKey(pos_); }
-  Object* GetValue() { return descriptors_->GetValue(pos_); }
-  PropertyDetails GetDetails() {
-    return PropertyDetails(descriptors_->GetDetails(pos_));
-  }
-
-  int GetFieldIndex() { return Descriptor::IndexFromValue(GetValue()); }
-
-  bool IsDontEnum() { return GetDetails().IsDontEnum(); }
-
-  PropertyType type() { return GetDetails().type(); }
-
-  // Tells whether the type is a transition.
-  bool IsTransition() {
-    PropertyType t = type();
-    ASSERT(t != INTERCEPTOR);
-    return t == MAP_TRANSITION || t == CONSTANT_TRANSITION;
-  }
-
-  bool IsNullDescriptor() {
-    return type() == NULL_DESCRIPTOR;
-  }
-
-  bool IsProperty() {
-    return type() < FIRST_PHANTOM_PROPERTY_TYPE;
-  }
-
-  JSFunction* GetConstantFunction() { return JSFunction::cast(GetValue()); }
-
-  AccessorDescriptor* GetCallbacks() {
-    ASSERT(type() == CALLBACKS);
-    Proxy* p = Proxy::cast(GetCallbacksObject());
-    return reinterpret_cast<AccessorDescriptor*>(p->proxy());
-  }
-
-  Object* GetCallbacksObject() {
-    ASSERT(type() == CALLBACKS);
-    return GetValue();
-  }
-
-  bool Equals(String* name) { return name->Equals(GetKey()); }
-
-  void Get(Descriptor* desc) {
-    descriptors_->Get(pos_, desc);
-  }
-};
-
-class DescriptorWriter: public DescriptorStream {
- public:
-  explicit DescriptorWriter(DescriptorArray* descriptors)
-      : DescriptorStream(descriptors, 0) {}
-
-  // Append a descriptor to this stream.
-  void Write(Descriptor* desc);
-  // Read a descriptor from the reader and append it to this stream.
-  void WriteFrom(DescriptorReader* reader);
-};
-
 } }  // namespace v8::internal
 
 #endif  // V8_PROPERTY_H_

src/string-stream.cc

@@ -343,10 +343,11 @@ void StringStream::PrintUsingMap(JSObject* js_object) {
     Add("<Invalid map>\n");
     return;
   }
-  for (DescriptorReader r(map->instance_descriptors()); !r.eos(); r.advance()) {
-    switch (r.type()) {
+  DescriptorArray* descs = map->instance_descriptors();
+  for (int i = 0; i < descs->number_of_descriptors(); i++) {
+    switch (descs->GetType(i)) {
       case FIELD: {
-        Object* key = r.GetKey();
+        Object* key = descs->GetKey(i);
         if (key->IsString() || key->IsNumber()) {
           int len = 3;
           if (key->IsString()) {
@@ -360,7 +361,7 @@ void StringStream::PrintUsingMap(JSObject* js_object) {
             key->ShortPrint();
           }
           Add(": ");
-          Object* value = js_object->FastPropertyAt(r.GetFieldIndex());
+          Object* value = js_object->FastPropertyAt(descs->GetFieldIndex(i));
           Add("%o\n", value);
         }
       }