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

node-api: create reference only when needed #44827

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Sep 29, 2022

Optimize napi_create_external() to create the reference for calling finalizer only if user actually provides a finalizer callback.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/node-api

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. labels Sep 29, 2022
@Flarna Flarna removed the needs-ci PRs that need a full CI run. label Sep 29, 2022
Optimize napi_create_external() to create the reference for calling
finalizer only if user actually provides a finalizer callback.
@Flarna Flarna changed the title n-api, src: create reference only when needed node-api: create reference only when needed Sep 30, 2022
@Flarna Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2022
@nodejs-github-bot

This comment was marked as outdated.

@Flarna Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2022
@daeyeon daeyeon added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 2, 2022
@Flarna Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2022
@Flarna Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2022
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@Flarna Flarna added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 3, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 3, 2022
@nodejs-github-bot nodejs-github-bot merged commit ecd5de0 into nodejs:main Oct 3, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in ecd5de0

@Flarna Flarna deleted the napi-external-no-ref branch October 3, 2022 17:33
panva pushed a commit to panva/node that referenced this pull request Oct 4, 2022
Optimize napi_create_external() to create the reference for calling
finalizer only if user actually provides a finalizer callback.

PR-URL: nodejs#44827
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Oct 11, 2022
Optimize napi_create_external() to create the reference for calling
finalizer only if user actually provides a finalizer callback.

PR-URL: #44827
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants