Commit f98caf44 authored by littledan's avatar littledan Committed by Commit bot

Various species micro-optimizations

- Inline reads to the species protector
- Put V8_LIKELY/V8_UNLIKELY to guide Array.prototype.{slice,splice,concat}
  to the fast paths
- Put the Array species good path checks directly in
  Array.prototype.concat to avoid a couple reads of the array constructor
  from the native context

These changes together bring a ~4-6% win on
kraken-orig/stanford-crypto-pbkdf2-orig which more than makes up from
the performance degradation from a recent other species-related change.

BUG=chromium:609739
R=cbruni@chromium.org
LOG=Y

Review-Url: https://codereview.chromium.org/1958713003
Cr-Commit-Position: refs/heads/master@{#36121}
parent 7536f837
...@@ -558,11 +558,11 @@ BUILTIN(ArraySlice) { ...@@ -558,11 +558,11 @@ BUILTIN(ArraySlice) {
if (receiver->IsJSArray()) { if (receiver->IsJSArray()) {
DisallowHeapAllocation no_gc; DisallowHeapAllocation no_gc;
JSArray* array = JSArray::cast(*receiver); JSArray* array = JSArray::cast(*receiver);
if (!array->HasFastElements() || if (V8_UNLIKELY(!array->HasFastElements() ||
!IsJSArrayFastElementMovingAllowed(isolate, array) || !IsJSArrayFastElementMovingAllowed(isolate, array) ||
!isolate->IsArraySpeciesLookupChainIntact() || !isolate->IsArraySpeciesLookupChainIntact() ||
// If this is a subclass of Array, then call out to JS // If this is a subclass of Array, then call out to JS
!array->HasArrayPrototype(isolate)) { !array->HasArrayPrototype(isolate))) {
AllowHeapAllocation allow_allocation; AllowHeapAllocation allow_allocation;
return CallJsIntrinsic(isolate, isolate->array_slice(), args); return CallJsIntrinsic(isolate, isolate->array_slice(), args);
} }
...@@ -621,11 +621,12 @@ BUILTIN(ArraySlice) { ...@@ -621,11 +621,12 @@ BUILTIN(ArraySlice) {
BUILTIN(ArraySplice) { BUILTIN(ArraySplice) {
HandleScope scope(isolate); HandleScope scope(isolate);
Handle<Object> receiver = args.receiver(); Handle<Object> receiver = args.receiver();
if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3) || if (V8_UNLIKELY(
// If this is a subclass of Array, then call out to JS. !EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3) ||
!Handle<JSArray>::cast(receiver)->HasArrayPrototype(isolate) || // If this is a subclass of Array, then call out to JS.
// If anything with @@species has been messed with, call out to JS. !Handle<JSArray>::cast(receiver)->HasArrayPrototype(isolate) ||
!isolate->IsArraySpeciesLookupChainIntact()) { // If anything with @@species has been messed with, call out to JS.
!isolate->IsArraySpeciesLookupChainIntact())) {
return CallJsIntrinsic(isolate, isolate->array_splice(), args); return CallJsIntrinsic(isolate, isolate->array_splice(), args);
} }
Handle<JSArray> array = Handle<JSArray>::cast(receiver); Handle<JSArray> array = Handle<JSArray>::cast(receiver);
...@@ -1491,12 +1492,21 @@ BUILTIN(ArrayConcat) { ...@@ -1491,12 +1492,21 @@ BUILTIN(ArrayConcat) {
Handle<JSArray> result_array; Handle<JSArray> result_array;
// Avoid a real species read to avoid extra lookups to the array constructor
if (V8_LIKELY(receiver->IsJSArray() &&
Handle<JSArray>::cast(receiver)->HasArrayPrototype(isolate) &&
isolate->IsArraySpeciesLookupChainIntact())) {
if (Fast_ArrayConcat(isolate, &args).ToHandle(&result_array)) {
return *result_array;
}
if (isolate->has_pending_exception()) return isolate->heap()->exception();
}
// Reading @@species happens before anything else with a side effect, so // Reading @@species happens before anything else with a side effect, so
// we can do it here to determine whether to take the fast path. // we can do it here to determine whether to take the fast path.
Handle<Object> species; Handle<Object> species;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION( ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, species, Object::ArraySpeciesConstructor(isolate, receiver)); isolate, species, Object::ArraySpeciesConstructor(isolate, receiver));
if (*species == isolate->context()->native_context()->array_function()) { if (*species == *isolate->array_function()) {
if (Fast_ArrayConcat(isolate, &args).ToHandle(&result_array)) { if (Fast_ArrayConcat(isolate, &args).ToHandle(&result_array)) {
return *result_array; return *result_array;
} }
......
...@@ -101,6 +101,24 @@ Isolate::ExceptionScope::~ExceptionScope() { ...@@ -101,6 +101,24 @@ Isolate::ExceptionScope::~ExceptionScope() {
NATIVE_CONTEXT_FIELDS(NATIVE_CONTEXT_FIELD_ACCESSOR) NATIVE_CONTEXT_FIELDS(NATIVE_CONTEXT_FIELD_ACCESSOR)
#undef NATIVE_CONTEXT_FIELD_ACCESSOR #undef NATIVE_CONTEXT_FIELD_ACCESSOR
bool Isolate::IsArraySpeciesLookupChainIntact() {
if (!FLAG_harmony_species) return true;
// Note: It would be nice to have debug checks to make sure that the
// species protector is accurate, but this would be hard to do for most of
// what the protector stands for:
// - You'd need to traverse the heap to check that no Array instance has
// a constructor property
// - To check that Array[Symbol.species] == Array, JS code has to execute,
// but JS cannot be invoked in callstack overflow situations
// All that could be checked reliably is that
// Array.prototype.constructor == Array. Given that limitation, no check is
// done here. In place, there are mjsunit tests harmony/array-species* which
// ensure that behavior is correct in various invalid protector cases.
PropertyCell* species_cell = heap()->species_protector();
return species_cell->value()->IsSmi() &&
Smi::cast(species_cell->value())->value() == kArrayProtectorValid;
}
} // namespace internal } // namespace internal
} // namespace v8 } // namespace v8
......
...@@ -2592,25 +2592,6 @@ bool Isolate::IsFastArrayConstructorPrototypeChainIntact() { ...@@ -2592,25 +2592,6 @@ bool Isolate::IsFastArrayConstructorPrototypeChainIntact() {
return cell_reports_intact; return cell_reports_intact;
} }
bool Isolate::IsArraySpeciesLookupChainIntact() {
if (!FLAG_harmony_species) return true;
// Note: It would be nice to have debug checks to make sure that the
// species protector is accurate, but this would be hard to do for most of
// what the protector stands for:
// - You'd need to traverse the heap to check that no Array instance has
// a constructor property
// - To check that Array[Symbol.species] == Array, JS code has to execute,
// but JS cannot be invoked in callstack overflow situations
// All that could be checked reliably is that
// Array.prototype.constructor == Array. Given that limitation, no check is
// done here. In place, there are mjsunit tests harmony/array-species* which
// ensure that behavior is correct in various invalid protector cases.
PropertyCell* species_cell = heap()->species_protector();
return species_cell->value()->IsSmi() &&
Smi::cast(species_cell->value())->value() == kArrayProtectorValid;
}
void Isolate::InvalidateArraySpeciesProtector() { void Isolate::InvalidateArraySpeciesProtector() {
if (!FLAG_harmony_species) return; if (!FLAG_harmony_species) return;
DCHECK(factory()->species_protector()->value()->IsSmi()); DCHECK(factory()->species_protector()->value()->IsSmi());
......
...@@ -958,7 +958,7 @@ class Isolate { ...@@ -958,7 +958,7 @@ class Isolate {
static const int kArrayProtectorInvalid = 0; static const int kArrayProtectorInvalid = 0;
bool IsFastArrayConstructorPrototypeChainIntact(); bool IsFastArrayConstructorPrototypeChainIntact();
bool IsArraySpeciesLookupChainIntact(); inline bool IsArraySpeciesLookupChainIntact();
// On intent to set an element in object, make sure that appropriate // On intent to set an element in object, make sure that appropriate
// notifications occur if the set is on the elements of the array or // notifications occur if the set is on the elements of the array or
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment