Revert "TransitionArray now uses <is_data_property, name, attributes> tuple as...

Revert "TransitionArray now uses <is_data_property, name, attributes> tuple as a key, which allows to have several entries for the same property name."
Revert "Fix for an assertion failure in Map::FindTransitionToField(...). Appeared after r25136."

This revert is made in order to revert r25099 which potentially causes renderer hangs.

R=verwaest@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#25332}

src/globals.h

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

src/hydrogen.cc

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

src/objects-debug.cc

@@ -1169,13 +1169,15 @@ bool DescriptorArray::IsSortedNoDuplicates(int valid_entries) {
   for (int i = 0; i < number_of_descriptors(); i++) {
     Name* key = GetSortedKey(i);
     if (key == current_key) {
-      Print();
+      OFStream os(stdout);
+      PrintDescriptors(os);
       return false;
     }
     current_key = key;
     uint32_t hash = GetSortedKey(i)->Hash();
     if (hash < current) {
-      Print();
+      OFStream os(stdout);
+      PrintDescriptors(os);
       return false;
     }
     current = hash;
@@ -1207,36 +1209,23 @@ bool LayoutDescriptor::IsConsistentWithMap(Map* map) {
 
 bool TransitionArray::IsSortedNoDuplicates(int valid_entries) {
   DCHECK(valid_entries == -1);
-  Name* prev_key = NULL;
-  bool prev_is_data_property = false;
-  PropertyAttributes prev_attributes = NONE;
-  uint32_t prev_hash = 0;
+  Name* current_key = NULL;
+  uint32_t current = 0;
   for (int i = 0; i < number_of_transitions(); i++) {
     Name* key = GetSortedKey(i);
-    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);
+    if (key == current_key) {
+      OFStream os(stdout);
+      PrintTransitions(os);
+      return false;
     }
-
-    int cmp =
-        CompareKeys(prev_key, prev_hash, prev_is_data_property, prev_attributes,
-                    key, hash, is_data_property, attributes);
-    if (cmp >= 0) {
-      Print();
+    current_key = key;
+    uint32_t hash = GetSortedKey(i)->Hash();
+    if (hash < current) {
+      OFStream os(stdout);
+      PrintTransitions(os);
       return false;
     }
-    prev_key = key;
-    prev_hash = hash;
-    prev_attributes = attributes;
-    prev_is_data_property = is_data_property;
+    current = hash;
   }
   return true;
 }

src/objects-inl.h

@@ -1901,11 +1901,11 @@ 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(FIELD, *key, NONE);
+  int transition = transitions->Search(*key);
   if (transition == TransitionArray::kNotFound) return Handle<Map>::null();
-  PropertyDetails details = transitions->GetTargetDetails(transition);
-  if (details.type() != FIELD) return Handle<Map>::null();
-  DCHECK_EQ(NONE, details.attributes());
+  PropertyDetails target_details = transitions->GetTargetDetails(transition);
+  if (target_details.type() != FIELD) return Handle<Map>::null();
+  if (target_details.attributes() != NONE) return Handle<Map>::null();
   return Handle<Map>(transitions->GetTarget(transition));
 }
 
@@ -2995,10 +2995,8 @@ void Map::LookupDescriptor(JSObject* holder,
 }
 
 
-void Map::LookupTransition(JSObject* holder, Name* name,
-                           PropertyAttributes attributes,
-                           LookupResult* result) {
-  int transition_index = this->SearchTransition(FIELD, name, attributes);
+void Map::LookupTransition(JSObject* holder, Name* name, LookupResult* result) {
+  int transition_index = this->SearchTransition(name);
   if (transition_index == TransitionArray::kNotFound) return result->NotFound();
   result->TransitionResult(holder, this->GetTransition(transition_index));
 }
@@ -5320,8 +5318,7 @@ bool Map::HasTransitionArray() const {
 
 
 Map* Map::elements_transition_map() {
-  int index =
-      transitions()->SearchSpecial(GetHeap()->elements_transition_symbol());
+  int index = transitions()->Search(GetHeap()->elements_transition_symbol());
   return transitions()->GetTarget(index);
 }
 
@@ -5338,19 +5335,8 @@ Map* Map::GetTransition(int transition_index) {
 }
 
 
-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);
-  }
+int Map::SearchTransition(Name* name) {
+  if (HasTransitionArray()) return transitions()->Search(name);
   return TransitionArray::kNotFound;
 }
 
@@ -5401,17 +5387,9 @@ 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;
-        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));
+        int new_target_index = transition_array->Search(key);
+        DCHECK(new_target_index != TransitionArray::kNotFound);
+        DCHECK(transition_array->GetTarget(new_target_index) == target);
       }
     }
 #endif

src/objects-printer.cc

@@ -1134,19 +1134,18 @@ 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(target->elements_kind())
-         << ")";
+      os << " (transition to "
+         << ElementsKindToString(GetTarget(i)->elements_kind()) << ")";
     } else if (key == GetHeap()->observed_symbol()) {
       os << " (transition to Object.observe)";
     } else {
-      PropertyDetails details = GetTargetDetails(key, target);
+      PropertyDetails details = GetTargetDetails(i);
       switch (details.type()) {
         case FIELD: {
           os << " (transition to field)";
@@ -1165,7 +1164,7 @@ void TransitionArray::PrintTransitions(std::ostream& os,
       }
       os << ", attrs: " << details.attributes();
     }
-    os << " -> " << Brief(target) << "\n";
+    os << " -> " << Brief(GetTarget(i)) << "\n";
   }
 }
 

