Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v8.x backport] crypto: make createXYZ inlineable #16446

Closed
wants to merge 1 commit into from

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Oct 24, 2017

This is a backport of #16067 as requested.

cc: @MylesBorins

This commit increase by around 10% hot code paths that are hitting
createXYZ functions. Before this change the createXYZ called the XYZ
constructor without new.

PR-URL: nodejs#16067
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yosuke Furukawa <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Benedikt Meurer <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. v8.x labels Oct 24, 2017
@lpinca lpinca added the blocked PRs that are blocked by other issues or PRs. label Oct 24, 2017
@lpinca lpinca mentioned this pull request Oct 24, 2017
3 tasks
@lpinca lpinca removed the blocked PRs that are blocked by other issues or PRs. label Oct 24, 2017
@gibfahn gibfahn force-pushed the v8.x-staging branch 5 times, most recently from b183192 to fc8acc8 Compare October 30, 2017 21:42
@gibfahn gibfahn added the baking-for-lts PRs that need to wait before landing in a LTS release. label Oct 30, 2017
@gibfahn
Copy link
Member

gibfahn commented Oct 30, 2017

Leaving this to bake for a while in 8.x sounds good to me (#16067 (comment))

Unfortunately that probably means this will have conflicts at some point. @lpinca don't worry about rebasing until someone pings you 😁 .

@gibfahn gibfahn force-pushed the v8.x-staging branch 2 times, most recently from 97c2301 to ab0d7a6 Compare October 31, 2017 00:16
@lpinca
Copy link
Member Author

lpinca commented Dec 19, 2017

Ping @nodejs/lts. I think this has baked long enough, probably burnt by now :)

@gibfahn
Copy link
Member

gibfahn commented Dec 19, 2017

Thanks for the ping @lpinca

CI: https://ci.nodejs.org/job/node-test-commit/14953/

gibfahn pushed a commit that referenced this pull request Dec 19, 2017
This commit increase by around 10% hot code paths that are hitting
createXYZ functions. Before this change the createXYZ called the XYZ
constructor without new.

PR-URL: #16067
Backport-PR-URL: #16446
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yosuke Furukawa <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Benedikt Meurer <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@gibfahn
Copy link
Member

gibfahn commented Dec 19, 2017

Landed in 64164c7

@gibfahn gibfahn closed this Dec 19, 2017
@lpinca lpinca deleted the backport/16067 branch December 19, 2017 19:05
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
This commit increase by around 10% hot code paths that are hitting
createXYZ functions. Before this change the createXYZ called the XYZ
constructor without new.

PR-URL: #16067
Backport-PR-URL: #16446
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yosuke Furukawa <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Benedikt Meurer <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants