Skip to content

ffi: reuse the callable created per symbol - #64971

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-get-function-reuse-wrapper
Open

ffi: reuse the callable created per symbol#64971
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-get-function-reuse-wrapper

Conversation

@trivikr

@trivikr trivikr commented Aug 2, 2026

Copy link
Copy Markdown
Member

Fixes: #64970

CreateFunction() ran on every getFunction() call, every getFunctions() call, and every read of the functions accessor, each time emitting a trampoline, allocating an FFIFunctionInfo, and on the SharedBuffer path an ArrayBuffer. lib.functions.foo was therefore a different function on each read, and calling through the accessor in a loop leaked a page per iteration until GC: 20000 calls grew RSS by 58 MiB.

Cache the created callable per symbol in function_wrappers_, and memoize the JS wrapper composed around it. Both entries are weak, so dropping the last user reference still releases the wrapper and its trampoline. The JS side stores a WeakRef because V8 can keep a raw function alive after the wrapper is gone, and a strong value would pin every wrapper for the lifetime of the library.

Before

same wrapper on two reads: false
sum: 840000
rss growth: 56.0 MiB

After

same wrapper on two reads: true
sum: 840000
rss growth: 9.8 MiB

Assisted-by: claude:opus-5

CreateFunction() ran on every getFunction() call, every getFunctions()
call, and every read of the functions accessor, each time emitting a
trampoline, allocating an FFIFunctionInfo, and on the SharedBuffer path
an ArrayBuffer. lib.functions.foo was therefore a different function on
each read, and calling through the accessor in a loop leaked a page per
iteration until GC: 20000 calls grew RSS by 58 MiB.

Cache the created callable per symbol in function_wrappers_, and
memoize the JS wrapper composed around it. Both entries are weak, so
dropping the last user reference still releases the wrapper and its
trampoline. The JS side stores a WeakRef because V8 can keep a raw
function alive after the wrapper is gone, and a strong value would pin
every wrapper for the lifetime of the library.

Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Assisted-by: claude:opus-5
@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++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run. labels Aug 2, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2026
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.37838% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.28%. Comparing base (f43086d) to head (16a5d6e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/node_ffi.cc 54.54% 4 Missing and 1 partial ⚠️
lib/ffi.js 88.46% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #64971   +/-   ##
=======================================
  Coverage   90.27%   90.28%           
=======================================
  Files         762      762           
  Lines      247515   247541   +26     
  Branches    46683    46688    +5     
=======================================
+ Hits       223447   223492   +45     
- Misses      15485    15491    +6     
+ Partials     8583     8558   -25     
Files with missing lines Coverage Δ
src/node_ffi.h 61.53% <ø> (ø)
lib/ffi.js 94.62% <88.46%> (-0.62%) ⬇️
src/node_ffi.cc 70.70% <54.54%> (+0.02%) ⬆️

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

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 3, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 3, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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++. 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.

ffi: getFunction() and the functions accessor rebuild wrappers on every call

3 participants