Move Add to the elements accessor for everything but dictionary-arguments

BUG=v8:4137
LOG=n

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

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

src/elements.cc

@@ -638,6 +638,19 @@ class ElementsAccessorBase : public ElementsAccessor {
     UNREACHABLE();
   }
 
+  virtual void Add(Handle<JSObject> object, uint32_t index,
+                   Handle<Object> value, PropertyAttributes attributes,
+                   uint32_t new_capacity) final {
+    ElementsAccessorSubclass::AddImpl(object, index, value, attributes,
+                                      new_capacity);
+  }
+
+  static void AddImpl(Handle<JSObject> object, uint32_t index,
+                      Handle<Object> value, PropertyAttributes attributes,
+                      uint32_t new_capacity) {
+    UNREACHABLE();
+  }
+
   virtual MaybeHandle<AccessorPair> GetAccessorPair(
       Handle<JSObject> holder, uint32_t key,
       Handle<FixedArrayBase> backing_store) final {
@@ -919,17 +932,6 @@ class FastElementsAccessor
       : ElementsAccessorBase<FastElementsAccessorSubclass,
                              KindTraits>(name) {}
 
-  static void ReconfigureImpl(Handle<JSObject> object,
-                              Handle<FixedArrayBase> store, uint32_t index,
-                              Handle<Object> value,
-                              PropertyAttributes attributes) {
-    Handle<SeededNumberDictionary> dictionary =
-        JSObject::NormalizeElements(object);
-    index = dictionary->FindEntry(index);
-    object->GetElementsAccessor()->Reconfigure(object, dictionary, index, value,
-                                               attributes);
-  }
-
  protected:
   friend class ElementsAccessorBase<FastElementsAccessorSubclass, KindTraits>;
   friend class SloppyArgumentsElementsAccessor;
@@ -992,6 +994,42 @@ class FastElementsAccessor
     }
   }
 
+  static void ReconfigureImpl(Handle<JSObject> object,
+                              Handle<FixedArrayBase> store, uint32_t index,
+                              Handle<Object> value,
+                              PropertyAttributes attributes) {
+    Handle<SeededNumberDictionary> dictionary =
+        JSObject::NormalizeElements(object);
+    index = dictionary->FindEntry(index);
+    object->GetElementsAccessor()->Reconfigure(object, dictionary, index, value,
+                                               attributes);
+  }
+
+  static void AddImpl(Handle<JSObject> object, uint32_t index,
+                      Handle<Object> value, PropertyAttributes attributes,
+                      uint32_t new_capacity) {
+    DCHECK_EQ(NONE, attributes);
+    ElementsKind from_kind = object->GetElementsKind();
+    ElementsKind to_kind = FastElementsAccessorSubclass::kind();
+    if (IsDictionaryElementsKind(from_kind) ||
+        IsFastDoubleElementsKind(from_kind) !=
+            IsFastDoubleElementsKind(to_kind) ||
+        FastElementsAccessorSubclass::GetCapacityImpl(
+            *object, object->elements()) != new_capacity) {
+      FastElementsAccessorSubclass::GrowCapacityAndConvertImpl(object,
+                                                               new_capacity);
+    } else {
+      if (from_kind != to_kind) {
+        JSObject::TransitionElementsKind(object, to_kind);
+      }
+      if (IsFastSmiOrObjectElementsKind(from_kind)) {
+        DCHECK(IsFastSmiOrObjectElementsKind(to_kind));
+        JSObject::EnsureWritableFastElements(object);
+      }
+    }
+    FastElementsAccessorSubclass::SetImpl(object->elements(), index, *value);
+  }
+
   virtual void Delete(Handle<JSObject> obj, uint32_t key,
                       LanguageMode language_mode) final {
     DeleteCommon(obj, key, language_mode);
@@ -1455,6 +1493,22 @@ class DictionaryElementsAccessor
     dictionary->DetailsAtPut(index, details);
   }
 
