Skip to content

[v10.x] lib,src: replace all C++ promises with JS promises#21879

Closed
targos wants to merge 1 commit intonodejs:v10.x-stagingfrom
targos:backport-20830
Closed

[v10.x] lib,src: replace all C++ promises with JS promises#21879
targos wants to merge 1 commit intonodejs:v10.x-stagingfrom
targos:backport-20830

Conversation

@targos
Copy link
Copy Markdown
Member

@targos targos commented Jul 19, 2018 •
edited
Loading

Backport of #20830
/cc @BridgeAR
@BridgeAR @targos
lib,src: replace all C++ promises with JS promises
e637159 
C++ promises can not be properly optimized by V8. They also behave
a tiny bit different than "regular" promises.

PR-URL: nodejs#20830
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Jul 19, 2018

@targos build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/312/pipeline
@targos targos added promises Issues and PRs related to ECMAScript promises. v10.x and removed promises Issues and PRs related to ECMAScript promises. labels Jul 19, 2018
@targos
Copy link
Copy Markdown
Member Author

targos commented Jul 19, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/15942/
@targos
Copy link
Copy Markdown
Member Author

targos commented Jul 20, 2018

@TimothyGu @devsnek @benjamingr
@benjamingr
Copy link
Copy Markdown
Member

benjamingr commented Jul 21, 2018

@targos can we be sure there is no performance degradation with this backport to older V8?
@targos
Copy link
Copy Markdown
Member Author

targos commented Jul 21, 2018

@benjamingr The V8 in 10.x is the same as master.
@targos
Copy link
Copy Markdown
Member Author

targos commented Jul 24, 2018

Ping
targos pushed a commit that referenced this pull request Jul 24, 2018
@BridgeAR @targos
lib,src: replace all C++ promises with JS promises
9817e40 
C++ promises can not be properly optimized by V8. They also behave
a tiny bit different than "regular" promises.

PR-URL: #20830
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21879
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@targos
Copy link
Copy Markdown
Member Author

targos commented Jul 24, 2018

Landed in 9817e40
@targos targos closed this Jul 24, 2018
@targos targos deleted the backport-20830 branch July 24, 2018 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@targos @nodejs-github-bot @benjamingr @BridgeAR