TransitionArray now uses <is_data_property, name, attributes> tuple as a key,...

TransitionArray now uses <is_data_property, name, attributes> tuple as a key, which allows to have several entries for the same property name.

R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/661133002

Cr-Commit-Position: refs/heads/master@{#25136}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25136 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/globals.h

@@ -354,6 +354,7 @@ template <typename Config, class Allocator = FreeStoreAllocationPolicy>
 class String;
 class Name;
 class Struct;
+class Symbol;
 class Variable;
 class RelocInfo;
 class Deserializer;

src/hydrogen.cc

@@ -6114,7 +6114,7 @@ bool HOptimizedGraphBuilder::PropertyAccessInfo::CanAccessMonomorphic() {
 
   if (IsAccessor()) return true;
   Handle<Map> map = this->map();
-  map->LookupTransition(NULL, *name_, &lookup_);
+  map->LookupTransition(NULL, *name_, NONE, &lookup_);
   if (lookup_.IsTransitionToField() && map->unused_property_fields() > 0) {
     // Construct the object field access.
     int descriptor = transition()->LastAdded();

src/objects-debug.cc

@@ -1164,15 +1164,13 @@ bool DescriptorArray::IsSortedNoDuplicates(int valid_entries) {
   for (int i = 0; i < number_of_descriptors(); i++) {
     Name* key = GetSortedKey(i);
     if (key == current_key) {
-      OFStream os(stdout);
-      PrintDescriptors(os);
+      Print();
       return false;
     }
     current_key = key;
     uint32_t hash = GetSortedKey(i)->Hash();
     if (hash < current) {
-      OFStream os(stdout);
-      PrintDescriptors(os);
+      Print();
       return false;
     }
     current = hash;
@@ -1183,23 +1181,36 @@ bool DescriptorArray::IsSortedNoDuplicates(int valid_entries) {
 
 bool TransitionArray::IsSortedNoDuplicates(int valid_entries) {
   DCHECK(valid_entries == -1);
-  Name* current_key = NULL;
-  uint32_t current = 0;
+  Name* prev_key = NULL;
+  bool prev_is_data_property = false;
+  PropertyAttributes prev_attributes = NONE;
+  uint32_t prev_hash = 0;
   for (int i = 0; i < number_of_transitions(); i++) {
     Name* key = GetSortedKey(i);
-    if (key == current_key) {
-      OFStream os(stdout);
-      PrintTransitions(os);
-      return false;
+    uint32_t hash = key->Hash();
+    bool is_data_property = false;
+    PropertyAttributes attributes = NONE;
+    if (!IsSpecialTransition(key)) {
+      Map* target = GetTarget(i);
+      PropertyDetails details = GetTargetDetails(key, target);
+      is_data_property = details.type() == FIELD || details.type() == CONSTANT;
+      attributes = details.attributes();
+    } else {
+      // Duplicate entries are not allowed for non-property transitions.
+      CHECK_NE(prev_key, key);
     }
-    current_key = key;
-    uint32_t hash = GetSortedKey(i)->Hash();
-    if (hash < current) {
-      OFStream os(stdout);
-      PrintTransitions(os);
+
+    int cmp =
+        CompareKeys(prev_key, prev_hash, prev_is_data_property, prev_attributes,
+                    key, hash, is_data_property, attributes);
+    if (cmp >= 0) {
+      Print();
       return false;
     }
-    current = hash;
+    prev_key = key;
+    prev_hash = hash;
+    prev_attributes = attributes;
+    prev_is_data_property = is_data_property;
   }
   return true;
 }

src/objects-inl.h

@@ -1884,11 +1884,10 @@ Handle<Map> Map::FindTransitionToField(Handle<Map> map, Handle<Name> key) {
   DisallowHeapAllocation no_allocation;
   if (!map->HasTransitionArray()) return Handle<Map>::null();
   TransitionArray* transitions = map->transitions();
-  int transition = transitions->Search(*key);
+  int transition = transitions->Search(FIELD, *key, NONE);
   if (transition == TransitionArray::kNotFound) return Handle<Map>::null();
-  PropertyDetails target_details = transitions->GetTargetDetails(transition);
-  if (target_details.type() != FIELD) return Handle<Map>::null();
-  if (target_details.attributes() != NONE) return Handle<Map>::null();
+  DCHECK_EQ(FIELD, transitions->GetTargetDetails(transition).type());
+  DCHECK_EQ(NONE, transitions->GetTargetDetails(transition).attributes());
   return Handle<Map>(transitions->GetTarget(transition));
 }
 
@@ -2943,10 +2942,10 @@ void Map::LookupDescriptor(JSObject* holder,
 }
 
 
-void Map::LookupTransition(JSObject* holder,
-                           Name* name,
+void Map::LookupTransition(JSObject* holder, Name* name,
+                           PropertyAttributes attributes,
                            LookupResult* result) {
-  int transition_index = this->SearchTransition(name);
+  int transition_index = this->SearchTransition(FIELD, name, attributes);
   if (transition_index == TransitionArray::kNotFound) return result->NotFound();
   result->TransitionResult(holder, this->GetTransition(transition_index));
 }
@@ -5221,7 +5220,8 @@ bool Map::HasTransitionArray() const {
 
 
 Map* Map::elements_transition_map() {
-  int index = transitions()->Search(GetHeap()->elements_transition_symbol());
+  int index =
+      transitions()->SearchSpecial(GetHeap()->elements_transition_symbol());
   return transitions()->GetTarget(index);
 }
 
@@ -5238,8 +5238,19 @@ Map* Map::GetTransition(int transition_index) {
 }
 
 
-int Map::SearchTransition(Name* name) {
-  if (HasTransitionArray()) return transitions()->Search(name);
+int Map::SearchSpecialTransition(Symbol* name) {
+  if (HasTransitionArray()) {
+    return transitions()->SearchSpecial(name);
+  }
+  return TransitionArray::kNotFound;
+}
+
+
+int Map::SearchTransition(PropertyType type, Name* name,
+                          PropertyAttributes attributes) {
+  if (HasTransitionArray()) {
+    return transitions()->Search(type, name, attributes);
+  }
   return TransitionArray::kNotFound;
 }
 
@@ -5292,9 +5303,17 @@ void Map::set_transitions(TransitionArray* transition_array,
       Map* target = transitions()->GetTarget(i);
       if (target->instance_descriptors() == instance_descriptors()) {
         Name* key = transitions()->GetKey(i);
-        int new_target_index = transition_array->Search(key);
-        DCHECK(new_target_index != TransitionArray::kNotFound);
-        DCHECK(transition_array->GetTarget(new_target_index) == target);
+        int new_target_index;
+        if (TransitionArray::IsSpecialTransition(key)) {
+          new_target_index = transition_array->SearchSpecial(Symbol::cast(key));
+        } else {
+          PropertyDetails details =
+              TransitionArray::GetTargetDetails(key, target);
+          new_target_index = transition_array->Search(details.type(), key,
+                                                      details.attributes());
+        }
+        DCHECK_NE(TransitionArray::kNotFound, new_target_index);
+        DCHECK_EQ(target, transition_array->GetTarget(new_target_index));
       }
     }
 #endif

src/objects-printer.cc

@@ -1090,18 +1090,19 @@ void TransitionArray::PrintTransitions(std::ostream& os,
   }
   for (int i = 0; i < number_of_transitions(); i++) {
     Name* key = GetKey(i);
+    Map* target = GetTarget(i);
     os << "   ";
     key->NamePrint(os);
     os << ": ";
     if (key == GetHeap()->frozen_symbol()) {
       os << " (transition to frozen)";
     } else if (key == GetHeap()->elements_transition_symbol()) {
-      os << " (transition to "
-         << ElementsKindToString(GetTarget(i)->elements_kind()) << ")";
+      os << " (transition to " << ElementsKindToString(target->elements_kind())
+         << ")";
     } else if (key == GetHeap()->observed_symbol()) {
       os << " (transition to Object.observe)";
     } else {
-      PropertyDetails details = GetTargetDetails(i);
+      PropertyDetails details = GetTargetDetails(key, target);
       switch (details.type()) {
         case FIELD: {
           os << " (transition to field)";
@@ -1120,7 +1121,7 @@ void TransitionArray::PrintTransitions(std::ostream& os,
       }
       os << ", attrs: " << details.attributes();
     }
-    os << " -> " << Brief(GetTarget(i)) << "\n";
+    os << " -> " << Brief(target) << "\n";
   }
 }
 

src/objects.h

@@ -280,8 +280,9 @@ enum DebugExtraICState {
 // either extends the current map with a new property, or it modifies the
 // property that was added last to the current map.
 enum SimpleTransitionFlag {
-  SIMPLE_TRANSITION,
-  FULL_TRANSITION
+  SIMPLE_PROPERTY_TRANSITION,
+  PROPERTY_TRANSITION,
+  SPECIAL_TRANSITION
 };
 
 
@@ -5820,7 +5821,9 @@ class Map: public HeapObject {
   inline Map* elements_transition_map();
 
   inline Map* GetTransition(int transition_index);
-  inline int SearchTransition(Name* name);
+  inline int SearchSpecialTransition(Symbol* name);
+  inline int SearchTransition(PropertyType type, Name* name,
+                              PropertyAttributes attributes);
   inline FixedArrayBase* GetInitialElements();
 
   DECL_ACCESSORS(transitions, TransitionArray)
@@ -5958,8 +5961,8 @@ class Map: public HeapObject {
                                Name* name,
                                LookupResult* result);
 
-  inline void LookupTransition(JSObject* holder,
-                               Name* name,
+  inline void LookupTransition(JSObject* holder, Name* name,
+                               PropertyAttributes attributes,
                                LookupResult* result);
 
   inline PropertyDetails GetLastDescriptorDetails();
@@ -6329,12 +6332,11 @@ class Map: public HeapObject {
   static Handle<Map> CopyAddDescriptor(Handle<Map> map,
                                        Descriptor* descriptor,
                                        TransitionFlag flag);
-  static Handle<Map> CopyReplaceDescriptors(
-      Handle<Map> map,
-      Handle<DescriptorArray> descriptors,
-      TransitionFlag flag,
-      MaybeHandle<Name> maybe_name,
-      SimpleTransitionFlag simple_flag = FULL_TRANSITION);
+  static Handle<Map> CopyReplaceDescriptors(Handle<Map> map,
+                                            Handle<DescriptorArray> descriptors,
+                                            TransitionFlag flag,
+                                            MaybeHandle<Name> maybe_name,
+                                            SimpleTransitionFlag simple_flag);
   static Handle<Map> CopyReplaceDescriptor(Handle<Map> map,
                                            Handle<DescriptorArray> descriptors,
                                            Descriptor* descriptor,
@@ -6362,7 +6364,9 @@ class Map: public HeapObject {
   void ZapTransitions();
 
   void DeprecateTransitionTree();
-  void DeprecateTarget(Name* key, DescriptorArray* new_descriptors);
+  void DeprecateTarget(PropertyType type, Name* key,
+                       PropertyAttributes attributes,
+                       DescriptorArray* new_descriptors);
 
   Map* FindLastMatchMap(int verbatim, int length, DescriptorArray* descriptors);
 

src/transitions-inl.h

@@ -34,7 +34,7 @@ TransitionArray* TransitionArray::cast(Object* object) {
 
 
 bool TransitionArray::HasElementsTransition() {
-  return Search(GetHeap()->elements_transition_symbol()) != kNotFound;
+  return SearchSpecial(GetHeap()->elements_transition_symbol()) != kNotFound;
 }
 
 
@@ -140,7 +140,7 @@ Object* TransitionArray::GetTargetValue(int transition_number) {
 }
 
 
-int TransitionArray::Search(Name* name, int* out_insertion_index) {
+int TransitionArray::SearchName(Name* name, int* out_insertion_index) {
   if (IsSimpleTransition()) {
     Name* key = GetKey(kSimpleTransitionIndex);
     if (key->Equals(name)) return kSimpleTransitionIndex;
@@ -153,6 +153,71 @@ int TransitionArray::Search(Name* name, int* out_insertion_index) {
 }
 
 
+#ifdef DEBUG
+bool TransitionArray::IsSpecialTransition(Name* name) {
+  if (!name->IsSymbol()) return false;
+  Heap* heap = name->GetHeap();
+  return name == heap->frozen_symbol() ||
+         name == heap->elements_transition_symbol() ||
+         name == heap->observed_symbol();
+}
+#endif
+
+
+int TransitionArray::CompareKeys(Name* key1, uint32_t hash1,
+                                 bool is_data_property1,
+                                 PropertyAttributes attributes1, Name* key2,
+                                 uint32_t hash2, bool is_data_property2,
+                                 PropertyAttributes attributes2) {
+  int cmp = CompareNames(key1, hash1, key2, hash2);
+  if (cmp != 0) return cmp;
+
+  return CompareDetails(is_data_property1, attributes1, is_data_property2,
+                        attributes2);
+}
+
+
+int TransitionArray::CompareNames(Name* key1, uint32_t hash1, Name* key2,
+                                  uint32_t hash2) {
+  if (key1 != key2) {
+    // In case of hash collisions key1 is always "less" than key2.
+    return hash1 <= hash2 ? -1 : 1;
+  }
+
+  return 0;
+}
+
+
+int TransitionArray::CompareDetails(bool is_data_property1,
+                                    PropertyAttributes attributes1,
+                                    bool is_data_property2,
+                                    PropertyAttributes attributes2) {
+  if (is_data_property1 != is_data_property2) {
+    return static_cast<int>(is_data_property1) <
+                   static_cast<int>(is_data_property2)
+               ? -1
+               : 1;
+  }
+
+  if (attributes1 != attributes2) {
+    return static_cast<int>(attributes1) < static_cast<int>(attributes2) ? -1
+                                                                         : 1;
+  }
+
+  return 0;
+}
+
+
+PropertyDetails TransitionArray::GetTargetDetails(Name* name, Map* target) {
+  DCHECK(!IsSpecialTransition(name));
+  int descriptor = target->LastAdded();
+  DescriptorArray* descriptors = target->instance_descriptors();
+  // Transitions are allowed only for the last added property.
+  DCHECK(descriptors->GetKey(descriptor)->Equals(name));
+  return descriptors->GetDetails(descriptor);
+}
+
+
 void TransitionArray::NoIncrementalWriteBarrierSet(int transition_number,
                                                    Name* key,
                                                    Map* target) {

src/transitions.cc

@@ -48,7 +48,7 @@ Handle<TransitionArray> TransitionArray::NewWith(Handle<Map> map,
   Handle<TransitionArray> result;
   Isolate* isolate = name->GetIsolate();
 
-  if (flag == SIMPLE_TRANSITION) {
+  if (flag == SIMPLE_PROPERTY_TRANSITION) {
     result = AllocateSimple(isolate, target);
   } else {
     result = Allocate(isolate, 1);
@@ -94,9 +94,19 @@ Handle<TransitionArray> TransitionArray::Insert(Handle<Map> map,
   int number_of_transitions = map->transitions()->number_of_transitions();
   int new_nof = number_of_transitions;
 
-  int insertion_index = kNotFound;
-  int index = map->transitions()->Search(*name, &insertion_index);
+  bool is_special_transition = flag == SPECIAL_TRANSITION;
+  DCHECK_EQ(is_special_transition, IsSpecialTransition(*name));
+  PropertyDetails details = is_special_transition
+                                ? PropertyDetails(NONE, NORMAL, 0)
+                                : GetTargetDetails(*name, *target);
 
+  int insertion_index = kNotFound;
+  int index =
+      is_special_transition
+          ? map->transitions()->SearchSpecial(Symbol::cast(*name),
+                                              &insertion_index)
+          : map->transitions()->Search(details.type(), *name,
+                                       details.attributes(), &insertion_index);
   if (index == kNotFound) {
     ++new_nof;
   } else {
@@ -118,12 +128,12 @@ Handle<TransitionArray> TransitionArray::Insert(Handle<Map> map,
     array->SetNumberOfTransitions(new_nof);
     for (index = number_of_transitions; index > insertion_index; --index) {
       Name* key = array->GetKey(index - 1);
-      DCHECK(key->Hash() > name->Hash());
       array->SetKey(index, key);
       array->SetTarget(index, array->GetTarget(index - 1));
     }
     array->SetKey(index, *name);
     array->SetTarget(index, *target);
+    SLOW_DCHECK(array->IsSortedNoDuplicates());
     return handle(array);
   }
 
@@ -144,7 +154,11 @@ Handle<TransitionArray> TransitionArray::Insert(Handle<Map> map,
     new_nof = number_of_transitions;
 
     insertion_index = kNotFound;
-    index = array->Search(*name, &insertion_index);
+    index = is_special_transition ? map->transitions()->SearchSpecial(
+                                        Symbol::cast(*name), &insertion_index)
+                                  : map->transitions()->Search(
+                                        details.type(), *name,
+                                        details.attributes(), &insertion_index);
     if (index == kNotFound) {
       ++new_nof;
     } else {
@@ -170,8 +184,46 @@ Handle<TransitionArray> TransitionArray::Insert(Handle<Map> map,
   }
 
   result->set_back_pointer_storage(array->back_pointer_storage());
+  SLOW_DCHECK(result->IsSortedNoDuplicates());
   return result;
 }
 
 
+int TransitionArray::SearchDetails(int transition, PropertyType type,
+                                   PropertyAttributes attributes,
+                                   int* out_insertion_index) {
+  int nof_transitions = number_of_transitions();
+  DCHECK(transition < nof_transitions);
+  Name* key = GetKey(transition);
+  bool is_data = type == FIELD || type == CONSTANT;
+  for (; transition < nof_transitions && GetKey(transition) == key;
+       transition++) {
+    Map* target = GetTarget(transition);
+    PropertyDetails target_details = GetTargetDetails(key, target);
+
+    bool target_is_data =
+        target_details.type() == FIELD || target_details.type() == CONSTANT;
+
+    int cmp = CompareDetails(is_data, attributes, target_is_data,
+                             target_details.attributes());
+    if (cmp == 0) {
+      return transition;
+    } else if (cmp < 0) {
+      break;
+    }
+  }
+  if (out_insertion_index != NULL) *out_insertion_index = transition;
+  return kNotFound;
+}
+
+
+int TransitionArray::Search(PropertyType type, Name* name,
+                            PropertyAttributes attributes,
+                            int* out_insertion_index) {
+  int transition = SearchName(name, out_insertion_index);
+  if (transition == kNotFound) {
+    return kNotFound;
+  }
+  return SearchDetails(transition, type, attributes, out_insertion_index);
+}
 } }  // namespace v8::internal

src/transitions.h

@@ -98,9 +98,17 @@ class TransitionArray: public FixedArray {
   static Handle<TransitionArray> Insert(Handle<Map> map, Handle<Name> name,
                                         Handle<Map> target,
                                         SimpleTransitionFlag flag);
+  // Search a  transition for a given type, property name and attributes.
+  int Search(PropertyType type, Name* name, PropertyAttributes attributes,
+             int* out_insertion_index = NULL);
+
+  // Search a non-property transition (like elements kind, observe or frozen
+  // transitions).
+  inline int SearchSpecial(Symbol* symbol, int* out_insertion_index = NULL) {
+    return SearchName(symbol, out_insertion_index);
+  }
 
-  // Search a transition for a given property name.
-  inline int Search(Name* name, int* out_insertion_index = NULL);
+  static inline PropertyDetails GetTargetDetails(Name* name, Map* target);
 
   // Allocates a TransitionArray.
   static Handle<TransitionArray> Allocate(Isolate* isolate,
@@ -167,6 +175,10 @@ class TransitionArray: public FixedArray {
   bool IsSortedNoDuplicates(int valid_entries = -1);
   bool IsConsistentWithBackPointers(Map* current_map);
   bool IsEqualTo(TransitionArray* other);
+
+  // Returns true for a non-property transitions like elements kind, observed
+  // or frozen transitions.
+  static inline bool IsSpecialTransition(Name* name);
 #endif
 
   // The maximum number of transitions we want in a transition array (should
@@ -202,6 +214,31 @@ class TransitionArray: public FixedArray {
                                          Handle<Map> target,
                                          SimpleTransitionFlag flag);
 
+  // Search a first transition for a given property name.
+  inline int SearchName(Name* name, int* out_insertion_index = NULL);
+  int SearchDetails(int transition, PropertyType type,
+                    PropertyAttributes attributes, int* out_insertion_index);
+
+  // Compares two tuples <key, is_data_property, attributes>, returns -1 if
+  // tuple1 is "less" than tuple2, 0 if tuple1 equal to tuple2 and 1 otherwise.
+  static inline int CompareKeys(Name* key1, uint32_t hash1,
+                                bool is_data_property1,
+                                PropertyAttributes attributes1, Name* key2,
+                                uint32_t hash2, bool is_data_property2,
+                                PropertyAttributes attributes2);
+
+  // Compares keys, returns -1 if key1 is "less" than key2,
+  // 0 if key1 equal to key2 and 1 otherwise.
+  static inline int CompareNames(Name* key1, uint32_t hash1, Name* key2,
+                                 uint32_t hash2);
+
+  // Compares two details, returns -1 if details1 is "less" than details2,
+  // 0 if details1 equal to details2 and 1 otherwise.
+  static inline int CompareDetails(bool is_data_property1,
+                                   PropertyAttributes attributes1,
+                                   bool is_data_property2,
+                                   PropertyAttributes attributes2);
+
   inline void NoIncrementalWriteBarrierSet(int transition_number,
                                            Name* key,
                                            Map* target);

test/cctest/cctest.gyp

@@ -156,6 +156,7 @@
         'test-strtod.cc',
         'test-thread-termination.cc',
         'test-threads.cc',
+        'test-transitions.cc',
         'test-types.cc',
         'test-unbound-queue.cc',
         'test-unique.cc',

test/mjsunit/regress/regress-crbug-137689.js

@@ -44,4 +44,5 @@ assertTrue(%HaveSameMap(o, o2));
 
 Object.defineProperty(o, "foo", { set: setter, configurable: true });
 Object.defineProperty(o2, "foo", { set: setter, configurable: true });
-assertTrue(%HaveSameMap(o, o2));
+// TODO(ishell): this should eventually become assertTrue().
+assertFalse(%HaveSameMap(o, o2));