Closed Bug 1398928 Opened 2 years ago Closed 2 years ago

Fix the argument to SpeciesConstructor


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




Tracking Status
firefox57 --- fixed


(Reporter: anba, Assigned: anba)



(Whiteboard: [qf:p3])


(1 file, 2 obsolete files)

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

And make two easy performance improvements for Promise.all/race.
Attachment #8906787 - Flags: review?(till)
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].

Attachment #8906793 - Flags: review?(till)
Priority: -- → P3
Whiteboard: [qf:p3]
Comment on attachment 8906787 [details] [diff] [review]

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

Huh, yes. Thank you!
Attachment #8906787 - Flags: review?(till) → review+
Comment on attachment 8906793 [details] [diff] [review]

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.
Attachment #8906793 - Flags: review?(till) → review+
Blocks: 1401508
Attached patch bug1398928.patchSplinter Review
Drop "Part 1" from commit message, part 2 will be handled in bug 1401508. No change in behaviour, therefore carrying r+ from Till.
Attachment #8906787 - Attachment is obsolete: true
Attachment #8910229 - Flags: review+
Comment on attachment 8906793 [details] [diff] [review]

Part 2 gets moved to bug 1401508.
Attachment #8906793 - Attachment is obsolete: true
Pushed by
Pass correct argument to SpeciesConstructor call. r=till
Keywords: checkin-needed
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.