More cleanup related to setting array.length

BUG=

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

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

src/accessors.cc

@@ -204,6 +204,7 @@ void Accessors::ArrayLengthSetter(
     v8::Local<v8::Name> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
+  // TODO(verwaest): Speed up.
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
   Handle<JSObject> object = Utils::OpenHandle(*info.This());
@@ -227,7 +228,9 @@ void Accessors::ArrayLengthSetter(
   }
 
   if (uint32_v->Number() == number_v->Number()) {
-    maybe = JSArray::SetElementsLength(array_handle, uint32_v);
+    uint32_t new_length = 0;
+    CHECK(uint32_v->ToArrayLength(&new_length));
+    maybe = JSArray::ObservableSetLength(array_handle, new_length);
     if (maybe.is_null()) isolate->OptionalRescheduleException(false);
     return;
   }

src/ast.cc

@@ -538,8 +538,7 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
   }
 
   if (array_index != values()->length()) {
-    JSArray::SetElementsLength(
-        array, handle(Smi::FromInt(array_index), isolate)).Assert();
+    JSArray::SetLength(array, array_index);
   }
   Handle<FixedArrayBase> element_values(array->elements());
 

src/builtins.cc

@@ -445,15 +445,12 @@ BUILTIN(ArrayPop) {
     return CallJsBuiltin(isolate, "$arrayPop", args);
   }
 
-  ElementsAccessor* accessor = array->GetElementsAccessor();
   uint32_t new_length = len - 1;
   Handle<Object> element;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, element, Object::GetElement(isolate, array, new_length));
 
-  RETURN_FAILURE_ON_EXCEPTION(
-      isolate,
-      accessor->SetLength(array, handle(Smi::FromInt(new_length), isolate)));
+  JSArray::SetLength(array, new_length);
   return *element;
 }
 

src/elements.h

