Keep new arrays allocated with 'new Array(N)' in fast mode (revisited)

Also explicit length setting with a.length = N should remain in fast mode.

R=verwaest@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22645 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/array.js

@@ -86,11 +86,20 @@ function SparseJoin(array, len, convert) {
 }
 
 
-function UseSparseVariant(object, length, is_array) {
-   return is_array &&
-       length > 1000 &&
-       (!%_IsSmi(length) ||
-        %EstimateNumberOfElements(object) < (length >> 2));
+function UseSparseVariant(array, length, is_array, touched) {
+  // Only use the sparse variant on arrays that are likely to be sparse and the
+  // number of elements touched in the operation is relatively small compared to
+  // the overall size of the array.
+  if (!is_array || length < 1000 || %IsObserved(array)) {
+    return false;
+  }
+  if (!%_IsSmi(length)) {
+    return true;
+  }
+  var elements_threshold = length >> 2;  // No more than 75% holes
+  var estimated_elements = %EstimateNumberOfElements(array);
+  return (estimated_elements < elements_threshold) &&
+    (touched > estimated_elements * 4);
 }
 
 
@@ -107,7 +116,8 @@ function Join(array, length, separator, convert) {
 
   // Attempt to convert the elements.
   try {
-    if (UseSparseVariant(array, length, is_array)) {
+    if (UseSparseVariant(array, length, is_array, length)) {
+      %NormalizeElements(array);
       if (separator.length == 0) {
         return SparseJoin(array, length, convert);
       } else {
@@ -518,13 +528,15 @@ function ArrayReverse() {
   CHECK_OBJECT_COERCIBLE(this, "Array.prototype.reverse");
 
   var array = TO_OBJECT_INLINE(this);
-  var j = TO_UINT32(array.length) - 1;
+  var len = TO_UINT32(array.length);
 
-  if (UseSparseVariant(array, j, IS_ARRAY(array))) {
-    SparseReverse(array, j+1);
+  if (UseSparseVariant(array, len, IS_ARRAY(array), len)) {
+    %NormalizeElements(array);
+    SparseReverse(array, len);
     return array;
   }
 
+  var j = len - 1;
   for (var i = 0; i < j; i++, j--) {
     var current_i = array[i];
     if (!IS_UNDEFINED(current_i) || i in array) {
@@ -670,10 +682,9 @@ function ArraySlice(start, end) {
 
   if (end_i < start_i) return result;
 
-  if (IS_ARRAY(array) &&
-      !%IsObserved(array) &&
-      (end_i > 1000) &&
-      (%EstimateNumberOfElements(array) < end_i)) {
+  if (UseSparseVariant(array, len, IS_ARRAY(array), end_i - start_i)) {
+    %NormalizeElements(array);
+    %NormalizeElements(result);
     SmartSlice(array, start_i, end_i - start_i, len, result);
   } else {
     SimpleSlice(array, start_i, end_i - start_i, len, result);
@@ -781,24 +792,20 @@ function ArraySplice(start, delete_count) {
                         ["Array.prototype.splice"]);
   }
 
-  var use_simple_splice = true;
-  if (IS_ARRAY(array) &&
-      num_elements_to_add !== del_count) {
-    // If we are only deleting/moving a few things near the end of the
-    // array then the simple version is going to be faster, because it
-    // doesn't touch most of the array.
-    var estimated_non_hole_elements = %EstimateNumberOfElements(array);
-    if (len > 20 && (estimated_non_hole_elements >> 2) < (len - start_i)) {
-      use_simple_splice = false;
-    }
+  var changed_elements = del_count;
+  if (num_elements_to_add != del_count) {
+    // If the slice needs to do a actually move elements after the insertion
+    // point, then include those in the estimate of changed elements.
+    changed_elements += len - start_i - del_count;
   }
-
-  if (use_simple_splice) {
-    SimpleSlice(array, start_i, del_count, len, deleted_elements);
-    SimpleMove(array, start_i, del_count, len, num_elements_to_add);
-  } else {
+  if (UseSparseVariant(array, len, IS_ARRAY(array), changed_elements)) {
+    %NormalizeElements(array);
+    %NormalizeElements(deleted_elements);
     SmartSlice(array, start_i, del_count, len, deleted_elements);
     SmartMove(array, start_i, del_count, len, num_elements_to_add);
+  } else {
+    SimpleSlice(array, start_i, del_count, len, deleted_elements);
+    SimpleMove(array, start_i, del_count, len, num_elements_to_add);
   }
 
   // Insert the arguments into the resulting array in
@@ -1283,7 +1290,8 @@ function ArrayIndexOf(element, index) {
   }
   var min = index;
   var max = length;
-  if (UseSparseVariant(this, length, IS_ARRAY(this))) {
+  if (UseSparseVariant(this, length, IS_ARRAY(this), max - min)) {
+    %NormalizeElements(this);
     var indices = %GetArrayKeys(this, length);
     if (IS_NUMBER(indices)) {
       // It's an interval.
@@ -1338,7 +1346,8 @@ function ArrayLastIndexOf(element, index) {
   }
   var min = 0;
   var max = index;
-  if (UseSparseVariant(this, length, IS_ARRAY(this))) {
+  if (UseSparseVariant(this, length, IS_ARRAY(this), index)) {
+    %NormalizeElements(this);
     var indices = %GetArrayKeys(this, index + 1);
     if (IS_NUMBER(indices)) {
       // It's an interval.

src/elements.cc

@@ -887,8 +887,7 @@ class FastElementsAccessor
 
   typedef typename KindTraits::BackingStore BackingStore;
 
-  // Adjusts the length of the fast backing store or returns the new length or
-  // undefined in case conversion to a slow backing store should be performed.
+  // Adjusts the length of the fast backing store.
   static Handle<Object> SetLengthWithoutNormalize(
       Handle<FixedArrayBase> backing_store,
       Handle<JSArray> array,
@@ -940,15 +939,10 @@ class FastElementsAccessor
     // Check whether the backing store should be expanded.
     uint32_t min = JSObject::NewElementsCapacity(old_capacity);
     uint32_t new_capacity = length > min ? length : min;
-    if (!array->ShouldConvertToSlowElements(new_capacity)) {
-      FastElementsAccessorSubclass::SetFastElementsCapacityAndLength(
-          array, new_capacity, length);
-      JSObject::ValidateElements(array);
-      return length_object;
-    }
-
-    // Request conversion to slow elements.
-    return isolate->factory()->undefined_value();
+    FastElementsAccessorSubclass::SetFastElementsCapacityAndLength(
+        array, new_capacity, length);
+    JSObject::ValidateElements(array);
+    return length_object;
   }
 
   static Handle<Object> DeleteCommon(Handle<JSObject> obj,

src/objects.cc

@@ -11738,6 +11738,17 @@ static void EndPerformSplice(Handle<JSArray> object) {
 MaybeHandle<Object> JSArray::SetElementsLength(
     Handle<JSArray> array,
     Handle<Object> new_length_handle) {
+  if (array->HasFastElements()) {
+    // 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>(
+        (array->GetHeap()->MaxOldGenerationSize() / kDoubleSize) / 4);
+    if (new_length_handle->IsNumber() &&
+        NumberToInt32(*new_length_handle) >= max_fast_array_size) {
+      NormalizeElements(array);
+    }
+  }
+
   // We should never end in here with a pixel or external array.
   ASSERT(array->AllowsSetElementsLength());
   if (!array->map()->is_observed()) {

src/runtime.cc

@@ -10574,15 +10574,39 @@ RUNTIME_FUNCTION(Runtime_MoveArrayContents) {
 
 // How many elements does this object/array have?
 RUNTIME_FUNCTION(Runtime_EstimateNumberOfElements) {
-  SealHandleScope shs(isolate);
+  HandleScope scope(isolate);
   ASSERT(args.length() == 1);
-  CONVERT_ARG_CHECKED(JSArray, object, 0);
-  HeapObject* elements = object->elements();
+  CONVERT_ARG_HANDLE_CHECKED(JSArray, array, 0);
+  Handle<FixedArrayBase> elements(array->elements(), isolate);
+  SealHandleScope shs(isolate);
   if (elements->IsDictionary()) {
-    int result = SeededNumberDictionary::cast(elements)->NumberOfElements();
+    int result =
+        Handle<SeededNumberDictionary>::cast(elements)->NumberOfElements();
     return Smi::FromInt(result);
   } else {
-    return object->length();
+    ASSERT(array->length()->IsSmi());
+    // For packed elements, we know the exact number of elements
+    int length = elements->length();
+    ElementsKind kind = array->GetElementsKind();
+    if (IsFastPackedElementsKind(kind)) {
+      return Smi::FromInt(length);
+    }
+    // For holey elements, take samples from the buffer checking for holes
+    // to generate the estimate.
+    const int kNumberOfHoleCheckSamples = 97;
+    int increment = (length < kNumberOfHoleCheckSamples)
+                        ? 1
+                        : static_cast<int>(length / kNumberOfHoleCheckSamples);
+    ElementsAccessor* accessor = array->GetElementsAccessor();
+    int holes = 0;
+    for (int i = 0; i < length; i += increment) {
+      if (!accessor->HasElement(array, array, i, elements)) {
+        ++holes;
+      }
+    }
+    int estimate = static_cast<int>((kNumberOfHoleCheckSamples - holes) /
+                                    kNumberOfHoleCheckSamples * length);
+    return Smi::FromInt(estimate);
   }
 }
 
@@ -14956,6 +14980,15 @@ RUNTIME_FUNCTION(Runtime_InternalArrayConstructor) {
 }
 
 
+RUNTIME_FUNCTION(Runtime_NormalizeElements) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, array, 0);
+  JSObject::NormalizeElements(array);
+  return *array;
+}
+
+
 RUNTIME_FUNCTION(Runtime_MaxSmi) {
   ASSERT(args.length() == 0);
   return Smi::FromInt(Smi::kMaxValue);

src/runtime.h

@@ -238,6 +238,7 @@ namespace internal {
   F(GetArrayKeys, 2, 1)                                \
   F(MoveArrayContents, 2, 1)                           \
   F(EstimateNumberOfElements, 1, 1)                    \
+  F(NormalizeElements, 1, 1)                           \
                                                        \
   /* Getters and Setters */                            \
   F(LookupAccessor, 3, 1)                              \

test/fuzz-natives/base.js

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Flags: --allow-natives-syntax
+
 // TODO(jkummerow): There are many ways to improve these tests, e.g.:
 // - more variance in randomized inputs
 // - better time complexity management
@@ -15,7 +17,9 @@ function makeArguments() {
   result.push(17);
   result.push(-31);
   result.push(new Array(100));
-  result.push(new Array(100003));
+  var a = %NormalizeElements([]);
+  a.length = 100003;
+  result.push(a);
   result.push(Number.MIN_VALUE);
   result.push("whoops");
   result.push("x");

test/mjsunit/allocation-site-info.js

@@ -297,10 +297,6 @@ obj = newarraycase_onearg(10, 5);
 assertKind(elements_kind.fast_double, obj);
 obj = newarraycase_onearg(0, 5);
 assertKind(elements_kind.fast_double, obj);
-// Now pass a length that forces the dictionary path.
-obj = newarraycase_onearg(100000, 5);
-assertKind(elements_kind.dictionary, obj);
-assertTrue(obj.length == 100000);
 
 // Verify that cross context calls work
 var realmA = Realm.current();

test/mjsunit/apply.js

@@ -25,6 +25,8 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+// Flags: --allow-natives-syntax
+
 function f0() {
   return this;
 }
@@ -114,7 +116,8 @@ function al() {
 
 for (var j = 1; j < 0x40000000; j <<= 1) {
   try {
-    var a = new Array(j);
+    var a = %NormalizeElements([]);
+    a.length = j;
     a[j - 1] = 42;
     assertEquals(42 + j, al.apply(345, a));
   } catch (e) {
@@ -122,7 +125,8 @@ for (var j = 1; j < 0x40000000; j <<= 1) {
     for (; j < 0x40000000; j <<= 1) {
       var caught = false;
       try {
-        a = new Array(j);
+        a = %NormalizeElements([]);
+        a.length = j;
         a[j - 1] = 42;
         al.apply(345, a);
         assertUnreachable("Apply of array with length " + a.length +

test/mjsunit/array-constructor-feedback.js

@@ -130,8 +130,7 @@ function assertKind(expected, obj, name_opt) {
   a = bar(10);
   assertKind(elements_kind.fast, a);
   assertOptimized(bar);
-  a = bar(100000);
-  assertKind(elements_kind.dictionary, a);
+  bar(100000);
   assertOptimized(bar);
 
   // If the argument isn't a smi, things should still work.

test/mjsunit/array-feedback.js

@@ -92,7 +92,7 @@ function assertKind(expected, obj, name_opt) {
   assertKind(elements_kind.fast, b);
 
   a = create1(100000);
-  assertKind(elements_kind.dictionary, a);
+  assertKind(elements_kind.fast_smi_only, a);
 
   function create3(arg1, arg2, arg3) {
     return Array(arg1, arg2, arg3);

test/mjsunit/elements-kind.js

@@ -145,7 +145,9 @@ function test_wrapper() {
   }
   assertKind(elements_kind.fast, you);
 
-  assertKind(elements_kind.dictionary, new Array(0xDECAF));
+  var temp = [];
+  temp[0xDECAF] = 0;
+  assertKind(elements_kind.dictionary, temp);
 
   var fast_double_array = new Array(0xDECAF);
   for (var i = 0; i < 0xDECAF; i++) fast_double_array[i] = i / 2;

test/mjsunit/es7/object-observe.js

@@ -1234,8 +1234,9 @@ observer2.assertCallbackRecords([
 
 // Updating length on large (slow) array
 reset();
-var slow_arr = new Array(1000000000);
+var slow_arr = %NormalizeElements([]);
 slow_arr[500000000] = 'hello';
+slow_arr.length = 1000000000;
 Object.observe(slow_arr, observer.callback);
 var spliceRecords;
 function slowSpliceCallback(records) {

test/mjsunit/polymorph-arrays.js

@@ -37,7 +37,7 @@ function init_sparse_array(a) {
     a[i] = i;
   }
   a[5000000] = 256;
-  assertTrue(%HasDictionaryElements(a));
+  return %NormalizeElements(a);
 }
 
 function testPolymorphicLoads() {
@@ -49,7 +49,7 @@ function testPolymorphicLoads() {
     var object_array = new Object;
     var sparse_object_array = new Object;
     var js_array = new Array(10);
-    var sparse_js_array = new Array(5000001);
+    var sparse_js_array = %NormalizeElements([]);
 
     init_array(object_array);
     init_array(js_array);
@@ -67,7 +67,7 @@ function testPolymorphicLoads() {
   var object_array = new Object;
   var sparse_object_array = new Object;
   var js_array = new Array(10);
-  var sparse_js_array = new Array(5000001);
+  var sparse_js_array = %NormalizeElements([]);
 
   init_array(object_array);
   init_array(js_array);
@@ -114,7 +114,8 @@ function testPolymorphicStores() {
     var object_array = new Object;
     var sparse_object_array = new Object;
     var js_array = new Array(10);
-    var sparse_js_array = new Array(5000001);
+    var sparse_js_array = [];
+    sparse_js_array.length = 5000001;
 
     init_array(object_array);
     init_array(js_array);
@@ -132,7 +133,8 @@ function testPolymorphicStores() {
   var object_array = new Object;
   var sparse_object_array = new Object;
   var js_array = new Array(10);
-  var sparse_js_array = new Array(5000001);
+  var sparse_js_array = %NormalizeElements([]);
+  sparse_js_array.length = 5000001;
 
   init_array(object_array);
   init_array(js_array);

test/mjsunit/regress/regress-2790.js

@@ -26,6 +26,6 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 // Test that we can create arrays of any size.
-for (var i = 1000; i < 1000000; i += 197) {
+for (var i = 1000; i < 1000000; i += 19703) {
   new Array(i);
 }

tools/generate-runtime-tests.py

@@ -47,7 +47,7 @@ EXPAND_MACROS = [
 # that the parser doesn't bit-rot. Change the values as needed when you add,
 # remove or change runtime functions, but make sure we don't lose our ability
 # to parse them!
-EXPECTED_FUNCTION_COUNT = 420
+EXPECTED_FUNCTION_COUNT = 421
 EXPECTED_FUZZABLE_COUNT = 335
 EXPECTED_CCTEST_COUNT = 8
 EXPECTED_UNKNOWN_COUNT = 4
@@ -124,6 +124,7 @@ BLACKLISTED = [
   # Arrays
   "ArrayConstructor",
   "InternalArrayConstructor",
+  "NormalizeElements",
 
   # Literals
   "MaterializeRegExpLiteral",