Reviewers: Vitaly,
Message:
Vitaly,
may you have a look?
Description:
Do not turn source array elements into writable if doing Array.slice.
Array.slice doesn't mutate original array, so it's fine with read only data.
Plus nuke unnecessary cast.
Please review this at http://codereview.chromium.org/5972004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/builtins.cc
Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index
21381f15d5cb578e49c2594d4cfec3749e366471..0c76f6944f86684b31e360d5fe1d2f6d40ba8b78
100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -380,7 +380,7 @@ static inline MaybeObject*
EnsureJSArrayWithWritableFastElements(
Object* receiver) {
if (!receiver->IsJSArray()) return NULL;
JSArray* array = JSArray::cast(receiver);
- HeapObject* elms = HeapObject::cast(array->elements());
+ HeapObject* elms = array->elements();
if (elms->map() == Heap::fixed_array_map()) return elms;
if (elms->map() == Heap::fixed_cow_array_map()) {
return array->EnsureWritableFastElements();
@@ -613,42 +613,38 @@ BUILTIN(ArraySlice) {
Object* receiver = *args.receiver();
FixedArray* elms;
int len = -1;
- { MaybeObject* maybe_elms_obj =
- EnsureJSArrayWithWritableFastElements(receiver);
- 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());
+ if (receiver->IsJSArray()) {
+ JSArray* array = JSArray::cast(receiver);
+ if (!array->HasFastElements() ||
+ !IsJSArrayFastElementMovingAllowed(array)) {
+ return CallJsBuiltin("ArraySlice", args);
+ }
- 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();
+ elms = FixedArray::cast(array->elements());
+ 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());
- }
+ // 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
}
+#endif
}
ASSERT(len >= 0);
int n_arguments = args.length() - 1;
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
