Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(59)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/builtins.cc

Issue 223413002: Partial recover from performance degradation after handlification of ElementsAccessor::CopyElements… (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: Addressing review comments Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | src/elements.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index 200f7fed632619b30f0011dc8bbbac118c34196d..6dfa04924c0b835203666ab4266e9bf251189bab 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -302,7 +302,7 @@ static inline Handle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
Handle<JSArray> array = Handle<JSArray>::cast(receiver);
if (array->map()->is_observed()) return Handle<FixedArrayBase>::null();
if (!array->map()->is_extensible()) return Handle<FixedArrayBase>::null();
- Handle<FixedArrayBase> elms(array->elements());
+ Handle<FixedArrayBase> elms(array->elements(), isolate);
Heap* heap = isolate->heap();
Map* map = elms->map();
if (map == heap->fixed_array_map()) {
@@ -319,7 +319,7 @@ static inline Handle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
// Need to ensure that the arguments passed in args can be contained in
// the array.
int args_length = args->length();
- if (first_added_arg >= args_length) return handle(array->elements());
+ if (first_added_arg >= args_length) return handle(array->elements(), isolate);
ElementsKind origin_kind = array->map()->elements_kind();
ASSERT(!IsFastObjectElementsKind(origin_kind));
@@ -339,7 +339,7 @@ static inline Handle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
}
if (target_kind != origin_kind) {
JSObject::TransitionElementsKind(array, target_kind);
- return handle(array->elements());
+ return handle(array->elements(), isolate);
}
return elms;
}
@@ -418,8 +418,8 @@ BUILTIN(ArrayPush) {
ElementsAccessor* accessor = array->GetElementsAccessor();
accessor->CopyElements(
- Handle<JSObject>::null(), 0, kind, new_elms, 0,
- ElementsAccessor::kCopyToEndAndInitializeToHole, elms_obj);
+ elms_obj, 0, kind, new_elms, 0,
+ ElementsAccessor::kCopyToEndAndInitializeToHole);
elms = new_elms;
}
@@ -461,8 +461,8 @@ BUILTIN(ArrayPush) {
ElementsAccessor* accessor = array->GetElementsAccessor();
accessor->CopyElements(
- Handle<JSObject>::null(), 0, kind, new_elms, 0,
- ElementsAccessor::kCopyToEndAndInitializeToHole, elms_obj);
+ elms_obj, 0, kind, new_elms, 0,
+ ElementsAccessor::kCopyToEndAndInitializeToHole);
} else {
// to_add is > 0 and new_length <= elms_len, so elms_obj cannot be the
@@ -489,18 +489,6 @@ BUILTIN(ArrayPush) {
}
-// TODO(ishell): Temporary wrapper until handlified.
-static bool ElementsAccessorHasElementWrapper(
- ElementsAccessor* accessor,
- Handle<Object> receiver,
- Handle<JSObject> holder,
- uint32_t key,
- Handle<FixedArrayBase> backing_store = Handle<FixedArrayBase>::null()) {
- return accessor->HasElement(*receiver, *holder, key,
- backing_store.is_null() ? NULL : *backing_store);
-}
-
-
BUILTIN(ArrayPop) {
HandleScope scope(isolate);
Handle<Object> receiver = args.receiver();
@@ -517,8 +505,7 @@ BUILTIN(ArrayPop) {
ElementsAccessor* accessor = array->GetElementsAccessor();
int new_length = len - 1;
Handle<Object> element;
- if (ElementsAccessorHasElementWrapper(
- accessor, array, array, new_length, elms_obj)) {
+ if (accessor->HasElement(*array, *array, new_length, *elms_obj)) {
element = accessor->Get(
array, array, new_length, elms_obj);
} else {
@@ -618,8 +605,8 @@ BUILTIN(ArrayUnshift) {
ElementsKind kind = array->GetElementsKind();
ElementsAccessor* accessor = array->GetElementsAccessor();
accessor->CopyElements(
- Handle<JSObject>::null(), 0, kind, new_elms, to_add,
- ElementsAccessor::kCopyToEndAndInitializeToHole, elms);
+ elms, 0, kind, new_elms, to_add,
+ ElementsAccessor::kCopyToEndAndInitializeToHole);
elms = new_elms;
array->set_elements(*elms);
@@ -645,87 +632,95 @@ BUILTIN(ArraySlice) {
HandleScope scope(isolate);
Heap* heap = isolate->heap();
Handle<Object> receiver = args.receiver();
- Handle<FixedArrayBase> elms;
int len = -1;
- if (receiver->IsJSArray()) {
- Handle<JSArray> array = Handle<JSArray>::cast(receiver);
- if (!IsJSArrayFastElementMovingAllowed(heap, *array)) {
- return CallJsBuiltin(isolate, "ArraySlice", args);
- }
-
- if (array->HasFastElements()) {
- elms = handle(array->elements());
- } else {
- return CallJsBuiltin(isolate, "ArraySlice", args);
- }
+ int relative_start = 0;
+ int relative_end = 0;
+ {
+ DisallowHeapAllocation no_gc;
+ if (receiver->IsJSArray()) {
+ JSArray* array = JSArray::cast(*receiver);
+ if (!IsJSArrayFastElementMovingAllowed(heap, array)) {
+ AllowHeapAllocation allow_allocation;
+ return CallJsBuiltin(isolate, "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.
- Handle<Map> arguments_map(isolate->context()->native_context()->
- sloppy_arguments_boilerplate()->map());
-
- bool is_arguments_object_with_fast_elements =
- receiver->IsJSObject() &&
- Handle<JSObject>::cast(receiver)->map() == *arguments_map;
- if (!is_arguments_object_with_fast_elements) {
- return CallJsBuiltin(isolate, "ArraySlice", args);
- }
- Handle<JSObject> object = Handle<JSObject>::cast(receiver);
+ if (!array->HasFastElements()) {
+ AllowHeapAllocation allow_allocation;
+ return CallJsBuiltin(isolate, "ArraySlice", args);
+ }
- if (object->HasFastElements()) {
- elms = handle(object->elements());
+ len = Smi::cast(array->length())->value();
} else {
- return CallJsBuiltin(isolate, "ArraySlice", args);
- }
- Handle<Object> len_obj(
- object->InObjectPropertyAt(Heap::kArgumentsLengthIndex), isolate);
- if (!len_obj->IsSmi()) {
- return CallJsBuiltin(isolate, "ArraySlice", args);
- }
- len = Handle<Smi>::cast(len_obj)->value();
- if (len > elms->length()) {
- return CallJsBuiltin(isolate, "ArraySlice", args);
- }
- }
-
- Handle<JSObject> object = Handle<JSObject>::cast(receiver);
+ // 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 = isolate->context()->native_context()->
+ sloppy_arguments_boilerplate()->map();
+
+ bool is_arguments_object_with_fast_elements =
+ receiver->IsJSObject() &&
+ JSObject::cast(*receiver)->map() == arguments_map;
+ if (!is_arguments_object_with_fast_elements) {
+ AllowHeapAllocation allow_allocation;
+ return CallJsBuiltin(isolate, "ArraySlice", args);
+ }
+ JSObject* object = JSObject::cast(*receiver);
- ASSERT(len >= 0);
- int n_arguments = args.length() - 1;
+ if (!object->HasFastElements()) {
+ AllowHeapAllocation allow_allocation;
+ return CallJsBuiltin(isolate, "ArraySlice", args);
+ }
- // Note carefully choosen defaults---if argument is missing,
- // it's undefined which gets converted to 0 for relative_start
- // and to len for relative_end.
- int relative_start = 0;
- int relative_end = len;
- if (n_arguments > 0) {
- Handle<Object> arg1 = args.at<Object>(1);
- if (arg1->IsSmi()) {
- relative_start = Handle<Smi>::cast(arg1)->value();
- } else if (arg1->IsHeapNumber()) {
- double start = Handle<HeapNumber>::cast(arg1)->value();
- if (start < kMinInt || start > kMaxInt) {
+ Object* len_obj = object->InObjectPropertyAt(Heap::kArgumentsLengthIndex);
+ if (!len_obj->IsSmi()) {
+ AllowHeapAllocation allow_allocation;
+ return CallJsBuiltin(isolate, "ArraySlice", args);
+ }
+ len = Smi::cast(len_obj)->value();
+ if (len > object->elements()->length()) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArraySlice", args);
}
- relative_start = std::isnan(start) ? 0 : static_cast<int>(start);
- } else if (!arg1->IsUndefined()) {
- return CallJsBuiltin(isolate, "ArraySlice", args);
}
- if (n_arguments > 1) {
- Handle<Object> arg2 = args.at<Object>(2);
- if (arg2->IsSmi()) {
- relative_end = Handle<Smi>::cast(arg2)->value();
- } else if (arg2->IsHeapNumber()) {
- double end = Handle<HeapNumber>::cast(arg2)->value();
- if (end < kMinInt || end > kMaxInt) {
+
+ ASSERT(len >= 0);
+ int n_arguments = args.length() - 1;
+
+ // Note carefully choosen defaults---if argument is missing,
+ // it's undefined which gets converted to 0 for relative_start
+ // and to len for relative_end.
+ relative_start = 0;
+ relative_end = len;
+ if (n_arguments > 0) {
+ Object* arg1 = args[1];
+ if (arg1->IsSmi()) {
+ relative_start = Smi::cast(arg1)->value();
+ } else if (arg1->IsHeapNumber()) {
+ double start = HeapNumber::cast(arg1)->value();
+ if (start < kMinInt || start > kMaxInt) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArraySlice", args);
}
- relative_end = std::isnan(end) ? 0 : static_cast<int>(end);
- } else if (!arg2->IsUndefined()) {
+ relative_start = std::isnan(start) ? 0 : static_cast<int>(start);
+ } else if (!arg1->IsUndefined()) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArraySlice", args);
}
+ if (n_arguments > 1) {
+ Object* arg2 = args[2];
+ if (arg2->IsSmi()) {
+ relative_end = Smi::cast(arg2)->value();
+ } else if (arg2->IsHeapNumber()) {
+ double end = HeapNumber::cast(arg2)->value();
+ if (end < kMinInt || end > kMaxInt) {
+ AllowHeapAllocation allow_allocation;
+ return CallJsBuiltin(isolate, "ArraySlice", args);
+ }
+ relative_end = std::isnan(end) ? 0 : static_cast<int>(end);
+ } else if (!arg2->IsUndefined()) {
+ AllowHeapAllocation allow_allocation;
+ return CallJsBuiltin(isolate, "ArraySlice", args);
+ }
+ }
}
}
@@ -740,13 +735,16 @@ BUILTIN(ArraySlice) {
// Calculate the length of result array.
int result_len = Max(final - k, 0);
+ Handle<JSObject> object = Handle<JSObject>::cast(receiver);
+ Handle<FixedArrayBase> elms(object->elements(), isolate);
+
ElementsKind kind = object->GetElementsKind();
if (IsHoleyElementsKind(kind)) {
+ DisallowHeapAllocation no_gc;
bool packed = true;
ElementsAccessor* accessor = ElementsAccessor::ForKind(kind);
for (int i = k; i < final; i++) {
- if (!ElementsAccessorHasElementWrapper(
- accessor, object, object, i, elms)) {
+ if (!accessor->HasElement(*object, *object, i, *elms)) {
packed = false;
break;
}
@@ -754,6 +752,7 @@ BUILTIN(ArraySlice) {
if (packed) {
kind = GetPackedElementsKind(kind);
} else if (!receiver->IsJSArray()) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArraySlice", args);
}
}
@@ -765,8 +764,8 @@ BUILTIN(ArraySlice) {
if (result_len == 0) return *result_array;
ElementsAccessor* accessor = object->GetElementsAccessor();
- accessor->CopyElements(Handle<JSObject>::null(), k, kind,
- handle(result_array->elements()), 0, result_len, elms);
+ accessor->CopyElements(
+ elms, k, kind, handle(result_array->elements(), isolate), 0, result_len);
return *result_array;
}
@@ -791,16 +790,19 @@ BUILTIN(ArraySplice) {
int relative_start = 0;
if (n_arguments > 0) {
- Handle<Object> arg1 = args.at<Object>(1);
+ DisallowHeapAllocation no_gc;
+ Object* arg1 = args[1];
if (arg1->IsSmi()) {
- relative_start = Handle<Smi>::cast(arg1)->value();
+ relative_start = Smi::cast(arg1)->value();
} else if (arg1->IsHeapNumber()) {
- double start = Handle<HeapNumber>::cast(arg1)->value();
+ double start = HeapNumber::cast(arg1)->value();
if (start < kMinInt || start > kMaxInt) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArraySplice", args);
}
relative_start = std::isnan(start) ? 0 : static_cast<int>(start);
} else if (!arg1->IsUndefined()) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArraySplice", args);
}
}
@@ -819,10 +821,12 @@ BUILTIN(ArraySplice) {
} else {
int value = 0; // ToInteger(undefined) == 0
if (n_arguments > 1) {
+ DisallowHeapAllocation no_gc;
Object* arg2 = args[2];
if (arg2->IsSmi()) {
value = Smi::cast(arg2)->value();
} else {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArraySplice", args);
}
}
@@ -856,8 +860,8 @@ BUILTIN(ArraySplice) {
DisallowHeapAllocation no_gc;
ElementsAccessor* accessor = array->GetElementsAccessor();
accessor->CopyElements(
- Handle<JSObject>::null(), actual_start, elements_kind,
- handle(result_array->elements()), 0, actual_delete_count, elms_obj);
+ elms_obj, actual_start, elements_kind,
+ handle(result_array->elements(), isolate), 0, actual_delete_count);
}
bool elms_changed = false;
@@ -881,7 +885,7 @@ BUILTIN(ArraySplice) {
if (heap->CanMoveObjectStart(*elms_obj)) {
// On the fast path we move the start of the object in memory.
- elms_obj = handle(LeftTrimFixedArray(heap, *elms_obj, delta));
+ elms_obj = handle(LeftTrimFixedArray(heap, *elms_obj, delta), isolate);
} else {
// This is the slow path. We are going to move the elements to the left
// by copying them. For trimmed values we store the hole.
@@ -935,12 +939,12 @@ BUILTIN(ArraySplice) {
if (actual_start > 0) {
// Copy the part before actual_start as is.
accessor->CopyElements(
- Handle<JSObject>::null(), 0, kind, new_elms, 0, actual_start, elms);
+ elms, 0, kind, new_elms, 0, actual_start);
}
accessor->CopyElements(
- Handle<JSObject>::null(), actual_start + actual_delete_count, kind,
+ elms, actual_start + actual_delete_count, kind,
new_elms, actual_start + item_count,
- ElementsAccessor::kCopyToEndAndInitializeToHole, elms);
+ ElementsAccessor::kCopyToEndAndInitializeToHole);
elms_obj = new_elms;
elms_changed = true;
@@ -984,9 +988,9 @@ BUILTIN(ArraySplice) {
BUILTIN(ArrayConcat) {
HandleScope scope(isolate);
Heap* heap = isolate->heap();
- Handle<Context> native_context(isolate->context()->native_context());
+ Handle<Context> native_context(isolate->context()->native_context(), isolate);
Handle<JSObject> array_proto(
- JSObject::cast(native_context->array_function()->prototype()));
+ JSObject::cast(native_context->array_function()->prototype()), isolate);
if (!ArrayPrototypeHasNoElements(heap, *native_context, *array_proto)) {
return CallJsBuiltin(isolate, "ArrayConcat", args);
}
@@ -999,13 +1003,15 @@ BUILTIN(ArrayConcat) {
bool has_double = false;
bool is_holey = false;
for (int i = 0; i < n_arguments; i++) {
- Handle<Object> arg = args.at<Object>(i);
+ DisallowHeapAllocation no_gc;
+ Object* arg = args[i];
if (!arg->IsJSArray() ||
- !Handle<JSArray>::cast(arg)->HasFastElements() ||
- Handle<JSArray>::cast(arg)->GetPrototype() != *array_proto) {
+ !JSArray::cast(arg)->HasFastElements() ||
+ JSArray::cast(arg)->GetPrototype() != *array_proto) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArrayConcat", args);
}
- int len = Smi::cast(Handle<JSArray>::cast(arg)->length())->value();
+ int len = Smi::cast(JSArray::cast(arg)->length())->value();
// We shouldn't overflow when adding another len.
const int kHalfOfMaxInt = 1 << (kBitsPerInt - 2);
@@ -1015,10 +1021,11 @@ BUILTIN(ArrayConcat) {
ASSERT(result_len >= 0);
if (result_len > FixedDoubleArray::kMaxLength) {
+ AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "ArrayConcat", args);
}
- ElementsKind arg_kind = Handle<JSArray>::cast(arg)->map()->elements_kind();
+ ElementsKind arg_kind = JSArray::cast(arg)->map()->elements_kind();
has_double = has_double || IsFastDoubleElementsKind(arg_kind);
is_holey = is_holey || IsFastHoleyElementsKind(arg_kind);
if (IsMoreGeneralElementsKindTransition(elements_kind, arg_kind)) {
@@ -1042,10 +1049,12 @@ BUILTIN(ArrayConcat) {
if (result_len == 0) return *result_array;
int j = 0;
- Handle<FixedArrayBase> storage(result_array->elements());
+ Handle<FixedArrayBase> storage(result_array->elements(), isolate);
ElementsAccessor* accessor = ElementsAccessor::ForKind(elements_kind);
for (int i = 0; i < n_arguments; i++) {
- Handle<JSArray> array = args.at<JSArray>(i);
+ // TODO(ishell): It is crucial to keep |array| as a raw pointer to avoid
+ // performance degradation. Revisit this later.
+ JSArray* array = JSArray::cast(args[i]);
int len = Smi::cast(array->length())->value();
ElementsKind from_kind = array->GetElementsKind();
if (len > 0) {
« no previous file with comments | « no previous file | src/elements.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698