@@ -74,9 +74,7 @@ class ElementsAccessor {
   // changing array sizes as defined in EcmaScript 5.1 15.4.5.2, i.e. array that
   // have non-deletable elements can only be shrunk to the size of highest
   // element that is non-deletable.
-  MUST_USE_RESULT virtual MaybeHandle<Object> SetLength(
-      Handle<JSArray> holder,
-      Handle<Object> new_length) = 0;
+  virtual void SetLength(Handle<JSArray> holder, uint32_t new_length) = 0;
 
   // Modifies both the length and capacity of a JSArray, resizing the underlying
   // backing store as necessary. This method does NOT honor the semantics of

src/objects-inl.h

@@ -6970,18 +6970,16 @@ void JSArray::set_length(Smi* length) {
 }
 
 
-bool JSArray::SetElementsLengthWouldNormalize(
-    Heap* heap, Handle<Object> new_length_handle) {
+bool JSArray::SetLengthWouldNormalize(Heap* heap, uint32_t new_length) {
   // If the new array won't fit in a some non-trivial fraction of the max old
   // space size, then force it to go dictionary mode.
-  int max_fast_array_size =
-      static_cast<int>((heap->MaxOldGenerationSize() / kDoubleSize) / 4);
-  return new_length_handle->IsNumber() &&
-         NumberToInt32(*new_length_handle) >= max_fast_array_size;
+  uint32_t max_fast_array_size =
+      static_cast<uint32_t>((heap->MaxOldGenerationSize() / kDoubleSize) / 4);
+  return new_length >= max_fast_array_size;
 }
 
 
-bool JSArray::AllowsSetElementsLength() {
+bool JSArray::AllowsSetLength() {
   bool result = elements()->IsFixedArray() || elements()->IsFixedDoubleArray();
   DCHECK(result == !HasExternalArrayElements());
   return result;

src/objects.cc

@@ -11976,18 +11976,19 @@ static bool GetOldValue(Isolate* isolate,
   return true;
 }
 
-MaybeHandle<Object> JSArray::SetElementsLength(
-    Handle<JSArray> array,
-    Handle<Object> new_length_handle) {
-  if (array->HasFastElements() &&
-      SetElementsLengthWouldNormalize(array->GetHeap(), new_length_handle)) {
-    NormalizeElements(array);
-  }
 
+void JSArray::SetLength(Handle<JSArray> array, uint32_t new_length) {
   // We should never end in here with a pixel or external array.
-  DCHECK(array->AllowsSetElementsLength());
+  DCHECK(array->AllowsSetLength());
+  array->GetElementsAccessor()->SetLength(array, new_length);
+}
+
+
+MaybeHandle<Object> JSArray::ObservableSetLength(Handle<JSArray> array,
+                                                 uint32_t new_length) {
   if (!array->map()->is_observed()) {
-    return array->GetElementsAccessor()->SetLength(array, new_length_handle);
+    SetLength(array, new_length);
+    return array;
   }
 
   Isolate* isolate = array->GetIsolate();
@@ -11996,8 +11997,6 @@ MaybeHandle<Object> JSArray::SetElementsLength(
   Handle<Object> old_length_handle(array->length(), isolate);
   uint32_t old_length = 0;
   CHECK(old_length_handle->ToArrayLength(&old_length));
-  uint32_t new_length = 0;
-  CHECK(new_length_handle->ToArrayLength(&new_length));
 
   static const PropertyAttributes kNoAttrFilter = NONE;
   int num_elements = array->NumberOfOwnElements(kNoAttrFilter);
@@ -12021,14 +12020,10 @@ MaybeHandle<Object> JSArray::SetElementsLength(
     }
   }
 
-  Handle<Object> hresult;
-  ASSIGN_RETURN_ON_EXCEPTION(
-      isolate, hresult,
-      array->GetElementsAccessor()->SetLength(array, new_length_handle),
-      Object);
+  SetLength(array, new_length);
 
   CHECK(array->length()->ToArrayLength(&new_length));
-  if (old_length == new_length) return hresult;
+  if (old_length == new_length) return array;
 
   RETURN_ON_EXCEPTION(isolate, BeginPerformSplice(array), Object);
 
@@ -12043,6 +12038,7 @@ MaybeHandle<Object> JSArray::SetElementsLength(
             old_values[i]),
         Object);
   }
+
   RETURN_ON_EXCEPTION(isolate,
                       JSObject::EnqueueChangeRecord(
                           array, "update", isolate->factory()->length_string(),
@@ -12064,15 +12060,13 @@ MaybeHandle<Object> JSArray::SetElementsLength(
           .Assert();
     }
 
-    ElementsAccessor* accessor = deleted->GetElementsAccessor();
-    accessor->SetLength(deleted, isolate->factory()->NewNumberFromUint(
-                                     delete_count)).Check();
+    JSArray::SetLength(deleted, delete_count);
   }
 
   RETURN_ON_EXCEPTION(
       isolate, EnqueueSpliceRecord(array, index, deleted, add_count), Object);
 
-  return hresult;
+  return array;
 }
 
 

src/objects.h

@@ -10193,15 +10193,15 @@ class JSArray: public JSObject {
 
   // If the JSArray has fast elements, and new_length would result in
   // normalization, returns true.
-  static inline bool SetElementsLengthWouldNormalize(
-      Heap* heap, Handle<Object> new_length_handle);
+  static inline bool SetLengthWouldNormalize(Heap* heap, uint32_t new_length);
 
   // Initializes the array to a certain length.
-  inline bool AllowsSetElementsLength();
-  // Can cause GC.
-  MUST_USE_RESULT static MaybeHandle<Object> SetElementsLength(
-      Handle<JSArray> array,
-      Handle<Object> length);
+  inline bool AllowsSetLength();
+
+  static void SetLength(Handle<JSArray> array, uint32_t length);
+  // Same as above but will also queue splice records if |array| is observed.
+  static MaybeHandle<Object> ObservableSetLength(Handle<JSArray> array,
+                                                 uint32_t length);
 
   // Set the content of the array to the content of storage.
   static inline void SetContent(Handle<JSArray> array,

src/runtime/runtime-array.cc

@@ -1068,8 +1068,8 @@ static Object* ArrayConstructorCommon(Isolate* isolate,
     Handle<Object> argument_one = caller_args->at<Object>(0);
     if (argument_one->IsSmi()) {
       int value = Handle<Smi>::cast(argument_one)->value();
-      if (value < 0 || JSArray::SetElementsLengthWouldNormalize(isolate->heap(),
-                                                                argument_one)) {
+      if (value < 0 ||
+          JSArray::SetLengthWouldNormalize(isolate->heap(), value)) {
         // the array is a dictionary in this case.
         can_use_type_feedback = false;
       } else if (value != 0) {

test/cctest/test-heap.cc

@@ -785,7 +785,7 @@ TEST(JSArray) {
   JSArray::Initialize(array, 0);
 
   // Set array length to 0.
-  JSArray::SetElementsLength(array, handle(Smi::FromInt(0), isolate)).Check();
+  JSArray::SetLength(array, 0);
   CHECK_EQ(Smi::FromInt(0), array->length());
   // Must be in fast mode.
   CHECK(array->HasFastSmiOrObjectElements());
@@ -797,13 +797,11 @@ TEST(JSArray) {
   CHECK_EQ(*element, *name);
 
   // Set array length with larger than smi value.
-  Handle<Object> length =
-      factory->NewNumberFromUint(static_cast<uint32_t>(Smi::kMaxValue) + 1);
-  JSArray::SetElementsLength(array, length).Check();
+  JSArray::SetLength(array, static_cast<uint32_t>(Smi::kMaxValue) + 1);
 
   uint32_t int_length = 0;
-  CHECK(length->ToArrayIndex(&int_length));
-  CHECK_EQ(*length, array->length());
+  CHECK(array->length()->ToArrayIndex(&int_length));
+  CHECK_EQ(static_cast<uint32_t>(Smi::kMaxValue) + 1, int_length);
   CHECK(array->HasDictionaryElements());  // Must be in slow mode.
 
   // array[length] = name.