Revision: 6085
Author: [email protected]
Date: Mon Dec 20 06:41:41 2010
Log: Teach C++ ArraySlice builtin to deal with arguments object.

Array.prototype.slice.call(arguments, ...) idiom is pretty common (up to 97% of invocations
in GMail), so we'd better handle it efficiently too.

Review URL: http://codereview.chromium.org/6034003
http://code.google.com/p/v8/source/detail?r=6085

Modified:
 /branches/bleeding_edge/src/builtins.cc
 /branches/bleeding_edge/test/mjsunit/array-slice.js

=======================================
--- /branches/bleeding_edge/src/builtins.cc     Tue Dec  7 03:31:57 2010
+++ /branches/bleeding_edge/src/builtins.cc     Mon Dec 20 06:41:41 2010
@@ -515,10 +515,10 @@
   Object* elms_obj;
   { MaybeObject* maybe_elms_obj =
         EnsureJSArrayWithWritableFastElements(receiver);
+    if (maybe_elms_obj == NULL) return CallJsBuiltin("ArrayShift", args);
     if (!maybe_elms_obj->ToObject(&elms_obj)) return maybe_elms_obj;
   }
-  if (elms_obj == NULL ||
-      !IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
+  if (!IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
     return CallJsBuiltin("ArrayShift", args);
   }
   FixedArray* elms = FixedArray::cast(elms_obj);
@@ -557,10 +557,10 @@
   Object* elms_obj;
   { MaybeObject* maybe_elms_obj =
         EnsureJSArrayWithWritableFastElements(receiver);
+    if (maybe_elms_obj == NULL) return CallJsBuiltin("ArrayUnshift", args);
     if (!maybe_elms_obj->ToObject(&elms_obj)) return maybe_elms_obj;
   }
-  if (elms_obj == NULL ||
-      !IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
+  if (!IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
     return CallJsBuiltin("ArrayUnshift", args);
   }
   FixedArray* elms = FixedArray::cast(elms_obj);
@@ -611,21 +611,46 @@

 BUILTIN(ArraySlice) {
   Object* receiver = *args.receiver();
-  Object* elms_obj;
+  FixedArray* elms;
+  int len = -1;
   { MaybeObject* maybe_elms_obj =
         EnsureJSArrayWithWritableFastElements(receiver);
-    if (!maybe_elms_obj->ToObject(&elms_obj)) return maybe_elms_obj;
-  }
-  if (elms_obj == NULL ||
-      !IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
-    return CallJsBuiltin("ArraySlice", args);
-  }
-  FixedArray* elms = FixedArray::cast(elms_obj);
-  JSArray* array = JSArray::cast(receiver);
-  ASSERT(array->HasFastElements());
-
-  int len = Smi::cast(array->length())->value();
-
+    Object* elms_obj;
+    if (maybe_elms_obj != NULL && maybe_elms_obj->ToObject(&elms_obj)) {
+      if (!IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
+        return CallJsBuiltin("ArraySlice", args);
+      }
+      elms = FixedArray::cast(elms_obj);
+      JSArray* array = JSArray::cast(receiver);
+      ASSERT(array->HasFastElements());
+
+      len = Smi::cast(array->length())->value();
+    } else {
+      // Array.slice(arguments, ...) is quite a common idiom (notably more
+      // than 50% of invocations in Web apps).  Treat it in C++ as well.
+      Map* arguments_map =
+          Top::context()->global_context()->arguments_boilerplate()->map();
+
+      bool is_arguments_object_with_fast_elements =
+          receiver->IsJSObject()
+          && JSObject::cast(receiver)->map() == arguments_map
+          && JSObject::cast(receiver)->HasFastElements();
+      if (!is_arguments_object_with_fast_elements) {
+        return CallJsBuiltin("ArraySlice", args);
+      }
+      elms = FixedArray::cast(JSObject::cast(receiver)->elements());
+      len = elms->length();
+#ifdef DEBUG
+      // Arguments object by construction should have no holes, check it.
+      if (FLAG_enable_slow_asserts) {
+        for (int i = 0; i < len; i++) {
+          ASSERT(elms->get(i) != Heap::the_hole_value());
+        }
+      }
+#endif
+    }
+  }
+  ASSERT(len >= 0);
   int n_arguments = args.length() - 1;

   // Note carefully choosen defaults---if argument is missing,
@@ -693,10 +718,10 @@
   Object* elms_obj;
   { MaybeObject* maybe_elms_obj =
         EnsureJSArrayWithWritableFastElements(receiver);
+    if (maybe_elms_obj == NULL) return CallJsBuiltin("ArraySplice", args);
     if (!maybe_elms_obj->ToObject(&elms_obj)) return maybe_elms_obj;
   }
-  if (elms_obj == NULL ||
-      !IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
+  if (!IsJSArrayFastElementMovingAllowed(JSArray::cast(receiver))) {
     return CallJsBuiltin("ArraySplice", args);
   }
   FixedArray* elms = FixedArray::cast(elms_obj);
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-slice.js Tue Dec 7 03:01:02 2010 +++ /branches/bleeding_edge/test/mjsunit/array-slice.js Mon Dec 20 06:41:41 2010
@@ -218,3 +218,16 @@
     assertTrue(delete Array.prototype[5]);
   }
 })();
+
+// Check slicing on arguments object.
+(function() {
+  function func(expected, a0, a1, a2) {
+    assertEquals(expected, Array.prototype.slice.call(arguments, 1));
+  }
+
+  func([]);
+  func(['a'], 'a');
+  func(['a', 1], 'a', 1);
+  func(['a', 1, undefined], 'a', 1, undefined);
+  func(['a', 1, undefined, void(0)], 'a', 1, undefined, void(0));
+})();

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to