+  static void AddImpl(Handle<JSObject> object, uint32_t index,
+                      Handle<Object> value, PropertyAttributes attributes,
+                      uint32_t new_capacity) {
+    PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell);
+    Handle<SeededNumberDictionary> dictionary =
+        object->HasFastElements()
+            ? JSObject::NormalizeElements(object)
+            : handle(SeededNumberDictionary::cast(object->elements()));
+    Handle<SeededNumberDictionary> new_dictionary =
+        SeededNumberDictionary::AddNumberEntry(dictionary, index, value,
+                                               details);
+    if (attributes != NONE) new_dictionary->set_requires_slow_elements();
+    if (dictionary.is_identical_to(new_dictionary)) return;
+    object->set_elements(*new_dictionary);
+  }
+
   static MaybeHandle<AccessorPair> GetAccessorPairImpl(
       Handle<JSObject> obj, uint32_t key, Handle<FixedArrayBase> store) {
     Handle<SeededNumberDictionary> backing_store =
@@ -1478,8 +1532,9 @@ class DictionaryElementsAccessor
   static uint32_t GetKeyForIndexImpl(FixedArrayBase* store, uint32_t index) {
     DisallowHeapAllocation no_gc;
     SeededNumberDictionary* dict = SeededNumberDictionary::cast(store);
-    Object* key = dict->KeyAt(index);
-    return Smi::cast(key)->value();
+    uint32_t result = 0;
+    CHECK(dict->KeyAt(index)->ToArrayIndex(&result));
+    return result;
   }
 
   static uint32_t GetIndexForKeyImpl(JSObject* holder, FixedArrayBase* store,
@@ -1611,6 +1666,20 @@ class SloppyArgumentsElementsAccessor : public ElementsAccessorBase<
     }
   }
 
