Skip to content

src: make the options structs smaller with packed bits - #65145

Open
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/make-options-structs-smaller
Open

src: make the options structs smaller with packed bits#65145
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/make-options-structs-smaller

Conversation

@jasnell

@jasnell jasnell commented Aug 8, 2026

Copy link
Copy Markdown
Member

While the Options objects are not a significant overhead, they still waste a few hundred bytes due to sloppy definition and alignment issues. Tighten it up and shave off a couple hundred wasted bytes.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config

@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 8, 2026
@jasnell
jasnell force-pushed the jasnell/make-options-structs-smaller branch from 5d00d67 to 1039041 Compare August 8, 2026 21:24
@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.26962% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.32%. Comparing base (404b0cf) to head (25d1a1a).
⚠️ Report is 45 commits behind head on main.

Files with missing lines Patch % Lines
src/node_options.h 96.07% 6 Missing ⚠️
src/node_options-inl.h 86.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65145      +/-   ##
==========================================
- Coverage   90.32%   90.32%   -0.01%     
==========================================
  Files         760      760              
  Lines      248525   248653     +128     
  Branches    46894    46889       -5     
==========================================
+ Hits       224488   224587      +99     
- Misses      15469    15475       +6     
- Partials     8568     8591      +23     
Files with missing lines Coverage Δ
src/node_options.cc 79.41% <100.00%> (+2.17%) ⬆️
src/node_options-inl.h 83.27% <86.66%> (-0.44%) ⬇️
src/node_options.h 95.32% <96.07%> (-2.71%) ⬇️

... and 35 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.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

nodejs-github-bot commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

@jasnell
jasnell requested review from addaleax and lemire August 9, 2026 15:24
While the Options objects are not a significant overhead,
they still waste a few hundred bytes due to sloppy definition
and alignment issues. Tighten it up and shave off a couple
hundred wasted bytes.

Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell
jasnell force-pushed the jasnell/make-options-structs-smaller branch from 1039041 to 25d1a1a Compare August 10, 2026 13:53
@nodejs-github-bot

nodejs-github-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@jasnell
jasnell requested review from Qard and mcollina August 12, 2026 16:01

@Qard Qard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally LGTM. Just with a question:

Is the EnvironmentOptions rearrangement an ABI break? Or is that private? I forget if that is exposed is any way in the embedding API.

@jasnell

jasnell commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

If that's even a minor possibility I've no problem making this a semver-major... Ultimately, however, I don't believe it's an issue. There's no direct use of node_options.h from node.h

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

4 participants