Optimize @@species based on a global 'protector' cell

This patch makes ArraySpeciesCreate fast in V8 by avoiding two property reads
when the following conditions are met:
- No Array instance has had its __proto__ reset
- No Array instance has had a constructor property defined
- Array.prototype has not had its constructor changed
- Array[Symbol.species] has not been reset

For subclasses of Array, or for conditions where one of these assumptions is
violated, the full lookup of species is done according to the ArraySpeciesCreate
algorithm. Although this is a "performance cliff", it does not come up in the
expected typical use case of @@species (Array subclassing), so it is hoped that
this can form a good start. Array subclasses will incur the slowness of looking
up @@species, but their use won't slow down invocations of, for example,
Array.prototype.slice on Array base class instances.

Possible future optimizations:
- For the fallback case where the assumptions don't hold, optimize the two
  property lookups.
- For Array.prototype.slice and Array.prototype.splice, even if the full lookup
  of @@species needs to take place, we still could take the rest of the C++
  fastpath. However, to do this correctly requires changing the calling convention
  from C++ to JS to pass the @@species out, so it is not attempted in this patch.

With this patch, microbenchmarks of Array.prototype.slice do not suffer a
noticeable performance regression, unlike their previous 2.5x penalty.

TBR=hpayer@chromium.org

Review URL: https://codereview.chromium.org/1689733002

Cr-Commit-Position: refs/heads/master@{#34199}

include/v8.h

@@ -5471,6 +5471,11 @@ class V8_EXPORT Isolate {
     kHtmlComment = 21,
     kSloppyModeBlockScopedFunctionRedefinition = 22,
     kForInInitializer = 23,
+    kArrayProtectorDirtied = 24,
+    kArraySpeciesModified = 25,
+    kArrayPrototypeConstructorModified = 26,
+    kArrayInstanceProtoModified = 27,
+    kArrayInstanceConstructorModified = 28,
 
     // If you add new values here, you'll also need to update V8Initializer.cpp
     // in Chromium.

src/builtins.cc

@@ -483,19 +483,14 @@ BUILTIN(ArraySlice) {
   int relative_end = 0;
   bool is_sloppy_arguments = false;
 
-  // TODO(littledan): Look up @@species only once, not once here and
-  // 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);
     if (!array->HasFastElements() ||
-        !IsJSArrayFastElementMovingAllowed(isolate, array)) {
+        !IsJSArrayFastElementMovingAllowed(isolate, array) ||
+        !isolate->IsArraySpeciesLookupChainIntact() ||
+        // If this is a subclass of Array, then call out to JS
+        !array->map()->new_target_is_base()) {
       AllowHeapAllocation allow_allocation;
       return CallJsIntrinsic(isolate, isolate->array_slice(), args);
     }
@@ -573,15 +568,11 @@ BUILTIN(ArraySplice) {
   MaybeHandle<FixedArrayBase> maybe_elms_obj =
       EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3);
   Handle<FixedArrayBase> elms_obj;
-  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
-  // 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()) {
+  if (!maybe_elms_obj.ToHandle(&elms_obj) ||
+      // If this is a subclass of Array, then call out to JS
+      !JSArray::cast(*receiver)->map()->new_target_is_base() ||
+      // If anything with @@species has been messed with, call out to JS
+      !isolate->IsArraySpeciesLookupChainIntact()) {
     return CallJsIntrinsic(isolate, isolate->array_splice(), args);
   }
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);

src/heap/heap.cc

@@ -2851,6 +2851,10 @@ void Heap::CreateInitialObjects() {
   cell->set_value(the_hole_value());
   set_empty_property_cell(*cell);
 
+  Handle<PropertyCell> species_cell = factory->NewPropertyCell();
+  species_cell->set_value(Smi::FromInt(Isolate::kArrayProtectorValid));
+  set_species_protector(*species_cell);
+
   set_weak_stack_trace_list(Smi::FromInt(0));
 
   set_noscript_shared_function_infos(Smi::FromInt(0));

src/heap/heap.h

@@ -188,7 +188,8 @@ namespace internal {
   V(Object, weak_stack_trace_list, WeakStackTraceList)                         \
   V(Object, noscript_shared_function_infos, NoScriptSharedFunctionInfos)       \
   V(Map, bytecode_array_map, BytecodeArrayMap)                                 \
-  V(WeakCell, empty_weak_cell, EmptyWeakCell)
+  V(WeakCell, empty_weak_cell, EmptyWeakCell)                                  \
+  V(PropertyCell, species_protector, SpeciesProtector)
 
 // Entries in this list are limited to Smis and are not visited during GC.
 #define SMI_ROOT_LIST(V)                                                   \

src/isolate.cc

@@ -2511,8 +2511,35 @@ bool Isolate::IsFastArrayConstructorPrototypeChainIntact() {
   return cell_reports_intact;
 }
 
+bool Isolate::IsArraySpeciesLookupChainIntact() {
+  // 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 or a modified __proto__
+  // - 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() {
+  DCHECK(factory()->species_protector()->value()->IsSmi());
+  DCHECK(IsArraySpeciesLookupChainIntact());
+  PropertyCell::SetValueWithInvalidation(
+      factory()->species_protector(),
+      handle(Smi::FromInt(kArrayProtectorInvalid), this));
+  DCHECK(!IsArraySpeciesLookupChainIntact());
+}
 
 void Isolate::UpdateArrayProtectorOnSetElement(Handle<JSObject> object) {
+  DisallowHeapAllocation no_gc;
   if (IsFastArrayConstructorPrototypeChainIntact() &&
       object->map()->is_prototype_map()) {
     Object* context = heap()->native_contexts_list();
@@ -2522,6 +2549,7 @@ void Isolate::UpdateArrayProtectorOnSetElement(Handle<JSObject> object) {
               *object ||
           current_context->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX) ==
               *object) {
+        CountUsage(v8::Isolate::UseCounterFeature::kArrayProtectorDirtied);
         PropertyCell::SetValueWithInvalidation(
             factory()->array_protector(),
             handle(Smi::FromInt(kArrayProtectorInvalid), this));

src/isolate.h

@@ -960,6 +960,7 @@ class Isolate {
   static const int kArrayProtectorInvalid = 0;
 
   bool IsFastArrayConstructorPrototypeChainIntact();
+  bool IsArraySpeciesLookupChainIntact();
 
   // 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
@@ -975,6 +976,7 @@ class Isolate {
   void UpdateArrayProtectorOnNormalizeElements(Handle<JSObject> object) {
     UpdateArrayProtectorOnSetElement(object);
   }
+  void InvalidateArraySpeciesProtector();
 
   // Returns true if array is the initial array prototype in any native context.
   bool IsAnyInitialArrayPrototype(Handle<JSArray> array);

src/lookup.cc

@@ -142,6 +142,52 @@ void LookupIterator::ReloadPropertyInformation() {
   DCHECK(IsFound() || !holder_->HasFastProperties());
 }
 
+bool LookupIterator::HolderIsInContextIndex(uint32_t index) const {
+  DisallowHeapAllocation no_gc;
+
+  Object* context = heap()->native_contexts_list();
+  while (!context->IsUndefined()) {
+    Context* current_context = Context::cast(context);
+    if (current_context->get(index) == *holder_) {
+      return true;
+    }
+    context = current_context->get(Context::NEXT_CONTEXT_LINK);
+  }
+  return false;
+}
+
+void LookupIterator::UpdateProtector() {
+  if (!FLAG_harmony_species) return;
+
+  if (IsElement()) return;
+  if (isolate_->bootstrapper()->IsActive()) return;
+  if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
+
+  if (*name_ == *isolate_->factory()->constructor_string()) {
+    // Setting the constructor property could change an instance's @@species
+    if (holder_->IsJSArray()) {
+      isolate_->CountUsage(
+          v8::Isolate::UseCounterFeature::kArrayInstanceConstructorModified);
+      isolate_->InvalidateArraySpeciesProtector();
+    } else if (holder_->map()->is_prototype_map()) {
+      // Setting the constructor of Array.prototype of any realm also needs
+      // to invalidate the species protector
+      if (HolderIsInContextIndex(Context::INITIAL_ARRAY_PROTOTYPE_INDEX)) {
+        isolate_->CountUsage(v8::Isolate::UseCounterFeature::
+                                 kArrayPrototypeConstructorModified);
+        isolate_->InvalidateArraySpeciesProtector();
+      }
+    }
+  } else if (*name_ == *isolate_->factory()->species_symbol()) {
+    // Setting the Symbol.species property of any Array constructor invalidates
+    // the species protector
+    if (HolderIsInContextIndex(Context::ARRAY_FUNCTION_INDEX)) {
+      isolate_->CountUsage(
+          v8::Isolate::UseCounterFeature::kArraySpeciesModified);
+      isolate_->InvalidateArraySpeciesProtector();
+    }
+  }
+}
 
 void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
   DCHECK(state_ == DATA || state_ == ACCESSOR);

src/lookup.h

@@ -256,6 +256,7 @@ class LookupIterator final BASE_EMBEDDED {
   }
   Handle<Object> GetDataValue() const;
   void WriteDataValue(Handle<Object> value);
+  void UpdateProtector();
 
  private:
   enum class InterceptorState {
@@ -320,6 +321,8 @@ class LookupIterator final BASE_EMBEDDED {
 
   State NotFound(JSReceiver* const holder) const;
 
+  bool HolderIsInContextIndex(uint32_t index) const;
+
   // If configuration_ becomes mutable, update
   // HolderIsReceiverOrHiddenPrototype.
   const Configuration configuration_;

src/objects.cc

@@ -1551,8 +1551,14 @@ bool Object::SameValueZero(Object* other) {
 MaybeHandle<Object> Object::ArraySpeciesConstructor(
     Isolate* isolate, Handle<Object> original_array) {
   Handle<Context> native_context = isolate->native_context();
+  Handle<Object> default_species = isolate->array_function();
   if (!FLAG_harmony_species) {
-    return Handle<Object>(native_context->array_function(), isolate);
+    return default_species;
+  }
+  if (original_array->IsJSArray() &&
+      Handle<JSReceiver>::cast(original_array)->map()->new_target_is_base() &&
+      isolate->IsArraySpeciesLookupChainIntact()) {
+    return default_species;
   }
   Handle<Object> constructor = isolate->factory()->undefined_value();
   Maybe<bool> is_array = Object::IsArray(original_array);
@@ -1586,7 +1592,7 @@ MaybeHandle<Object> Object::ArraySpeciesConstructor(
     }
   }
   if (constructor->IsUndefined()) {
-    return Handle<Object>(native_context->array_function(), isolate);
+    return default_species;
   } else {
     if (!constructor->IsConstructor()) {
       THROW_NEW_ERROR(isolate,
@@ -4144,6 +4150,7 @@ Maybe<bool> Object::SetPropertyInternal(LookupIterator* it,
                                         LanguageMode language_mode,
                                         StoreFromKeyed store_mode,
                                         bool* found) {
+  it->UpdateProtector();
   ShouldThrow should_throw =
       is_sloppy(language_mode) ? DONT_THROW : THROW_ON_ERROR;
 
@@ -5279,6 +5286,7 @@ MaybeHandle<Object> JSObject::DefineOwnPropertyIgnoreAttributes(
 Maybe<bool> JSObject::DefineOwnPropertyIgnoreAttributes(
     LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
     ShouldThrow should_throw, AccessorInfoHandling handling) {
+  it->UpdateProtector();
   Handle<JSObject> object = Handle<JSObject>::cast(it->GetReceiver());
   bool is_observed = object->map()->is_observed() &&
                      (it->IsElement() || !it->name()->IsPrivate());
@@ -6158,6 +6166,8 @@ void JSReceiver::DeleteNormalizedProperty(Handle<JSReceiver> object,
 
 Maybe<bool> JSReceiver::DeleteProperty(LookupIterator* it,
                                        LanguageMode language_mode) {
+  it->UpdateProtector();
+
   Isolate* isolate = it->isolate();
 
   if (it->state() == LookupIterator::JSPROXY) {
@@ -15712,6 +15722,16 @@ Maybe<bool> JSObject::SetPrototype(Handle<JSObject> object,
                                    ShouldThrow should_throw) {
   Isolate* isolate = object->GetIsolate();
 
+  // Setting the prototype of an Array instance invalidates the species
+  // protector
+  // because it could change the constructor property of the instance, which
+  // could change the @@species constructor.
+  if (object->IsJSArray() && isolate->IsArraySpeciesLookupChainIntact()) {
+    isolate->CountUsage(
+        v8::Isolate::UseCounterFeature::kArrayInstanceProtoModified);
+    isolate->InvalidateArraySpeciesProtector();
+  }
+
   const bool observed = from_javascript && object->map()->is_observed();
   Handle<Object> old_value;
   if (observed) {

test/mjsunit/harmony/array-species-constructor-delete.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-species
+
+// Overwriting the constructor of an instance updates the protector
+
+let x = [];
+
+assertEquals(Array, x.map(()=>{}).constructor);
+assertEquals(Array, x.filter(()=>{}).constructor);
+assertEquals(Array, x.slice().constructor);
+assertEquals(Array, x.splice().constructor);
+assertEquals(Array, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);
+
+class MyArray extends Array { }
+
+Object.prototype.constructor = MyArray;
+delete Array.prototype.constructor;
+
+assertEquals(MyArray, x.map(()=>{}).constructor);
+assertEquals(MyArray, x.filter(()=>{}).constructor);
+assertEquals(MyArray, x.slice().constructor);
+assertEquals(MyArray, x.splice().constructor);
+assertEquals(MyArray, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);

test/mjsunit/harmony/array-species-constructor.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-species
+
+// Overwriting the constructor of an instance updates the protector
+
+let x = [];
+
+assertEquals(Array, x.map(()=>{}).constructor);
+assertEquals(Array, x.filter(()=>{}).constructor);
+assertEquals(Array, x.slice().constructor);
+assertEquals(Array, x.splice().constructor);
+assertEquals(Array, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);
+
+class MyArray extends Array { }
+
+x.constructor = MyArray;
+
+assertEquals(MyArray, x.map(()=>{}).constructor);
+assertEquals(MyArray, x.filter(()=>{}).constructor);
+assertEquals(MyArray, x.slice().constructor);
+assertEquals(MyArray, x.splice().constructor);
+assertEquals(MyArray, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);

test/mjsunit/harmony/array-species-delete.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-species
+
+// Overwriting the constructor of an instance updates the protector
+
+let x = [];
+
+assertEquals(Array, x.map(()=>{}).constructor);
+assertEquals(Array, x.filter(()=>{}).constructor);
+assertEquals(Array, x.slice().constructor);
+assertEquals(Array, x.splice().constructor);
+assertEquals(Array, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);
+
+class MyArray extends Array { }
+
+Object.prototype[Symbol.species] = MyArray;
+delete Array[Symbol.species];
+
+assertEquals(MyArray, x.map(()=>{}).constructor);
+assertEquals(MyArray, x.filter(()=>{}).constructor);
+assertEquals(MyArray, x.slice().constructor);
+assertEquals(MyArray, x.splice().constructor);
+assertEquals(MyArray, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);

test/mjsunit/harmony/array-species-modified.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-species
+
+// Overwriting Array[Symbol.species] updates the protector
+
+let x = [];
+
+assertEquals(Array, x.map(()=>{}).constructor);
+assertEquals(Array, x.filter(()=>{}).constructor);
+assertEquals(Array, x.slice().constructor);
+assertEquals(Array, x.splice().constructor);
+assertEquals(Array, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);
+
+class MyArray extends Array { }
+
+Object.defineProperty(Array, Symbol.species, {value: MyArray});
+
+assertEquals(MyArray, x.map(()=>{}).constructor);
+assertEquals(MyArray, x.filter(()=>{}).constructor);
+assertEquals(MyArray, x.slice().constructor);
+assertEquals(MyArray, x.splice().constructor);
+assertEquals(MyArray, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);

test/mjsunit/harmony/array-species-parent-constructor.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-species
+
+// Overwriting Array.prototype.constructor updates the protector
+
+let x = [];
+
+assertEquals(Array, x.map(()=>{}).constructor);
+assertEquals(Array, x.filter(()=>{}).constructor);
+assertEquals(Array, x.slice().constructor);
+assertEquals(Array, x.splice().constructor);
+assertEquals(Array, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);
+
+class MyArray extends Array { }
+
+Array.prototype.constructor = MyArray;
+
+assertEquals(MyArray, x.map(()=>{}).constructor);
+assertEquals(MyArray, x.filter(()=>{}).constructor);
+assertEquals(MyArray, x.slice().constructor);
+assertEquals(MyArray, x.splice().constructor);
+assertEquals(MyArray, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);

test/mjsunit/harmony/array-species-proto.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-species
+
+// Overwriting an array instance's __proto__ updates the protector
+
+let x = [];
+
+assertEquals(Array, x.map(()=>{}).constructor);
+assertEquals(Array, x.filter(()=>{}).constructor);
+assertEquals(Array, x.slice().constructor);
+assertEquals(Array, x.splice().constructor);
+assertEquals(Array, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);
+
+class MyArray extends Array { }
+
+x.__proto__ = MyArray.prototype;
+
+assertEquals(MyArray, x.map(()=>{}).constructor);
+assertEquals(MyArray, x.filter(()=>{}).constructor);
+assertEquals(MyArray, x.slice().constructor);
+assertEquals(MyArray, x.splice().constructor);
+assertEquals(MyArray, x.concat([1]).constructor);
+assertEquals(1, x.concat([1])[0]);

test/mjsunit/harmony/array-species.js

@@ -153,16 +153,14 @@ count = 0;
 params = undefined;
 assertEquals(MyObservedArray,
              new MyObservedArray().slice().constructor);
-// TODO(littledan): Should be 1
-assertEquals(2, count);
+assertEquals(1, count);
 assertArrayEquals([0], params);
 
 count = 0;
 params = undefined;
 assertEquals(MyObservedArray,
              new MyObservedArray().splice().constructor);
-// TODO(littledan): Should be 1
-assertEquals(2, count);
+assertEquals(1, count);
 assertArrayEquals([0], params);
 
 // @@species constructor can be a Proxy, and the realm access doesn't