Skip to content

src: make a number of minor improvements to buffer #58377

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented May 18, 2025

When using Buffer::New with an externally allocated buffer and v8 sandbox enabled, defer to copy instead of wrapping the buffer.

Also, when immediately memcpy'ing into the full buffer, allocate the buffer with kUninitialized as a slight perf optimization.

@jasnell jasnell requested review from anonrig and joyeecheung May 18, 2025 18:47
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/http2
  • @nodejs/net
  • @nodejs/sqlite

@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 May 18, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

This comment was marked as outdated.

When using Buffer::New with an externally allocated buffer and
v8 sandbox enabled, defer to copy instead of wrapping the buffer.

Also, when immediately memcpy'ing into the full buffer, allocate
the buffer with kUninitialized as a slight perf optimization.
@jasnell jasnell force-pushed the jasnell/minor-buffer-improvements branch from 97f4652 to f099b34 Compare May 19, 2025 00:18
@jasnell jasnell requested review from legendecas and anonrig May 19, 2025 00:18
@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label May 19, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 19, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 19, 2025
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++. 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.

6 participants