1398928 - Fix the argument to SpeciesConstructor

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, enhancement, P3)

Description

[André Bargull [:anba]]

because I didn't spot in while reviewing bug 1386534.

And make two easy performance improvements for Promise.all/race.

Comment 1

[André Bargull [:anba]]

Attachment: bug1398928-part1-species-constructor-arg.patch

Comment 2

[André Bargull [:anba]]

Attachment: bug1398928-part2-perf-changes.patch

We may not be able to skip the creation of the resolving functions, but we can still skip creating the GetCapabilitiesExecutor function for the common case when the constructor argument is the built-in Promise constructor in Promise.race/all.

I've changed the arguments of CreateResolvingFunctions from Value types to Object types, so it's more easy to directly call this function from the new CreatePromiseWithDefaultResolutionFunctions method. 

And I've also changed the GetProperty(...) calls to no longer go through the Interpreter's GetProperty(...) function, because that allows us to skip this code [1].

[1] http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/js/src/vm/Interpreter.cpp#4394-4419

Comment 3

[Till Schneidereit [:till]]

Comment on attachment 8906787 [details] [diff] [review]
bug1398928-part1-species-constructor-arg.patch

Review of attachment 8906787 [details] [diff] [review]:
-----------------------------------------------------------------

Huh, yes. Thank you!

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 8906793 [details] [diff] [review]
bug1398928-part2-perf-changes.patch

Review of attachment 8906793 [details] [diff] [review]:
-----------------------------------------------------------------

Very nice! This is indeed an easy perf win that I hadn't thought of.

Given that we're solidly in the stabilization phase for 57, I think it'd be good to split this into a new bug and land it after the 57 branch, but still land part 1 here in time for 57.

Comment 5

[André Bargull [:anba]]

Attachment: bug1398928.patch

Drop "Part 1" from commit message, part 2 will be handled in bug 1401508. No change in behaviour, therefore carrying r+ from Till.

Comment 6

[André Bargull [:anba]]

Comment on attachment 8906793 [details] [diff] [review]
bug1398928-part2-perf-changes.patch

Part 2 gets moved to bug 1401508.

Comment 7

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=870329d65fc96d37befc13269ffca15d95693ef1

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7aa9a38d74ba
Pass correct argument to SpeciesConstructor call. r=till

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7aa9a38d74ba