Skip to content
New issue

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

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

Already on GitHub? # to your account

async_hooks: avoid unneeded AsyncResource creation #34616

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Aug 3, 2020

Inspired by the callstack at #34556 (comment)

If the wanted store is equal to the active store it's not needed to create an AsyncResource.

Refs: #34556 (comment)

fyi @vdeturckheim @puzpuzpuz @Qard

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the async_hooks Issues and PRs related to the async hooks subsystem. label Aug 3, 2020
@puzpuzpuz
Copy link
Member

@Flarna there is a conflict in lib/async_hooks.js. Could you resolve it?

Inspired by the callstack at nodejs#34556 (comment)

If the wanted store is equal to the active store it's not needed to
create an AsyncResource.

Refs: nodejs#34556 (comment)
@Flarna Flarna force-pushed the async-local-no-unneeded-resource branch from 048632c to 5eb30aa Compare August 4, 2020 08:08
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32623/

@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 4, 2020
@Flarna Flarna changed the title [async_hooks] avoid unneeded AsyncResource creation async_hooks: avoid unneeded AsyncResource creation Aug 4, 2020
Flarna added a commit that referenced this pull request Aug 6, 2020
Inspired by the callstack at #34556 (comment)

If the wanted store is equal to the active store it's not needed to
create an AsyncResource.

Refs: #34556 (comment)

PR-URL: #34616
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@Flarna
Copy link
Member Author

Flarna commented Aug 6, 2020

Landed in 4824988

@Flarna Flarna closed this Aug 6, 2020
@Flarna Flarna deleted the async-local-no-unneeded-resource branch August 6, 2020 21:28
addaleax pushed a commit that referenced this pull request Aug 8, 2020
Inspired by the callstack at #34556 (comment)

If the wanted store is equal to the active store it's not needed to
create an AsyncResource.

Refs: #34556 (comment)

PR-URL: #34616
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@codebytere codebytere mentioned this pull request Aug 10, 2020
codebytere pushed a commit that referenced this pull request Aug 11, 2020
Inspired by the callstack at #34556 (comment)

If the wanted store is equal to the active store it's not needed to
create an AsyncResource.

Refs: #34556 (comment)

PR-URL: #34616
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Inspired by the callstack at #34556 (comment)

If the wanted store is equal to the active store it's not needed to
create an AsyncResource.

Refs: #34556 (comment)

PR-URL: #34616
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants