Don't double-check elements in the prototype chain in array builtins

BUG=
R=mvstanton@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#25076}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25076 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/builtins.cc b/src/builtins.cc
index dca2037..8dbaa48 100644 (file)
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -202,6 +202,19 @@ static bool ArrayPrototypeHasNoElements(Heap* heap,
 }
 
 
+static inline bool IsJSArrayFastElementMovingAllowed(Heap* heap,
+                                                     JSArray* receiver) {
+  if (!FLAG_clever_optimizations) return false;
+  DisallowHeapAllocation no_gc;
+  Context* native_context = heap->isolate()->context()->native_context();
+  JSObject* array_proto =
+      JSObject::cast(native_context->array_function()->prototype());
+  PrototypeIterator iter(heap->isolate(), receiver);
+  return iter.GetCurrent() == array_proto &&
+         ArrayPrototypeHasNoElements(heap, native_context, array_proto);
+}
+
+
 // Returns empty handle if not applicable.
 MUST_USE_RESULT
 static inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
@@ -213,13 +226,13 @@ static inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   // If there may be elements accessors in the prototype chain, the fast path
   // cannot be used if there arguments to add to the array.
-  if (args != NULL && array->map()->DictionaryElementsInPrototypeChainOnly()) {
+  Heap* heap = isolate->heap();
+  if (args != NULL && !IsJSArrayFastElementMovingAllowed(heap, *array)) {
     return MaybeHandle<FixedArrayBase>();
   }
   if (array->map()->is_observed()) return MaybeHandle<FixedArrayBase>();
   if (!array->map()->is_extensible()) return MaybeHandle<FixedArrayBase>();
   Handle<FixedArrayBase> elms(array->elements(), isolate);
-  Heap* heap = isolate->heap();
   Map* map = elms->map();
   if (map == heap->fixed_array_map()) {
     if (args == NULL || array->HasFastObjectElements()) return elms;
@@ -264,19 +277,6 @@ static inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
 }
 
 
-static inline bool IsJSArrayFastElementMovingAllowed(Heap* heap,
-                                                     JSArray* receiver) {
-  if (!FLAG_clever_optimizations) return false;
-  DisallowHeapAllocation no_gc;
-  Context* native_context = heap->isolate()->context()->native_context();
-  JSObject* array_proto =
-      JSObject::cast(native_context->array_function()->prototype());
-  PrototypeIterator iter(heap->isolate(), receiver);
-  return iter.GetCurrent() == array_proto &&
-         ArrayPrototypeHasNoElements(heap, native_context, array_proto);
-}
-
-
 MUST_USE_RESULT static Object* CallJsBuiltin(
     Isolate* isolate,
     const char* name,
@@ -453,8 +453,7 @@ BUILTIN(ArrayShift) {
       EnsureJSArrayWithWritableFastElements(isolate, receiver, NULL, 0);
   Handle<FixedArrayBase> elms_obj;
   if (!maybe_elms_obj.ToHandle(&elms_obj) ||
-      !IsJSArrayFastElementMovingAllowed(heap,
-                                         *Handle<JSArray>::cast(receiver))) {
+      !IsJSArrayFastElementMovingAllowed(heap, JSArray::cast(*receiver))) {
     return CallJsBuiltin(isolate, "ArrayShift", args);
   }
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
@@ -499,11 +498,9 @@ BUILTIN(ArrayUnshift) {
   Heap* heap = isolate->heap();
   Handle<Object> receiver = args.receiver();
   MaybeHandle<FixedArrayBase> maybe_elms_obj =
-      EnsureJSArrayWithWritableFastElements(isolate, receiver, NULL, 0);
+      EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1);
   Handle<FixedArrayBase> elms_obj;
-  if (!maybe_elms_obj.ToHandle(&elms_obj) ||
-      !IsJSArrayFastElementMovingAllowed(heap,
-                                         *Handle<JSArray>::cast(receiver))) {
+  if (!maybe_elms_obj.ToHandle(&elms_obj)) {
     return CallJsBuiltin(isolate, "ArrayUnshift", args);
   }
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
@@ -524,9 +521,6 @@ BUILTIN(ArrayUnshift) {
 
   Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
 
-  JSObject::EnsureCanContainElements(array, &args, 1, to_add,
-                                     DONT_ALLOW_DOUBLE_ELEMENTS);
-
   if (new_length > elms->length()) {
     // New backing storage is needed.
     int capacity = new_length + (new_length >> 1) + 16;
@@ -708,9 +702,7 @@ BUILTIN(ArraySplice) {
   MaybeHandle<FixedArrayBase> maybe_elms_obj =
       EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3);
   Handle<FixedArrayBase> elms_obj;
-  if (!maybe_elms_obj.ToHandle(&elms_obj) ||
-      !IsJSArrayFastElementMovingAllowed(heap,
-                                         *Handle<JSArray>::cast(receiver))) {
+  if (!maybe_elms_obj.ToHandle(&elms_obj)) {
     return CallJsBuiltin(isolate, "ArraySplice", args);
   }
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);

diff --git a/test/mjsunit/array-natives-elements.js b/test/mjsunit/array-natives-elements.js
index e567e7f..a19a931 100644 (file)
--- a/test/mjsunit/array-natives-elements.js
+++ b/test/mjsunit/array-natives-elements.js
@@ -274,9 +274,7 @@ function array_natives_test() {
   assertEquals([1,1,2,3], a4);
   a4 = [1,2,3];
   a4.unshift(1.1);
-  // TODO(verwaest): We'll want to support double unshifting as well.
-  // assertTrue(%HasFastDoubleElements(a4));
-  assertTrue(%HasFastObjectElements(a4));
+  assertTrue(%HasFastDoubleElements(a4));
   assertEquals([1.1,1,2,3], a4);
   a4 = [1.1,2,3];
   a4.unshift(1);