src/objects.h

@@ -280,11 +280,7 @@ enum DebugExtraICState {
 // Indicates whether the transition is simple: the target map of the transition
 // 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_PROPERTY_TRANSITION,
-  PROPERTY_TRANSITION,
-  SPECIAL_TRANSITION
-};
+enum SimpleTransitionFlag { SIMPLE_TRANSITION, FULL_TRANSITION };
 
 
 // Indicates whether we are only interested in the descriptors of a particular
@@ -5793,9 +5789,7 @@ class Map: public HeapObject {
   inline Map* elements_transition_map();
 
   inline Map* GetTransition(int transition_index);
-  inline int SearchSpecialTransition(Symbol* name);
-  inline int SearchTransition(PropertyType type, Name* name,
-                              PropertyAttributes attributes);
+  inline int SearchTransition(Name* name);
   inline FixedArrayBase* GetInitialElements();
 
   DECL_ACCESSORS(transitions, TransitionArray)
@@ -5949,7 +5943,6 @@ class Map: public HeapObject {
                                LookupResult* result);
 
   inline void LookupTransition(JSObject* holder, Name* name,
-                               PropertyAttributes attributes,
                                LookupResult* result);
 
   inline PropertyDetails GetLastDescriptorDetails();
@@ -6340,8 +6333,7 @@ class Map: public HeapObject {
       Handle<Map> map, Handle<DescriptorArray> descriptors,
       Handle<LayoutDescriptor> layout_descriptor, TransitionFlag flag,
       MaybeHandle<Name> maybe_name, const char* reason,
-      SimpleTransitionFlag simple_flag);
-
+      SimpleTransitionFlag simple_flag = FULL_TRANSITION);
   static Handle<Map> CopyReplaceDescriptor(Handle<Map> map,
                                            Handle<DescriptorArray> descriptors,
                                            Descriptor* descriptor,
@@ -6369,9 +6361,7 @@ class Map: public HeapObject {
   void ZapTransitions();
 
   void DeprecateTransitionTree();
-  void DeprecateTarget(PropertyType type, Name* key,
-                       PropertyAttributes attributes,
-                       DescriptorArray* new_descriptors,
+  void DeprecateTarget(Name* key, DescriptorArray* new_descriptors,
                        LayoutDescriptor* new_layout_descriptor);
 
   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 SearchSpecial(GetHeap()->elements_transition_symbol()) != kNotFound;
+  return Search(GetHeap()->elements_transition_symbol()) != kNotFound;
 }
 
 
@@ -140,7 +140,7 @@ Object* TransitionArray::GetTargetValue(int transition_number) {
 }
 
 
-int TransitionArray::SearchName(Name* name, int* out_insertion_index) {
+int TransitionArray::Search(Name* name, int* out_insertion_index) {
   if (IsSimpleTransition()) {
     Name* key = GetKey(kSimpleTransitionIndex);
     if (key->Equals(name)) return kSimpleTransitionIndex;
@@ -153,71 +153,6 @@ int TransitionArray::SearchName(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_PROPERTY_TRANSITION) {
+  if (flag == SIMPLE_TRANSITION) {
     result = AllocateSimple(isolate, target);
   } else {
     result = Allocate(isolate, 1);
@@ -94,19 +94,9 @@ Handle<TransitionArray> TransitionArray::Insert(Handle<Map> map,
   int number_of_transitions = map->transitions()->number_of_transitions();
   int new_nof = number_of_transitions;
 
-  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);
+  int index = map->transitions()->Search(*name, &insertion_index);
+
   if (index == kNotFound) {
     ++new_nof;
   } else {
@@ -128,12 +118,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);
   }
 
@@ -154,11 +144,7 @@ Handle<TransitionArray> TransitionArray::Insert(Handle<Map> map,
     new_nof = number_of_transitions;
 
     insertion_index = kNotFound;
-    index = is_special_transition ? map->transitions()->SearchSpecial(
-                                        Symbol::cast(*name), &insertion_index)
-                                  : map->transitions()->Search(
-                                        details.type(), *name,
-                                        details.attributes(), &insertion_index);
+    index = array->Search(*name, &insertion_index);
     if (index == kNotFound) {
       ++new_nof;
     } else {
@@ -184,46 +170,8 @@ 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,17 +98,9 @@ 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);
-  }
 
-  static inline PropertyDetails GetTargetDetails(Name* name, Map* target);
+  // Search a transition for a given property name.
+  inline int Search(Name* name, int* out_insertion_index = NULL);
 
   // Allocates a TransitionArray.
   static Handle<TransitionArray> Allocate(Isolate* isolate,
@@ -175,10 +167,6 @@ 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
@@ -214,31 +202,6 @@ 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

@@ -155,7 +155,6 @@
         'test-strtod.cc',
         'test-thread-termination.cc',
         'test-threads.cc',
-        'test-transitions.cc',
         'test-types.cc',
         'test-unbound-queue.cc',
         'test-unboxed-doubles.cc',

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

-// Copyright 2014 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Flags: --allow-natives-syntax
-
-function foo() { return 1; };
-var o1 = {};
-o1.foo = foo;
-
-var json = '{"foo": {"x": 1}}';
-var o2 = JSON.parse(json);
-var o3 = JSON.parse(json);
-assertTrue(%HaveSameMap(o2, o3));