+  static void AddImpl(Handle<JSObject> object, uint32_t key,
+                      Handle<Object> value, PropertyAttributes attributes,
+                      uint32_t new_capacity) {
+    DCHECK_EQ(NONE, attributes);
+    Handle<FixedArray> parameter_map(FixedArray::cast(object->elements()));
+    Handle<FixedArrayBase> old_elements(
+        FixedArrayBase::cast(parameter_map->get(1)));
+    if (old_elements->IsSeededNumberDictionary() ||
+        static_cast<uint32_t>(old_elements->length()) < new_capacity) {
+      GrowCapacityAndConvertImpl(object, new_capacity);
+    }
+    SetImpl(object->elements(), key, *value);
+  }
+
   static MaybeHandle<AccessorPair> GetAccessorPairImpl(
       Handle<JSObject> obj, uint32_t key, Handle<FixedArrayBase> parameters) {
     Handle<FixedArray> parameter_map = Handle<FixedArray>::cast(parameters);

src/elements.h

@@ -144,6 +144,9 @@ class ElementsAccessor {
                            Handle<FixedArrayBase> backing_store, uint32_t index,
                            Handle<Object> value,
                            PropertyAttributes attributes) = 0;
+  virtual void Add(Handle<JSObject> object, uint32_t index,
+                   Handle<Object> value, PropertyAttributes attributes,
+                   uint32_t new_capacity) = 0;
 
  protected:
   friend class SloppyArgumentsElementsAccessor;

src/objects.cc

@@ -12316,31 +12316,6 @@ void JSObject::ValidateElements(Handle<JSObject> object) {
 }
 
 
-static void AddDictionaryElement(Handle<JSObject> object,
-                                 Handle<SeededNumberDictionary> dictionary,
-                                 uint32_t index, Handle<Object> value,
-                                 PropertyAttributes attributes) {
-// TODO(verwaest): Handle with the elements accessor.
-// Insert element in the dictionary.
-#ifdef DEBUG
-  int entry = dictionary->FindEntry(index);
-  DCHECK_EQ(SeededNumberDictionary::kNotFound, entry);
-#endif
-
-  PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell);
-  Handle<SeededNumberDictionary> new_dictionary =
-      SeededNumberDictionary::AddNumberEntry(dictionary, index, value, details);
-
-  if (*dictionary == *new_dictionary) return;
-
-  if (object->HasSloppyArgumentsElements()) {
-    FixedArray::cast(object->elements())->set(1, *new_dictionary);
-  } else {
-    object->set_elements(*new_dictionary);
-  }
-}
-
-
 // static
 MaybeHandle<Object> JSReceiver::SetElement(Handle<JSReceiver> object,
                                            uint32_t index, Handle<Object> value,
@@ -12351,36 +12326,6 @@ MaybeHandle<Object> JSReceiver::SetElement(Handle<JSReceiver> object,
 }
 
 
-static void AddFastElement(Handle<JSObject> object, uint32_t index,
-                           Handle<Object> value, ElementsKind from_kind,
-                           uint32_t array_length, uint32_t capacity,
-                           uint32_t new_capacity) {
-  bool introduces_holes = !object->IsJSArray() || index > array_length;
-
-  ElementsKind to_kind = value->OptimalElementsKind();
-  if (IsHoleyElementsKind(from_kind)) to_kind = GetHoleyElementsKind(to_kind);
-  to_kind = IsMoreGeneralElementsKindTransition(from_kind, to_kind) ? to_kind
-                                                                    : from_kind;
-  if (introduces_holes) to_kind = GetHoleyElementsKind(to_kind);
-  ElementsAccessor* accessor = ElementsAccessor::ForKind(to_kind);
-
-  // Increase backing store capacity if that's been decided previously.
-  // The capacity is indicated as 0 if the incoming object was dictionary or
-  // slow-mode sloppy arguments.
-  if (capacity != new_capacity ||
-      IsFastDoubleElementsKind(from_kind) !=
-          IsFastDoubleElementsKind(to_kind)) {
-    accessor->GrowCapacityAndConvert(object, new_capacity);
-  } else if (from_kind != to_kind) {
-    JSObject::TransitionElementsKind(object, to_kind);
-  }
-
-  accessor->Set(object->elements(), index, *value);
-}
-
-
-// Do we want to keep fast elements when adding an element at |index|? Returns
-// |new_capacity| indicating to which capacity the object should be increased.
 static bool ShouldConvertToSlowElements(JSObject* object, uint32_t capacity,
                                         uint32_t index,
                                         uint32_t* new_capacity) {
@@ -12476,10 +12421,7 @@ MaybeHandle<Object> JSObject::AddDataElement(Handle<JSObject> object,
 
   Isolate* isolate = object->GetIsolate();
 
-  ElementsKind kind = object->GetElementsKind();
-  bool handle_slow;
   uint32_t old_length = 0;
-  uint32_t old_capacity = 0;
   uint32_t new_capacity = 0;
 
   Handle<Object> old_length_handle;
@@ -12490,38 +12432,50 @@ MaybeHandle<Object> JSObject::AddDataElement(Handle<JSObject> object,
     }
   }
 
-  Handle<SeededNumberDictionary> dictionary;
+  ElementsKind kind = object->GetElementsKind();
   FixedArrayBase* elements = object->elements();
   if (IsSloppyArgumentsElements(kind)) {
     elements = FixedArrayBase::cast(FixedArray::cast(elements)->get(1));
   }
 
-  if (elements->IsSeededNumberDictionary()) {
-    dictionary = handle(SeededNumberDictionary::cast(elements));
-    handle_slow = attributes != NONE ||
-                  !ShouldConvertToFastElements(*object, *dictionary, index,
-                                               &new_capacity);
-    if (!handle_slow) kind = BestFittingFastElementsKind(*object);
-  } else {
-    old_capacity = static_cast<uint32_t>(elements->length());
-    handle_slow =
-        attributes != NONE || ShouldConvertToSlowElements(*object, old_capacity,
-                                                          index, &new_capacity);
-    if (handle_slow) {
-      dictionary = NormalizeElements(object);
-    } else if (IsFastSmiOrObjectElementsKind(kind)) {
-      EnsureWritableFastElements(object);
-    }
-  }
-
-  if (handle_slow) {
-    if (attributes != NONE) dictionary->set_requires_slow_elements();
-    DCHECK(object->HasDictionaryElements() ||
-           object->HasDictionaryArgumentsElements());
-    AddDictionaryElement(object, dictionary, index, value, attributes);
+  if (attributes != NONE) {
+    kind = DICTIONARY_ELEMENTS;
+  } else if (elements->IsSeededNumberDictionary()) {
+    kind = ShouldConvertToFastElements(*object,
+                                       SeededNumberDictionary::cast(elements),
+                                       index, &new_capacity)
+               ? BestFittingFastElementsKind(*object)
+               : DICTIONARY_ELEMENTS;  // Overwrite in case of arguments.
+  } else if (ShouldConvertToSlowElements(
+                 *object, static_cast<uint32_t>(elements->length()), index,
+                 &new_capacity)) {
+    kind = DICTIONARY_ELEMENTS;
+  }
+
+  if (kind == DICTIONARY_ELEMENTS && object->HasSloppyArgumentsElements()) {
+    // TODO(verwaest): Distinguish fast/slow sloppy elements in ElementsKind.
+    Handle<SeededNumberDictionary> dictionary =
+        elements->IsSeededNumberDictionary()
+            ? handle(SeededNumberDictionary::cast(elements))
+            : NormalizeElements(object);
+    PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell);
+    Handle<SeededNumberDictionary> new_dictionary =
+        SeededNumberDictionary::AddNumberEntry(dictionary, index, value,
+                                               details);
+    if (attributes != NONE) new_dictionary->set_requires_slow_elements();
+    if (*dictionary != *new_dictionary) {
+      FixedArray::cast(object->elements())->set(1, *new_dictionary);
+    }
   } else {
-    AddFastElement(object, index, value, kind, old_length, old_capacity,
-                   new_capacity);
+    ElementsKind to = value->OptimalElementsKind();
+    if (IsHoleyElementsKind(kind) || !object->IsJSArray() ||
+        index > old_length) {
+      to = GetHoleyElementsKind(to);
+      kind = GetHoleyElementsKind(kind);
+    }
+    to = IsMoreGeneralElementsKindTransition(kind, to) ? to : kind;
+    ElementsAccessor* accessor = ElementsAccessor::ForKind(to);
+    accessor->Add(object, index, value, attributes, new_capacity);
   }
 
   uint32_t new_length = old_length;

src/runtime/runtime-object.cc

@@ -394,24 +394,6 @@ RUNTIME_FUNCTION(Runtime_GetProperty) {
 }
 
 
-MUST_USE_RESULT static MaybeHandle<Object> TransitionElements(
-    Handle<Object> object, ElementsKind to_kind, Isolate* isolate) {
-  HandleScope scope(isolate);
-  if (!object->IsJSObject()) {
-    isolate->ThrowIllegalOperation();
-    return MaybeHandle<Object>();
-  }
-  ElementsKind from_kind =
-      Handle<JSObject>::cast(object)->map()->elements_kind();
-  if (Map::IsValidElementsTransition(from_kind, to_kind)) {
-    JSObject::TransitionElementsKind(Handle<JSObject>::cast(object), to_kind);
-    return object;
-  }
-  isolate->ThrowIllegalOperation();
-  return MaybeHandle<Object>();
-}
-
-
 // KeyedGetProperty is called from KeyedLoadIC::GenerateGeneric.
 RUNTIME_FUNCTION(Runtime_KeyedGetProperty) {
   HandleScope scope(isolate);
@@ -470,15 +452,11 @@ RUNTIME_FUNCTION(Runtime_KeyedGetProperty) {
       Handle<JSObject> js_object = Handle<JSObject>::cast(receiver_obj);
       ElementsKind elements_kind = js_object->GetElementsKind();
       if (IsFastDoubleElementsKind(elements_kind)) {
-        Handle<Smi> key = Handle<Smi>::cast(key_obj);
-        if (key->value() >= js_object->elements()->length()) {
-          if (IsFastHoleyElementsKind(elements_kind)) {
-            elements_kind = FAST_HOLEY_ELEMENTS;
-          } else {
-            elements_kind = FAST_ELEMENTS;
-          }
-          RETURN_FAILURE_ON_EXCEPTION(
-              isolate, TransitionElements(js_object, elements_kind, isolate));
+        if (Smi::cast(*key_obj)->value() >= js_object->elements()->length()) {
+          elements_kind = IsFastHoleyElementsKind(elements_kind)
+                              ? FAST_HOLEY_ELEMENTS
+                              : FAST_ELEMENTS;
+          JSObject::TransitionElementsKind(js_object, elements_kind);
         }
       } else {
         DCHECK(IsFastSmiOrObjectElementsKind(elements_kind) ||