Skip to content

ffi: allocate string argument storage lazily - #66178

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-lazy-string-storage
Open

soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-lazy-string-storage

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

InvokeFunction reserves a temporary std::vector<std::string> on every call with arguments, even when none of the arguments are strings. Defer that reservation until the first string argument needs to be copied, avoiding an allocation and deallocation for calls without string arguments.

The first string still reserves capacity for the full argument count before its pointer is saved. Subsequent strings therefore cannot reallocate the vector and invalidate pointers already prepared for the native call.

Add a test that exercises native argument conversion with multiple strings, empty and long strings, Unicode, mixed string/Buffer arguments, and errors while converting a later argument.

Local macOS ARM64 comparison using benchmark/compare.js, with 10 samples per binary and the default iteration counts across all 40 FFI benchmark configurations:

                                                              confidence   improvement   accuracy (*)    (**)   (***)
ffi/many-args.js n=10000000                                   ***             +5.97 %   ±2.33%  ±3.26%  ±4.60%
ffi/sum-5-i32.js n=10000000                                   ***            +11.18 %   ±2.49%  ±3.42%  ±4.67%

Both improvements remained significant at the 5% level after the runner's Holm-Bonferroni correction. The other 38 configurations showed no statistically significant difference in this run.

Validation:

  • Release build with Ninja (--without-node-snapshot --without-node-code-cache).
  • All 16 FFI test files passed, including the new test.
  • C++ lint, import checks, and ESLint for the new test passed.

The full Node.js test suite and cross-platform CI have not been run.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Sep 21, 2026
Reserve temporary string storage in InvokeFunction only when the
first string argument is encountered. Calls without string arguments
avoid allocating and freeing an unused vector buffer.

Reserve capacity for all arguments before saving the first string
pointer so subsequent strings cannot invalidate it. Add coverage for
multiple strings, mixed Buffer arguments, and errors during argument
conversion.

Signed-off-by: Soul Lee <alus20x@gmail.com>
@soulee-dev
soulee-dev force-pushed the ffi-lazy-string-storage branch from c85110b to aa0c8b5 Compare September 21, 2026 11:57
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (97af3d7) to head (aa0c8b5).
⚠️ Report is 41 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66178      +/-   ##
==========================================
- Coverage   90.29%   90.28%   -0.01%     
==========================================
  Files         790      790              
  Lines      272021   272045      +24     
  Branches    51917    51936      +19     
==========================================
+ Hits       245611   245612       +1     
- Misses      16903    16941      +38     
+ Partials     9507     9492      -15     
Files with missing lines Coverage Δ
src/node_ffi.cc 72.40% <100.00%> (+0.41%) ⬆️

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

@daeyeon daeyeon added the ffi Issues and PRs related to experimental Foreign Function Interface support. label Sep 22, 2026
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++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants