Make array __proto__ manipulations not disturb the species protector

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

Committed: https://crrev.com/04c8c11ee569a41d4b07839154eb0c718ff6e381
Cr-Commit-Position: refs/heads/master@{#36033}

[Dan Ehrenberg, 2016-05-02 21:47:20]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-02 21:47:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1936393002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1936393002/1

[Dan Ehrenberg, 2016-05-02 21:59:25]

Description was changed from

==========
Actually be flexible about prototype

BUG=
==========

to

==========
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.
- In the web platform, there is no use of hidden prototypes which are
  %ArrayPrototype%, and any other inaccuracy from not traversing the hidden
  prototype chain would be valid conservatism to take 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.

BUG=chromium:606207
LOG=Y
==========

[Dan Ehrenberg, 2016-05-02 21:59:25]

littledan@chromium.org changed reviewers:
+ adamk@chromium.org, cbruni@chromium.org

[Dan Ehrenberg, 2016-05-02 22:00:37]

Description was changed from

==========
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.
- In the web platform, there is no use of hidden prototypes which are
  %ArrayPrototype%, and any other inaccuracy from not traversing the hidden
  prototype chain would be valid conservatism to take 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.

BUG=chromium:606207
LOG=Y
==========

to

==========
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.
- In the web platform, there is no use of hidden prototypes which are
  %ArrayPrototype%, and any other inaccuracy from not traversing the hidden
  prototype chain would be valid conservatism to take 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
==========

[commit-bot: I haz the power, 2016-05-02 22:07:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-02 22:07:21]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_arm_rel_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng_trigger...)

[Dan Ehrenberg, 2016-05-03 00:29:04]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-03 00:29:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1936393002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1936393002/40001

[Dan Ehrenberg, 2016-05-03 00:30:31]

Description was changed from

==========
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.
- In the web platform, there is no use of hidden prototypes which are
  %ArrayPrototype%, and any other inaccuracy from not traversing the hidden
  prototype chain would be valid conservatism to take 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
==========

to

==========
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
==========

[adamk, 2016-05-03 00:33:10]

Description was changed from

==========
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
==========

to

==========
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
==========

[adamk, 2016-05-03 00:45:30]

adamk@chromium.org changed reviewers:
+ verwaest@chromium.org

[adamk, 2016-05-03 00:45:31]

+verwaest to verify that it's OK to avoid the PrototypeIterator here.

There are two sets of comments below, but the second set (on the implementation
of HasArrayPrototype) isn't relevant if you go the route suggested by the first.

https://codereview.chromium.org/1936393002/diff/40001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1936393002/diff/40001/src/builtins.cc#newcode701
src/builtins.cc:701: !array->HasArrayPrototype(isolate)) {
Given your claim in the CL description about the safety of comparing the
prototype, it seems like it might make more sense to inline HasArrayPrototype
here and below.

https://codereview.chromium.org/1936393002/diff/40001/src/builtins.cc#newcode762
src/builtins.cc:762: !receiver->IsJSArray() ||
The old code just casted directly here, I think because the || above ensures
that this is a JSArray already.

https://codereview.chromium.org/1936393002/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1936393002/diff/40001/src/objects-inl.h#newco...
src/objects-inl.h:7588: bool JSArray::HasArrayPrototype(Isolate* isolate) {
You don't need to pass in the isolate, since JSArray already knows the Isolate.
Just call GetIsolate().

https://codereview.chromium.org/1936393002/diff/40001/src/objects-inl.h#newco...
src/objects-inl.h:7589: return map()->prototype() ==
isolate->context()->initial_array_prototype();
This should be coming off the native_context(). But you should be able to just
use

GetIsolate()->initial_array_prototype()

(defined in a macro in isolates-inl.h)

https://codereview.chromium.org/1936393002/diff/40001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1936393002/diff/40001/src/objects.h#newcode10178
src/objects.h:10178: MUST_USE_RESULT inline bool HasArrayPrototype(Isolate*
isolate);
This doesn't need MUST_USE_RESULT (that's used to denote things that could
throw).

[commit-bot: I haz the power, 2016-05-03 00:49:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-03 00:49:52]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[Camillo Bruni, 2016-05-03 08:05:54]

https://codereview.chromium.org/1936393002/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1936393002/diff/40001/src/objects-inl.h#newco...
src/objects-inl.h:7588: bool JSArray::HasArrayPrototype(Isolate* isolate) {
On 2016/05/03 at 00:45:31, adamk wrote:
> You don't need to pass in the isolate, since JSArray already knows the
Isolate. Just call GetIsolate().

not sure what the performance impact is though. since in the places this is used
we have the isolate at hand i don't see it that much as a problem.

https://codereview.chromium.org/1936393002/diff/40001/src/runtime/runtime-tes...
File src/runtime/runtime-test.cc (right):

https://codereview.chromium.org/1936393002/diff/40001/src/runtime/runtime-tes...
src/runtime/runtime-test.cc:535: return
isolate->factory()->species_protector()->value();
could you make this return a bool?
returning the protector value directly is quite confusing in the tests below.

[Dan Ehrenberg, 2016-05-04 00:08:37]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-04 00:08:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1936393002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1936393002/60001

[Dan Ehrenberg, 2016-05-04 00:18:50]

https://codereview.chromium.org/1936393002/diff/40001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1936393002/diff/40001/src/builtins.cc#newcode701
src/builtins.cc:701: !array->HasArrayPrototype(isolate)) {
On 2016/05/03 at 00:45:31, adamk wrote:
> Given your claim in the CL description about the safety of comparing the
prototype, it seems like it might make more sense to inline HasArrayPrototype
here and below.

I improved the comment above HasArrayPrototype

https://codereview.chromium.org/1936393002/diff/40001/src/builtins.cc#newcode762
src/builtins.cc:762: !receiver->IsJSArray() ||
On 2016/05/03 at 00:45:31, adamk wrote:
> The old code just casted directly here, I think because the || above ensures
that this is a JSArray already.

Oops, done

https://codereview.chromium.org/1936393002/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1936393002/diff/40001/src/objects-inl.h#newco...
src/objects-inl.h:7589: return map()->prototype() ==
isolate->context()->initial_array_prototype();
On 2016/05/03 at 00:45:31, adamk wrote:
> This should be coming off the native_context(). But you should be able to just
use
> 
> GetIsolate()->initial_array_prototype()
> 
> (defined in a macro in isolates-inl.h)

Done

https://codereview.chromium.org/1936393002/diff/40001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1936393002/diff/40001/src/objects.h#newcode10178
src/objects.h:10178: MUST_USE_RESULT inline bool HasArrayPrototype(Isolate*
isolate);
On 2016/05/03 at 00:45:31, adamk wrote:
> This doesn't need MUST_USE_RESULT (that's used to denote things that could
throw).

Done

https://codereview.chromium.org/1936393002/diff/40001/src/runtime/runtime-tes...
File src/runtime/runtime-test.cc (right):

https://codereview.chromium.org/1936393002/diff/40001/src/runtime/runtime-tes...
src/runtime/runtime-test.cc:535: return
isolate->factory()->species_protector()->value();
On 2016/05/03 at 08:05:53, Camillo Bruni wrote:
> could you make this return a bool?
> returning the protector value directly is quite confusing in the tests below.

Done

[adamk, 2016-05-04 00:24:12]

This looks fine to me now, but I'd like cbruni to sign off too.

[commit-bot: I haz the power, 2016-05-04 00:33:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-04 00:33:49]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-05-04 09:21:16]

For me the CL is fine. JSArrays are not "api objects", so don't require anything
special to look at the prototype.

To solve the cross-realm perf issue, you could also look at
GetConstructor()->initial_map()->prototype() + base_is_new_target. Performance
will be slightly slower probably; you could also do both with ||.

[Toon Verwaest, 2016-05-04 16:45:33]

lgtm

[Dan Ehrenberg, 2016-05-04 16:45:46]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2016-05-04 16:45:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1936393002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1936393002/60001

[commit-bot: I haz the power, 2016-05-04 16:47:40]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-05-04 16:47:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-04 16:48:55]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/04c8c11ee569a41d4b07839154eb0c718ff6e381
Cr-Commit-Position: refs/heads/master@{#36033}
==========

[commit-bot: I haz the power, 2016-05-04 16:48:56]

Patchset 4 (id:??) landed as
https://crrev.com/04c8c11ee569a41d4b07839154eb0c718ff6e381
Cr-Commit-Position: refs/heads/master@{#36033}

[Camillo Bruni, 2016-05-11 12:50:08]

LGTM with nit

https://codereview.chromium.org/1936393002/diff/60001/test/mjsunit/harmony/ar...
File test/mjsunit/harmony/array-species-constructor.js (right):

https://codereview.chromium.org/1936393002/diff/60001/test/mjsunit/harmony/ar...
test/mjsunit/harmony/array-species-constructor.js:21:
assertFalse(%SpeciesProtector());
nit: could you also check that the SpeciesProtector is valid before you change
the constructor?

[Dan Ehrenberg, 2016-05-11 14:32:16]

https://codereview.chromium.org/1936393002/diff/60001/test/mjsunit/harmony/ar...
File test/mjsunit/harmony/array-species-constructor.js (right):

https://codereview.chromium.org/1936393002/diff/60001/test/mjsunit/harmony/ar...
test/mjsunit/harmony/array-species-constructor.js:21:
assertFalse(%SpeciesProtector());
On 2016/05/11 at 12:50:08, Camillo Bruni wrote:
> nit: could you also check that the SpeciesProtector is valid before you change
the constructor?

No because apparently 'stress' runs happen multiple times in the same isolate
and that fails.