Inline SetLengthWithoutNormalize into its callers

src/elements.cc

 // Copyright 2012 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.
 
 #include "src/v8.h"
 
 #include "src/arguments.h"
 #include "src/conversions.h"
 #include "src/elements.h"
 #include "src/messages.h"
@@ skipping 844 matching lines @@
  public:
   explicit FastElementsAccessor(const char* name)
       : ElementsAccessorBase<FastElementsAccessorSubclass,
                              KindTraits>(name) {}
  protected:
   friend class ElementsAccessorBase<FastElementsAccessorSubclass, KindTraits>;
   friend class SloppyArgumentsElementsAccessor;
 
   typedef typename KindTraits::BackingStore BackingStore;
 
-  // Adjusts the length of the fast backing store.
-  static uint32_t SetLengthWithoutNormalize(
-      Handle<FixedArrayBase> backing_store, Handle<JSArray> array,
-      uint32_t length) {
-    Isolate* isolate = array->GetIsolate();
-    uint32_t old_capacity = backing_store->length();
-    Handle<Object> old_length(array->length(), isolate);
-    bool same_or_smaller_size = old_length->IsSmi() &&
-        static_cast<uint32_t>(Handle<Smi>::cast(old_length)->value()) >= length;
-    ElementsKind kind = array->GetElementsKind();
-
-    if (!same_or_smaller_size && IsFastElementsKind(kind) &&
-        !IsFastHoleyElementsKind(kind)) {
-      kind = GetHoleyElementsKind(kind);
-      JSObject::TransitionElementsKind(array, kind);
-    }
-
-    // Check whether the backing store should be shrunk.
-    if (length <= old_capacity) {
-      if (array->HasFastSmiOrObjectElements()) {
-        backing_store = JSObject::EnsureWritableFastElements(array);
-      }
-      if (2 * length <= old_capacity) {
-        // If more than half the elements won't be used, trim the array.
-        if (length == 0) {
-          array->initialize_elements();
-        } else {
-          isolate->heap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
-              *backing_store, old_capacity - length);
-        }
-      } else {
-        // Otherwise, fill the unused tail with holes.
-        int old_length = FastD2IChecked(array->length()->Number());
-        for (int i = length; i < old_length; i++) {
-          Handle<BackingStore>::cast(backing_store)->set_the_hole(i);
-        }
-      }
-      return length;
-    }
-
-    // Check whether the backing store should be expanded.
-    uint32_t min = JSObject::NewElementsCapacity(old_capacity);
-    uint32_t new_capacity = length > min ? length : min;
-    FastElementsAccessorSubclass::SetFastElementsCapacityAndLength(
-        array, new_capacity, length);
-    JSObject::ValidateElements(array);
-    return length;
-  }
-
   static void DeleteCommon(Handle<JSObject> obj, uint32_t key,
                            LanguageMode language_mode) {
     DCHECK(obj->HasFastSmiOrObjectElements() ||
            obj->HasFastDoubleElements() ||
            obj->HasFastArgumentsElements());
     Isolate* isolate = obj->GetIsolate();
     Heap* heap = obj->GetHeap();
     Handle<FixedArrayBase> elements(obj->elements());
     if (*elements == heap->empty_fixed_array()) return;
 
@@ skipping 390 matching lines @@
 class DictionaryElementsAccessor
     : public ElementsAccessorBase<DictionaryElementsAccessor,
                                   ElementsKindTraits<DICTIONARY_ELEMENTS> > {
  public:
   explicit DictionaryElementsAccessor(const char* name)
       : ElementsAccessorBase<DictionaryElementsAccessor,
                              ElementsKindTraits<DICTIONARY_ELEMENTS> >(name) {}
 
   static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
                             Handle<FixedArrayBase> backing_store) {
-    uint32_t new_length =
-        SetLengthWithoutNormalize(backing_store, array, length);
-    // SetLengthWithoutNormalize does not allow length to drop below the last
-    // non-deletable element.
-    DCHECK_GE(new_length, length);
-    if (new_length <= Smi::kMaxValue) {
-      array->set_length(Smi::FromInt(new_length));
-    } else {
-      Isolate* isolate = array->GetIsolate();
-      Handle<Object> length_obj =
-          isolate->factory()->NewNumberFromUint(new_length);
-      array->set_length(*length_obj);
-    }
-  }
-
-  // Adjusts the length of the dictionary backing store and returns the new
-  // length according to ES5 section 15.4.5.2 behavior.
-  static uint32_t SetLengthWithoutNormalize(Handle<FixedArrayBase> store,
-                                            Handle<JSArray> array,
-                                            uint32_t length) {
     Handle<SeededNumberDictionary> dict =
-        Handle<SeededNumberDictionary>::cast(store);
+        Handle<SeededNumberDictionary>::cast(backing_store);
     Isolate* isolate = array->GetIsolate();
     int capacity = dict->Capacity();
     uint32_t old_length = 0;
     CHECK(array->length()->ToArrayLength(&old_length));
-    if (length < old_length) {
+    if (dict->requires_slow_elements() && length < old_length) {
       // Find last non-deletable element in range of elements to be
       // deleted and adjust range accordingly.
       for (int i = 0; i < capacity; i++) {
         DisallowHeapAllocation no_gc;
         Object* key = dict->KeyAt(i);
         if (key->IsNumber()) {
           uint32_t number = static_cast<uint32_t>(key->Number());
           if (length <= number && number < old_length) {
             PropertyDetails details = dict->DetailsAt(i);
             if (!details.IsConfigurable()) length = number + 1;
@@ skipping 17 matching lines @@
           if (length <= number && number < old_length) {
             dict->SetEntry(i, the_hole_value, the_hole_value);
             removed_entries++;
           }
         }
       }
 
       // Update the number of elements.
       dict->ElementsRemoved(removed_entries);
     }
-    return length;
+
+    if (length <= Smi::kMaxValue) {
+      array->set_length(Smi::FromInt(length));
+    } else {
+      Isolate* isolate = array->GetIsolate();

[brucedawson, 2015/06/22 17:12:35]

This declaration of 'isolate' shadows a seemingly identical declaration in the
outer scope. I don't think this is a bug, but it is a minor inefficiency and it
does have the potential to be confusing, especially after subsequent
modifications to the code.

Consider removing this declaration and using just the outer-scope declaration?

+      Handle<Object> length_obj = isolate->factory()->NewNumberFromUint(length);
+      array->set_length(*length_obj);
+    }
   }
 
   static void DeleteCommon(Handle<JSObject> obj, uint32_t key,
                            LanguageMode language_mode) {
     Isolate* isolate = obj->GetIsolate();
     Handle<FixedArray> backing_store(FixedArray::cast(obj->elements()),
                                      isolate);
     bool is_arguments =
         (obj->GetElementsKind() == SLOPPY_ARGUMENTS_ELEMENTS);
     if (is_arguments) {
@@ skipping 301 matching lines @@
   ELEMENTS_LIST(ACCESSOR_DELETE)
 #undef ACCESSOR_DELETE
   elements_accessors_ = NULL;
 }
 
 
 template <typename ElementsAccessorSubclass, typename ElementsKindTraits>
 void ElementsAccessorBase<ElementsAccessorSubclass, ElementsKindTraits>::
     SetLengthImpl(Handle<JSArray> array, uint32_t length,
                   Handle<FixedArrayBase> backing_store) {
-  // Normalize if the length does not fit in a smi. Fast mode arrays only
-  // support smi length.
-  if (JSArray::SetLengthWouldNormalize(array->GetHeap(), length)) {
-    Handle<SeededNumberDictionary> dictionary =
-        JSObject::NormalizeElements(array);
-    DCHECK(!dictionary.is_null());
-    DictionaryElementsAccessor::SetLengthImpl(array, length, dictionary);
+  DCHECK(!JSArray::SetLengthWouldNormalize(array->GetHeap(), length));
+  DCHECK(IsFastElementsKind(array->GetElementsKind()));
+  uint32_t old_length = 0;
+  CHECK(array->length()->ToArrayIndex(&old_length));
+
+  if (old_length < length) {
+    ElementsKind kind = array->GetElementsKind();
+    if (!IsFastHoleyElementsKind(kind)) {
+      kind = GetHoleyElementsKind(kind);
+      JSObject::TransitionElementsKind(array, kind);
+    }
+  }
+
+  // Check whether the backing store should be shrunk.
+  uint32_t capacity = backing_store->length();
+  if (length == 0) {
+    array->initialize_elements();
+  } else if (length <= capacity) {
+    if (array->HasFastSmiOrObjectElements()) {
+      backing_store = JSObject::EnsureWritableFastElements(array);
+    }
+    if (2 * length <= capacity) {
+      // If more than half the elements won't be used, trim the array.
+      array->GetHeap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
+          *backing_store, capacity - length);
+    } else {
+      // Otherwise, fill the unused tail with holes.
+      for (uint32_t i = length; i < old_length; i++) {
+        BackingStore::cast(*backing_store)->set_the_hole(i);
+      }
+    }
   } else {
-#ifdef DEBUG
-    uint32_t max = Smi::kMaxValue;
-    DCHECK_LE(length, max);
-#endif
-    uint32_t new_length = ElementsAccessorSubclass::SetLengthWithoutNormalize(
-        backing_store, array, length);
-    DCHECK_EQ(length, new_length);
-    array->set_length(Smi::FromInt(new_length));
+    // Check whether the backing store should be expanded.
+    capacity = Max(length, JSObject::NewElementsCapacity(capacity));
+    ElementsAccessorSubclass::SetFastElementsCapacityAndLength(array, capacity,
+                                                               length);
   }
+
+  array->set_length(Smi::FromInt(length));
+  JSObject::ValidateElements(array);
 }
 
 
 MaybeHandle<Object> ArrayConstructInitializeElements(Handle<JSArray> array,
                                                      Arguments* args) {
   if (args->length() == 0) {
     // Optimize the case where there are no parameters passed.
     JSArray::Initialize(array, JSArray::kPreallocatedArrayElements);
     return array;
 
@@ skipping 75 matching lines @@
       break;
   }
 
   array->set_elements(*elms);
   array->set_length(Smi::FromInt(number_of_elements));
   return array;
 }
 
 }  // namespace internal
 }  // namespace v8