Chromium Code Reviews| Index: src/builtins.cc |
| diff --git a/src/builtins.cc b/src/builtins.cc |
| index f89a5bc2b15d136d004bc32ac6d7024d2b4e3067..048dfe892f9356555ffaf860ecc67721fc731713 100644 |
| --- a/src/builtins.cc |
| +++ b/src/builtins.cc |
| @@ -458,6 +458,14 @@ BUILTIN(ArraySlice) { |
| int relative_end = 0; |
| bool is_sloppy_arguments = false; |
| + // TODO(littledan): Look up @@species only once, not once here and |
|
adamk
2016/01/06 00:10:12
Please add a test that exercises this; that'll mak
Dan Ehrenberg
2016/01/07 00:42:57
Done
|
| + // again in the JS builtin. Pass the species out? |
| + Handle<Object> species; |
| + ASSIGN_RETURN_FAILURE_ON_EXCEPTION( |
| + isolate, species, Object::ArraySpeciesConstructor(isolate, receiver)); |
| + if (*species != isolate->context()->native_context()->array_function()) { |
| + return CallJsIntrinsic(isolate, isolate->array_slice(), args); |
| + } |
| if (receiver->IsJSArray()) { |
| DisallowHeapAllocation no_gc; |
| JSArray* array = JSArray::cast(*receiver); |
| @@ -543,6 +551,14 @@ BUILTIN(ArraySplice) { |
| if (!maybe_elms_obj.ToHandle(&elms_obj)) { |
| return CallJsIntrinsic(isolate, isolate->array_splice(), args); |
| } |
| + // TODO(littledan): Look up @@species only once, not once here and |
|
adamk
2016/01/06 00:10:11
Same note as the TODO above.
Dan Ehrenberg
2016/01/07 00:42:57
Done
|
| + // again in the JS builtin. Pass the species out? |
| + Handle<Object> species; |
| + ASSIGN_RETURN_FAILURE_ON_EXCEPTION( |
| + isolate, species, Object::ArraySpeciesConstructor(isolate, receiver)); |
| + if (*species != isolate->context()->native_context()->array_function()) { |
| + return CallJsIntrinsic(isolate, isolate->array_splice(), args); |
| + } |
| Handle<JSArray> array = Handle<JSArray>::cast(receiver); |
| DCHECK(!array->map()->is_observed()); |
| @@ -611,28 +627,39 @@ namespace { |
| */ |
| class ArrayConcatVisitor { |
| public: |
| - ArrayConcatVisitor(Isolate* isolate, Handle<FixedArray> storage, |
| + ArrayConcatVisitor(Isolate* isolate, Handle<Object> storage, |
|
adamk
2016/01/06 00:10:11
I think you should consider doing the concat stuff
Dan Ehrenberg
2016/01/07 00:42:57
Good idea; this patch doesn't include the concat s
|
| bool fast_elements) |
| : isolate_(isolate), |
| - storage_(Handle<FixedArray>::cast( |
| - isolate->global_handles()->Create(*storage))), |
| + storage_(isolate->global_handles()->Create(*storage)), |
| index_offset_(0u), |
| bit_field_(FastElementsField::encode(fast_elements) | |
| - ExceedsLimitField::encode(false)) {} |
| + ExceedsLimitField::encode(false) | |
| + IsFixedArrayField::encode(storage->IsFixedArray())) { |
| + DCHECK(!(this->fast_elements() && !is_fixed_array())); |
| + } |
| ~ArrayConcatVisitor() { clear_storage(); } |
| - void visit(uint32_t i, Handle<Object> elm) { |
| + bool visit(uint32_t i, Handle<Object> elm) { |
| + uint32_t index = index_offset_ + i; |
| + |
| + if (!is_fixed_array()) { |
| + Handle<Object> element_value; |
| + ASSIGN_RETURN_ON_EXCEPTION_VALUE( |
| + isolate_, element_value, |
| + Object::SetElement(isolate_, storage_, index, elm, STRICT), false); |
| + return true; |
| + } |
| + |
| if (i >= JSObject::kMaxElementCount - index_offset_) { |
| set_exceeds_array_limit(true); |
| - return; |
| + return true; |
| } |
| - uint32_t index = index_offset_ + i; |
| if (fast_elements()) { |
| - if (index < static_cast<uint32_t>(storage_->length())) { |
| - storage_->set(index, *elm); |
| - return; |
| + if (index < static_cast<uint32_t>(storage_fixed_array()->length())) { |
| + storage_fixed_array()->set(index, *elm); |
| + return true; |
| } |
| // Our initial estimate of length was foiled, possibly by |
| // getters on the arrays increasing the length of later arrays |
| @@ -653,6 +680,7 @@ class ArrayConcatVisitor { |
| clear_storage(); |
| set_storage(*result); |
| } |
| + return true; |
| } |
| void increase_index_offset(uint32_t delta) { |
| @@ -676,6 +704,7 @@ class ArrayConcatVisitor { |
| } |
| Handle<JSArray> ToArray() { |
| + DCHECK(is_fixed_array()); |
| Handle<JSArray> array = isolate_->factory()->NewJSArray(0); |
| Handle<Object> length = |
| isolate_->factory()->NewNumber(static_cast<double>(index_offset_)); |
| @@ -683,15 +712,26 @@ class ArrayConcatVisitor { |
| array, fast_elements() ? FAST_HOLEY_ELEMENTS : DICTIONARY_ELEMENTS); |
| array->set_map(*map); |
| array->set_length(*length); |
| - array->set_elements(*storage_); |
| + array->set_elements(*storage_fixed_array()); |
| return array; |
| } |
| + // Storage is either a FixedArray (if is_fixed_array()) or a JSReciever |
| + // (otherwise) |
| + Handle<FixedArray> storage_fixed_array() { |
| + DCHECK(is_fixed_array()); |
| + return Handle<FixedArray>::cast(storage_); |
| + } |
| + Handle<JSReceiver> storage_jsreceiver() { |
| + DCHECK(!is_fixed_array()); |
| + return Handle<JSReceiver>::cast(storage_); |
| + } |
| + |
| private: |
| // Convert storage to dictionary mode. |
| void SetDictionaryMode() { |
| - DCHECK(fast_elements()); |
| - Handle<FixedArray> current_storage(*storage_); |
| + DCHECK(fast_elements() && is_fixed_array()); |
| + Handle<FixedArray> current_storage = storage_fixed_array(); |
| Handle<SeededNumberDictionary> slow_storage( |
| SeededNumberDictionary::New(isolate_, current_storage->length())); |
| uint32_t current_length = static_cast<uint32_t>(current_storage->length()); |
| @@ -719,12 +759,13 @@ class ArrayConcatVisitor { |
| } |
| inline void set_storage(FixedArray* storage) { |
| - storage_ = |
| - Handle<FixedArray>::cast(isolate_->global_handles()->Create(storage)); |
| + DCHECK(is_fixed_array()); |
| + storage_ = isolate_->global_handles()->Create(storage); |
| } |
| class FastElementsField : public BitField<bool, 0, 1> {}; |
| class ExceedsLimitField : public BitField<bool, 1, 1> {}; |
| + class IsFixedArrayField : public BitField<bool, 2, 1> {}; |
| bool fast_elements() const { return FastElementsField::decode(bit_field_); } |
| void set_fast_elements(bool fast) { |
| @@ -733,9 +774,10 @@ class ArrayConcatVisitor { |
| void set_exceeds_array_limit(bool exceeds) { |
| bit_field_ = ExceedsLimitField::update(bit_field_, exceeds); |
| } |
| + bool is_fixed_array() const { return IsFixedArrayField::decode(bit_field_); } |
| Isolate* isolate_; |
| - Handle<FixedArray> storage_; // Always a global handle. |
| + Handle<Object> storage_; // Always a global handle. |
| // Index after last seen index. Always less than or equal to |
| // JSObject::kMaxElementCount. |
| uint32_t index_offset_; |
| @@ -806,7 +848,7 @@ uint32_t EstimateElementCount(Handle<JSArray> array) { |
| template <class ExternalArrayClass, class ElementType> |
| -void IterateTypedArrayElements(Isolate* isolate, Handle<JSObject> receiver, |
| +bool IterateTypedArrayElements(Isolate* isolate, Handle<JSObject> receiver, |
| bool elements_are_ints, |
| bool elements_are_guaranteed_smis, |
| ArrayConcatVisitor* visitor) { |
| @@ -821,7 +863,7 @@ void IterateTypedArrayElements(Isolate* isolate, Handle<JSObject> receiver, |
| HandleScope loop_scope(isolate); |
| Handle<Smi> e(Smi::FromInt(static_cast<int>(array->get_scalar(j))), |
| isolate); |
| - visitor->visit(j, e); |
| + if (!visitor->visit(j, e)) return false; |
| } |
| } else { |
| for (uint32_t j = 0; j < len; j++) { |
| @@ -829,11 +871,11 @@ void IterateTypedArrayElements(Isolate* isolate, Handle<JSObject> receiver, |
| int64_t val = static_cast<int64_t>(array->get_scalar(j)); |
| if (Smi::IsValid(static_cast<intptr_t>(val))) { |
| Handle<Smi> e(Smi::FromInt(static_cast<int>(val)), isolate); |
| - visitor->visit(j, e); |
| + if (!visitor->visit(j, e)) return false; |
| } else { |
| Handle<Object> e = |
| isolate->factory()->NewNumber(static_cast<ElementType>(val)); |
| - visitor->visit(j, e); |
| + if (!visitor->visit(j, e)) return false; |
| } |
| } |
| } |
| @@ -841,9 +883,10 @@ void IterateTypedArrayElements(Isolate* isolate, Handle<JSObject> receiver, |
| for (uint32_t j = 0; j < len; j++) { |
| HandleScope loop_scope(isolate); |
| Handle<Object> e = isolate->factory()->NewNumber(array->get_scalar(j)); |
| - visitor->visit(j, e); |
| + if (!visitor->visit(j, e)) return false; |
| } |
| } |
| + return true; |
| } |
| @@ -960,7 +1003,7 @@ bool IterateElementsSlow(Isolate* isolate, Handle<JSReceiver> receiver, |
| ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, element_value, |
| Object::GetElement(isolate, receiver, i), |
| false); |
| - visitor->visit(i, element_value); |
| + if (!visitor->visit(i, element_value)) return false; |
| } |
| } |
| visitor->increase_index_offset(length); |
| @@ -1020,7 +1063,7 @@ bool IterateElements(Isolate* isolate, Handle<JSReceiver> receiver, |
| HandleScope loop_scope(isolate); |
| Handle<Object> element_value(elements->get(j), isolate); |
| if (!element_value->IsTheHole()) { |
| - visitor->visit(j, element_value); |
| + if (!visitor->visit(j, element_value)) return false; |
| } else { |
| Maybe<bool> maybe = JSReceiver::HasElement(array, j); |
| if (!maybe.IsJust()) return false; |
| @@ -1030,7 +1073,7 @@ bool IterateElements(Isolate* isolate, Handle<JSReceiver> receiver, |
| ASSIGN_RETURN_ON_EXCEPTION_VALUE( |
| isolate, element_value, Object::GetElement(isolate, array, j), |
| false); |
| - visitor->visit(j, element_value); |
| + if (!visitor->visit(j, element_value)) return false; |
| } |
| } |
| } |
| @@ -1056,7 +1099,7 @@ bool IterateElements(Isolate* isolate, Handle<JSReceiver> receiver, |
| double double_value = elements->get_scalar(j); |
| Handle<Object> element_value = |
| isolate->factory()->NewNumber(double_value); |
| - visitor->visit(j, element_value); |
| + if (!visitor->visit(j, element_value)) return false; |
| } else { |
| Maybe<bool> maybe = JSReceiver::HasElement(array, j); |
| if (!maybe.IsJust()) return false; |
| @@ -1067,7 +1110,7 @@ bool IterateElements(Isolate* isolate, Handle<JSReceiver> receiver, |
| ASSIGN_RETURN_ON_EXCEPTION_VALUE( |
| isolate, element_value, Object::GetElement(isolate, array, j), |
| false); |
| - visitor->visit(j, element_value); |
| + if (!visitor->visit(j, element_value)) return false; |
| } |
| } |
| } |
| @@ -1096,7 +1139,7 @@ bool IterateElements(Isolate* isolate, Handle<JSReceiver> receiver, |
| Handle<Object> element; |
| ASSIGN_RETURN_ON_EXCEPTION_VALUE( |
| isolate, element, Object::GetElement(isolate, array, index), false); |
| - visitor->visit(index, element); |
| + if (!visitor->visit(index, element)) return false; |
| // Skip to next different index (i.e., omit duplicates). |
| do { |
| j++; |
| @@ -1114,43 +1157,51 @@ bool IterateElements(Isolate* isolate, Handle<JSReceiver> receiver, |
| break; |
| } |
| case INT8_ELEMENTS: { |
| - IterateTypedArrayElements<FixedInt8Array, int8_t>(isolate, array, true, |
| - true, visitor); |
| + if (!IterateTypedArrayElements<FixedInt8Array, int8_t>( |
| + isolate, array, true, true, visitor)) |
| + return false; |
| break; |
| } |
| case UINT8_ELEMENTS: { |
| - IterateTypedArrayElements<FixedUint8Array, uint8_t>(isolate, array, true, |
| - true, visitor); |
| + if (!IterateTypedArrayElements<FixedUint8Array, uint8_t>( |
| + isolate, array, true, true, visitor)) |
| + return false; |
| break; |
| } |
| case INT16_ELEMENTS: { |
| - IterateTypedArrayElements<FixedInt16Array, int16_t>(isolate, array, true, |
| - true, visitor); |
| + if (!IterateTypedArrayElements<FixedInt16Array, int16_t>( |
| + isolate, array, true, true, visitor)) |
| + return false; |
| break; |
| } |
| case UINT16_ELEMENTS: { |
| - IterateTypedArrayElements<FixedUint16Array, uint16_t>( |
| - isolate, array, true, true, visitor); |
| + if (!IterateTypedArrayElements<FixedUint16Array, uint16_t>( |
| + isolate, array, true, true, visitor)) |
| + return false; |
| break; |
| } |
| case INT32_ELEMENTS: { |
| - IterateTypedArrayElements<FixedInt32Array, int32_t>(isolate, array, true, |
| - false, visitor); |
| + if (!IterateTypedArrayElements<FixedInt32Array, int32_t>( |
| + isolate, array, true, false, visitor)) |
| + return false; |
| break; |
| } |
| case UINT32_ELEMENTS: { |
| - IterateTypedArrayElements<FixedUint32Array, uint32_t>( |
| - isolate, array, true, false, visitor); |
| + if (!IterateTypedArrayElements<FixedUint32Array, uint32_t>( |
| + isolate, array, true, false, visitor)) |
| + return false; |
| break; |
| } |
| case FLOAT32_ELEMENTS: { |
| - IterateTypedArrayElements<FixedFloat32Array, float>(isolate, array, false, |
| - false, visitor); |
| + if (!IterateTypedArrayElements<FixedFloat32Array, float>( |
| + isolate, array, false, false, visitor)) |
| + return false; |
| break; |
| } |
| case FLOAT64_ELEMENTS: { |
| - IterateTypedArrayElements<FixedFloat64Array, double>( |
| - isolate, array, false, false, visitor); |
| + if (!IterateTypedArrayElements<FixedFloat64Array, double>( |
| + isolate, array, false, false, visitor)) |
| + return false; |
| break; |
| } |
| case FAST_SLOPPY_ARGUMENTS_ELEMENTS: |
| @@ -1160,7 +1211,7 @@ bool IterateElements(Isolate* isolate, Handle<JSReceiver> receiver, |
| Handle<Object> element; |
| ASSIGN_RETURN_ON_EXCEPTION_VALUE( |
| isolate, element, Object::GetElement(isolate, array, index), false); |
| - visitor->visit(index, element); |
| + if (!visitor->visit(index, element)) return false; |
| } |
| break; |
| } |
| @@ -1194,9 +1245,12 @@ static Maybe<bool> IsConcatSpreadable(Isolate* isolate, Handle<Object> obj) { |
| } |
| -Object* Slow_ArrayConcat(Arguments* args, Isolate* isolate) { |
| +Object* Slow_ArrayConcat(Arguments* args, Handle<Object> species, |
| + Isolate* isolate) { |
| int argument_count = args->length(); |
| + bool array_species = *species == isolate->context()->array_function(); |
| + |
| // Pass 1: estimate the length and number of elements of the result. |
| // The actual length can be larger if any of the arguments have getters |
| // that mutate other arguments (but will otherwise be precise). |
| @@ -1247,7 +1301,8 @@ Object* Slow_ArrayConcat(Arguments* args, Isolate* isolate) { |
| // If estimated number of elements is more than half of length, a |
| // fixed array (fast case) is more time and space-efficient than a |
| // dictionary. |
| - bool fast_case = (estimate_nof_elements * 2) >= estimate_result_length; |
| + bool fast_case = |
| + array_species && (estimate_nof_elements * 2) >= estimate_result_length; |
| if (fast_case && kind == FAST_DOUBLE_ELEMENTS) { |
| Handle<FixedArrayBase> storage = |
| @@ -1331,18 +1386,25 @@ Object* Slow_ArrayConcat(Arguments* args, Isolate* isolate) { |
| // In case of failure, fall through. |
| } |
| - Handle<FixedArray> storage; |
| + Handle<Object> storage; |
| if (fast_case) { |
| // The backing storage array must have non-existing elements to preserve |
| // holes across concat operations. |
| storage = |
| isolate->factory()->NewFixedArrayWithHoles(estimate_result_length); |
| - } else { |
| + } else if (array_species) { |
| // TODO(126): move 25% pre-allocation logic into Dictionary::Allocate |
| uint32_t at_least_space_for = |
| estimate_nof_elements + (estimate_nof_elements >> 2); |
| - storage = Handle<FixedArray>::cast( |
| - SeededNumberDictionary::New(isolate, at_least_space_for)); |
| + storage = SeededNumberDictionary::New(isolate, at_least_space_for); |
| + } else { |
| + DCHECK(species->IsConstructor()); |
| + Handle<Object> length(Smi::FromInt(0), isolate); |
| + Handle<Object> storage_object; |
| + ASSIGN_RETURN_FAILURE_ON_EXCEPTION( |
| + isolate, storage_object, |
| + Execution::New(isolate, species, species, 1, &length)); |
| + storage = storage_object; |
| } |
| ArrayConcatVisitor visitor(isolate, storage, fast_case); |
| @@ -1366,7 +1428,12 @@ Object* Slow_ArrayConcat(Arguments* args, Isolate* isolate) { |
| THROW_NEW_ERROR_RETURN_FAILURE( |
| isolate, NewRangeError(MessageTemplate::kInvalidArrayLength)); |
| } |
| - return *visitor.ToArray(); |
| + |
| + if (array_species) { |
| + return *visitor.ToArray(); |
| + } else { |
| + return *visitor.storage_jsreceiver(); |
| + } |
| } |
| @@ -1427,11 +1494,19 @@ BUILTIN(ArrayConcat) { |
| args[0] = *receiver; |
| Handle<JSArray> result_array; |
| - if (Fast_ArrayConcat(isolate, &args).ToHandle(&result_array)) { |
| - return *result_array; |
| + |
| + // Reading @@species happens before anything else with a side effect, so |
| + // we can do it here to determine whether to take the fast path. |
| + Handle<Object> species; |
| + ASSIGN_RETURN_FAILURE_ON_EXCEPTION( |
| + isolate, species, Object::ArraySpeciesConstructor(isolate, receiver)); |
| + if (*species == isolate->context()->native_context()->array_function()) { |
| + if (Fast_ArrayConcat(isolate, &args).ToHandle(&result_array)) { |
| + return *result_array; |
| + } |
| + if (isolate->has_pending_exception()) return isolate->heap()->exception(); |
| } |
| - if (isolate->has_pending_exception()) return isolate->heap()->exception(); |
| - return Slow_ArrayConcat(&args, isolate); |
| + return Slow_ArrayConcat(&args, species, isolate); |
| } |