Reviewers: mvstanton,
Message:
PTAL
Description:
Don't double-check elements in the prototype chain in array builtins
BUG=
Please review this at https://codereview.chromium.org/701513002/
Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+20, -30 lines):
M src/builtins.cc
M test/mjsunit/array-natives-elements.js
Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index
dca20379e9b846c70d9136ec9ba6ac0b91b72e25..8dbaa487b1bf7eacee5d18eb9c3030c404ec00a9
100644
--- 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);
Index: test/mjsunit/array-natives-elements.js
diff --git a/test/mjsunit/array-natives-elements.js
b/test/mjsunit/array-natives-elements.js
index
e567e7f48086615dcd634704ad58b3ab0842645e..a19a931adbf5ebee90cc50af5b66a37a8fbc85bf
100644
--- 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);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
