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

src: use string_view for report and related code #46723

Merged
merged 2 commits into from
Feb 20, 2023

Conversation

addaleax
Copy link
Member

Use std::string_view for process.report code and related code, drop a few unnecessary std::to_string calls, and use MaybeStackBuffer instead of MallocedBuffer, all in order to avoid unnecessary heap allocations.

Use `std::string_view` for process.report code and related
code, drop a few unnecessary `std::to_string` calls,
and use `MaybeStackBuffer` instead of `MallocedBuffer`,
all in order to avoid unnecessary heap allocations.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/node-api

@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 18, 2023
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Feb 18, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 18, 2023
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 18, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 18, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 20, 2023
@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 20, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 20, 2023
@nodejs-github-bot nodejs-github-bot merged commit b85b5ba into nodejs:main Feb 20, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in b85b5ba

@juanarbol juanarbol added the dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. label Mar 1, 2023
@juanarbol
Copy link
Member

This depends on #46410

targos pushed a commit that referenced this pull request Mar 13, 2023
Use `std::string_view` for process.report code and related
code, drop a few unnecessary `std::to_string` calls,
and use `MaybeStackBuffer` instead of `MallocedBuffer`,
all in order to avoid unnecessary heap allocations.

PR-URL: #46723
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@anonrig
Copy link
Member

anonrig commented Apr 5, 2023

I tried backporting but it didn't land cleanly on my pull request for Ada. #47435

anonrig pushed a commit to anonrig/node that referenced this pull request Apr 6, 2023
Use `std::string_view` for process.report code and related
code, drop a few unnecessary `std::to_string` calls,
and use `MaybeStackBuffer` instead of `MallocedBuffer`,
all in order to avoid unnecessary heap allocations.

PR-URL: nodejs#46723
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@anonrig anonrig added backport-open-v18.x Indicate that the PR has an open backport. and removed dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. labels Apr 6, 2023
@anonrig
Copy link
Member

anonrig commented Apr 6, 2023

@juanarbol I removed the dont-land-on-v18.x because I opened a backport for Ada (including this PR)

anonrig pushed a commit to anonrig/node that referenced this pull request Apr 11, 2023
Use `std::string_view` for process.report code and related
code, drop a few unnecessary `std::to_string` calls,
and use `MaybeStackBuffer` instead of `MallocedBuffer`,
all in order to avoid unnecessary heap allocations.

PR-URL: nodejs#46723
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
anonrig pushed a commit to anonrig/node that referenced this pull request Apr 11, 2023
Use `std::string_view` for process.report code and related
code, drop a few unnecessary `std::to_string` calls,
and use `MaybeStackBuffer` instead of `MallocedBuffer`,
all in order to avoid unnecessary heap allocations.

PR-URL: nodejs#46723
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
danielleadams pushed a commit that referenced this pull request Apr 12, 2023
Use `std::string_view` for process.report code and related
code, drop a few unnecessary `std::to_string` calls,
and use `MaybeStackBuffer` instead of `MallocedBuffer`,
all in order to avoid unnecessary heap allocations.

PR-URL: #46723
Backport-PR-URL: #47435
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@danielleadams danielleadams added backported-to-v18.x PRs backported to the v18.x-staging branch. and removed backport-open-v18.x Indicate that the PR has an open backport. labels Apr 12, 2023
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. backported-to-v18.x PRs backported to the v18.x-staging branch. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants