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

[v9.x backport] backport defaultTriggerAsyncIdScope changes #18474

Closed
wants to merge 3 commits into from
Closed

[v9.x backport] backport defaultTriggerAsyncIdScope changes #18474

wants to merge 3 commits into from

Conversation

AndreasMadsen
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

async_hooks

apapirovski and others added 3 commits January 30, 2018 17:58
The existing version of defaultTriggerAsyncIdScope creates an Array
for the callback's arguments which is highly inefficient. Instead,
use rest syntax and allow V8 to do that work for us. This yields
roughly 2x performance for this particular function.

PR-URL: #18004
Reviewed-By: Andreas Madsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
This allows more easy tracking of where HTTP requests come from. Before
this change the HTTPParser would have the HTTPServer as the
triggerAsyncId.

The HTTPParser will still have the executionAsyncId set to the HTTP
Server so that information is still directly available. Indirectly, the
TCP socket itself also has its triggerAsyncId set to the TCP Server.

PR-URL: #18003
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.

PR-URL: #18143
Fixes: #18003
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. http Issues or PRs related to the http subsystem. net Issues and PRs related to the net subsystem. v9.x labels Jan 31, 2018
@AndreasMadsen
Copy link
Member Author

evanlucas pushed a commit that referenced this pull request Jan 31, 2018
The existing version of defaultTriggerAsyncIdScope creates an Array
for the callback's arguments which is highly inefficient. Instead,
use rest syntax and allow V8 to do that work for us. This yields
roughly 2x performance for this particular function.

Backport-PR-URL: #18474
PR-URL: #18004
Reviewed-By: Andreas Madsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 31, 2018
This allows more easy tracking of where HTTP requests come from. Before
this change the HTTPParser would have the HTTPServer as the
triggerAsyncId.

The HTTPParser will still have the executionAsyncId set to the HTTP
Server so that information is still directly available. Indirectly, the
TCP socket itself also has its triggerAsyncId set to the TCP Server.

Backport-PR-URL: #18474
PR-URL: #18003
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 31, 2018
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.

Backport-PR-URL: #18474
PR-URL: #18143
Fixes: #18003
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@AndreasMadsen
Copy link
Member Author

Landed in 0a68018, ac2f98d, e9397d6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. http Issues or PRs related to the http subsystem. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants