Make array __proto__ manipulations not disturb the species protector

Previously, the species protector was invalidated whenever the __proto__ of
an Array instance was manipulated. Then, if the map's new_target_is_base field
remained set, it was correct to conclude that GetPrototypeOf(array) was
%ArrayPrototype%. However, this choice caused the popular D3 framework to
invalidate the species protector, causing many functions to become slower.

This patch eliminates that aspect of the species protector. Instead, the check
is to look at the instance->map()->prototype(). It is valid to look directly
at the map's prototype slot, ignoring hidden prototypes and proxies, because
- This is only called on Array instances, so the receiver cannot be a Proxy.
- For hidden prototypes, any inaccuracy would only result in conservatively
  taking the slow path.

Theoretically, this patch could make methods applied to arrays from other
contexts slower. However, the slowdown would only affect a particular array
instance and not have a global spill-over effect. Further, the slowdown could
be addressed by tracking, either in the instance's map or in the actual
prototype object, whether it is a %ArrayPrototype% from any context, in a way
which is cheap to query, and use that rather than comparing to the currently
executing native context.

In interactive testing, this patch led the OnShape CAD system to experience
faster load times (110+s -> 40s).

BUG=chromium:606207
LOG=Y

Review-Url: https://codereview.chromium.org/1936393002
Cr-Commit-Position: refs/heads/master@{#36033}

src/builtins.cc

@@ -562,7 +562,7 @@ BUILTIN(ArraySlice) {
         !IsJSArrayFastElementMovingAllowed(isolate, array) ||
         !isolate->IsArraySpeciesLookupChainIntact() ||
         // If this is a subclass of Array, then call out to JS
-        !array->map()->new_target_is_base()) {
+        !array->HasArrayPrototype(isolate)) {
       AllowHeapAllocation allow_allocation;
       return CallJsIntrinsic(isolate, isolate->array_slice(), args);
     }
@@ -623,7 +623,7 @@ BUILTIN(ArraySplice) {
   Handle<Object> receiver = args.receiver();
   if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3) ||
       // If this is a subclass of Array, then call out to JS.
-      !JSArray::cast(*receiver)->map()->new_target_is_base() ||
+      !Handle<JSArray>::cast(receiver)->HasArrayPrototype(isolate) ||
       // If anything with @@species has been messed with, call out to JS.
       !isolate->IsArraySpeciesLookupChainIntact()) {
     return CallJsIntrinsic(isolate, isolate->array_splice(), args);

src/isolate.cc

@@ -2559,7 +2559,7 @@ bool Isolate::IsArraySpeciesLookupChainIntact() {
   // 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__
+  //   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

src/objects-inl.h

@@ -22,6 +22,7 @@
 #include "src/heap/heap-inl.h"
 #include "src/heap/heap.h"
 #include "src/isolate.h"
+#include "src/isolate-inl.h"
 #include "src/layout-descriptor-inl.h"
 #include "src/lookup.h"
 #include "src/objects.h"
@@ -2261,7 +2262,6 @@ void Struct::InitializeBody(int object_size) {
   }
 }
 
-
 bool Object::ToArrayLength(uint32_t* index) { return Object::ToUint32(index); }
 
 
@@ -7576,6 +7576,11 @@ void JSArray::SetContent(Handle<JSArray> array,
 }
 
 
+bool JSArray::HasArrayPrototype(Isolate* isolate) {
+  return map()->prototype() == *isolate->initial_array_prototype();
+}
+
+
 int TypeFeedbackInfo::ic_total_count() {
   int current = Smi::cast(READ_FIELD(this, kStorage1Offset))->value();
   return ICTotalCountField::decode(current);

src/objects.cc

@@ -1553,7 +1553,7 @@ MaybeHandle<Object> Object::ArraySpeciesConstructor(
     return default_species;
   }
   if (original_array->IsJSArray() &&
-      Handle<JSReceiver>::cast(original_array)->map()->new_target_is_base() &&
+      Handle<JSArray>::cast(original_array)->HasArrayPrototype(isolate) &&
       isolate->IsArraySpeciesLookupChainIntact()) {
     return default_species;
   }
@@ -14319,16 +14319,6 @@ 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();
-  }
-
 #ifdef DEBUG
   int size = object->Size();
 #endif

src/objects.h

@@ -10108,6 +10108,12 @@ class JSArray: public JSObject {
                                                     PropertyDescriptor* desc,
                                                     ShouldThrow should_throw);
 
+  // Checks whether the Array has the current realm's Array.prototype as its
+  // prototype. This function is best-effort and only gives a conservative
+  // approximation, erring on the side of false, in particular with respect
+  // to Proxies and objects with a hidden prototype.
+  inline bool HasArrayPrototype(Isolate* isolate);
+
   DECLARE_CAST(JSArray)
 
   // Dispatched behavior.

src/runtime/runtime-test.cc

@@ -553,5 +553,14 @@ ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(FastProperties)
 TYPED_ARRAYS(FIXED_TYPED_ARRAYS_CHECK_RUNTIME_FUNCTION)
 
 #undef FIXED_TYPED_ARRAYS_CHECK_RUNTIME_FUNCTION
+
+
+RUNTIME_FUNCTION(Runtime_SpeciesProtector) {
+  SealHandleScope shs(isolate);
+  DCHECK_EQ(0, args.length());
+  return isolate->heap()->ToBoolean(isolate->IsArraySpeciesLookupChainIntact());
+}
+
+
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime.h

@@ -901,7 +901,8 @@ namespace internal {
   F(HasFixedInt32Elements, 1, 1)              \
   F(HasFixedFloat32Elements, 1, 1)            \
   F(HasFixedFloat64Elements, 1, 1)            \
-  F(HasFixedUint8ClampedElements, 1, 1)
+  F(HasFixedUint8ClampedElements, 1, 1)       \
+  F(SpeciesProtector, 0, 1)
 
 #define FOR_EACH_INTRINSIC_TYPEDARRAY(F)     \
   F(ArrayBufferGetByteLength, 1, 1)          \

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-species
+// Flags: --harmony-species --allow-natives-syntax
 
 // Overwriting the constructor of an instance updates the protector
 
@@ -18,6 +18,7 @@ assertEquals(1, x.concat([1])[0]);
 class MyArray extends Array { }
 
 Object.defineProperty(x, 'constructor', {get() { return MyArray; }});
+assertFalse(%SpeciesProtector());
 
 assertEquals(MyArray, x.map(()=>{}).constructor);
 assertEquals(MyArray, x.filter(()=>{}).constructor);

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-species
+// Flags: --harmony-species --allow-natives-syntax
 
 // Overwriting the constructor of an instance updates the protector
 
@@ -19,6 +19,7 @@ class MyArray extends Array { }
 
 Object.prototype.constructor = MyArray;
 delete Array.prototype.constructor;
+assertFalse(%SpeciesProtector());
 
 assertEquals(MyArray, x.map(()=>{}).constructor);
 assertEquals(MyArray, x.filter(()=>{}).constructor);

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-species
+// Flags: --harmony-species --allow-natives-syntax
 
 // Overwriting the constructor of an instance updates the protector
 
@@ -18,6 +18,7 @@ assertEquals(1, x.concat([1])[0]);
 class MyArray extends Array { }
 
 x.constructor = MyArray;
+assertFalse(%SpeciesProtector());
 
 assertEquals(MyArray, x.map(()=>{}).constructor);
 assertEquals(MyArray, x.filter(()=>{}).constructor);

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-species
+// Flags: --harmony-species --allow-natives-syntax
 
 // Overwriting the constructor of an instance updates the protector
 
@@ -19,6 +19,7 @@ class MyArray extends Array { }
 
 Object.prototype[Symbol.species] = MyArray;
 delete Array[Symbol.species];
+assertFalse(%SpeciesProtector());
 
 assertEquals(MyArray, x.map(()=>{}).constructor);
 assertEquals(MyArray, x.filter(()=>{}).constructor);

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-species
+// Flags: --harmony-species --allow-natives-syntax
 
 // Overwriting Array[Symbol.species] updates the protector
 
@@ -18,6 +18,7 @@ assertEquals(1, x.concat([1])[0]);
 class MyArray extends Array { }
 
 Object.defineProperty(Array, Symbol.species, {value: MyArray});
+assertFalse(%SpeciesProtector());
 
 assertEquals(MyArray, x.map(()=>{}).constructor);
 assertEquals(MyArray, x.filter(()=>{}).constructor);

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-species
+// Flags: --harmony-species --allow-natives-syntax
 
 // Overwriting Array.prototype.constructor updates the protector
 
@@ -18,6 +18,7 @@ assertEquals(1, x.concat([1])[0]);
 class MyArray extends Array { }
 
 Array.prototype.constructor = MyArray;
+assertFalse(%SpeciesProtector());
 
 assertEquals(MyArray, x.map(()=>{}).constructor);
 assertEquals(MyArray, x.filter(()=>{}).constructor);

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-species
+// Flags: --harmony-species --allow-natives-syntax
 
 // Overwriting an array instance's __proto__ updates the protector
 
@@ -18,6 +18,7 @@ assertEquals(1, x.concat([1])[0]);
 class MyArray extends Array { }
 
 x.__proto__ = MyArray.prototype;
+assertTrue(%SpeciesProtector());
 
 assertEquals(MyArray, x.map(()=>{}).constructor);
 assertEquals(MyArray, x.filter(()=>{}).constructor);