Skip to content

src: add v8::Local specialization for MaybeStackBuffer - #65159

Open
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/maybestackbuffer-v8-local
Open

src: add v8::Local specialization for MaybeStackBuffer#65159
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/maybestackbuffer-v8-local

Conversation

@jasnell

@jasnell jasnell commented Aug 9, 2026

Copy link
Copy Markdown
Member

Long-term itch. Per v8 rules, we're not supposed to be heap allocating v8::Local's; instead we're supposed to be using v8::LocalVector. Create a specialization of MaybeStackBuffer that uses either a stack array of v8::Locals or v8::LocalVector with some additional utility improvements.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

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

@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 Aug 9, 2026
@jasnell
jasnell requested a review from addaleax August 9, 2026 04:36
@jasnell
jasnell force-pushed the jasnell/maybestackbuffer-v8-local branch 4 times, most recently from 2b1fac0 to f819d7c Compare August 9, 2026 05:29
@nodejs-github-bot

This comment was marked as outdated.

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.55814% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (54ac460) to head (996535d).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/util-inl.h 72.72% 3 Missing and 3 partials ⚠️
src/js_udp_wrap.cc 0.00% 4 Missing ⚠️
src/util.h 89.28% 0 Missing and 3 partials ⚠️
src/node_dir.cc 66.66% 0 Missing and 1 partial ⚠️
src/node_env_var.cc 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65159      +/-   ##
==========================================
- Coverage   90.33%   90.31%   -0.02%     
==========================================
  Files         751      751              
  Lines      249835   249879      +44     
  Branches    47175    47192      +17     
==========================================
+ Hits       225681   225683       +2     
- Misses      15546    15587      +41     
- Partials     8608     8609       +1     
Files with missing lines Coverage Δ
src/api/callback.cc 83.17% <100.00%> (ø)
src/cares_wrap.cc 63.09% <100.00%> (ø)
src/crypto/crypto_tls.cc 78.77% <100.00%> (-0.02%) ⬇️
src/js_stream.cc 74.43% <100.00%> (-0.38%) ⬇️
src/node_http2.cc 81.76% <100.00%> (+<0.01%) ⬆️
src/node_messaging.cc 82.95% <100.00%> (ø)
src/node_messaging.h 63.15% <ø> (ø)
src/node_v8.cc 87.86% <100.00%> (ø)
src/spawn_sync.cc 68.88% <100.00%> (-0.05%) ⬇️
src/node_dir.cc 70.78% <66.66%> (-0.30%) ⬇️
... and 4 more

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jasnell

This comment was marked as resolved.

@jasnell
jasnell force-pushed the jasnell/maybestackbuffer-v8-local branch from f819d7c to abf8c83 Compare August 9, 2026 15:33
@nodejs-github-bot

This comment was marked as outdated.

@jasnell
jasnell requested a review from joyeecheung August 10, 2026 13:58
@nodejs-github-bot

This comment was marked as outdated.

@jasnell
jasnell requested a review from mcollina August 12, 2026 18:07
@jasnell
jasnell force-pushed the jasnell/maybestackbuffer-v8-local branch from abf8c83 to a052553 Compare August 12, 2026 18:46
@nodejs-github-bot

This comment was marked as outdated.

@jasnell
jasnell marked this pull request as draft August 12, 2026 19:43
@jasnell

This comment was marked as resolved.

Long-term itch. Per v8 rules, we're not supposed to be
heap allocating v8::Local's; instead we're supposed to
be using v8::LocalVector. Create a specialization of
MaybeStackBuffer that uses either a stack array of
v8::Locals or v8::LocalVector with some additional
utility improvements.

Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell
jasnell marked this pull request as ready for review August 14, 2026 02:53
@jasnell
jasnell force-pushed the jasnell/maybestackbuffer-v8-local branch from a052553 to 996535d Compare August 14, 2026 02:53
@jasnell
jasnell requested a review from Qard August 14, 2026 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants