Skip to content

src,ffi: create fast-call metadata Symbols lazily - #66015

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:fix-ffi-fast-api-snapshot-symbols
Sep 15, 2026
Merged

nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:fix-ffi-fast-api-snapshot-symbols

Conversation

@mcollina

Copy link
Copy Markdown
Member

The FFI fast-call wrappers key per-function metadata on raw FFI functions using two per-isolate Symbols (kFastArguments / kFastBufferInvoke) that were declared in src/env_properties.h. Everything in env_properties.h is allocated while the startup snapshot is built, so each Symbol advances the isolate's identity-hash RNG before Object.prototype / Function.prototype receive their snapshot identity hashes. In the snapshot produced for Node 26.4.0+ this shifted those hashes so a function map (a function whose length was redefined) and a plain-object map (an object literal with an accessor) collide in V8's 64-slot NormalizedMapCache. Every store into such objects then misses the inline cache, and the repro reported in the linked issue is roughly 7x slower.

Create the two Symbols lazily in the FFI binding's Initialize, on the first run of internalBinding('ffi') at runtime, instead of declaring them in env_properties.h. They are therefore not allocated during snapshot serialization and no longer bias the snapshot's prototype identity hashes. Their export, property layout, and the fast-call feature behavior are unchanged.

Fixes #66011


Ai generated, humanly reviewed.

@mcollina
mcollina requested a review from jasnell September 13, 2026 12:28
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@mcollina mcollina changed the title src: ffi: create fast-call metadata Symbols lazily src,ffi: create fast-call metadata Symbols lazily Sep 13, 2026
@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 13, 2026
@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.21%. Comparing base (7aaf9b4) to head (ac480bc).
⚠️ Report is 425 commits behind head on main.

Files with missing lines Patch % Lines
src/env-inl.h 90.90% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66015      +/-   ##
==========================================
+ Coverage   90.13%   90.21%   +0.07%     
==========================================
  Files         751      785      +34     
  Lines      253639   269701   +16062     
  Branches    47790    51585    +3795     
==========================================
+ Hits       228618   243301   +14683     
- Misses      16264    16878     +614     
- Partials     8757     9522     +765     
Files with missing lines Coverage Δ
src/env.h 97.26% <ø> (-0.96%) ⬇️
src/node_ffi.cc 72.38% <100.00%> (+0.65%) ⬆️
src/env-inl.h 93.92% <90.90%> (-1.11%) ⬇️

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

@Renegade334

Renegade334 commented Sep 13, 2026

Copy link
Copy Markdown
Member

Is this just kicking the can down the road? Is this behaviour going to be reintroduced some other time someone decides to define a new environment symbol?

@mcollina

Copy link
Copy Markdown
Member Author

Is this just kicking the can down the road? Is this behaviour going to be reintroduced some other time someone decides to define a new environment symbol?

Yes. However we can't assume when the V8 fix is going to land, and this seems better than reverting the ffi change.

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 13, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 13, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

The FFI fast-call wrappers key per-function metadata on raw FFI
functions using two per-isolate Symbols (kFastArguments /
kFastBufferInvoke) that were declared in src/env_properties.h.
Everything in env_properties.h is allocated while the startup
snapshot is built, so each Symbol advances the isolate's
identity-hash RNG before Object.prototype / Function.prototype
receive their snapshot identity hashes. In the snapshot produced
for Node 26.4.0+ this shifted those hashes so a function map (a
function whose `length` was redefined) and a plain-object map (an
object literal with an accessor) collide in V8's 64-slot
NormalizedMapCache. Every store into such objects then misses
the inline cache, and the repro reported in the linked issue is
roughly 7x slower.

Create the two Symbols lazily in the FFI binding's Initialize, on
the first run of internalBinding('ffi') at runtime, instead of
declaring them in env_properties.h. They are therefore not
allocated during snapshot serialization and no longer bias the
snapshot's prototype identity hashes. Their export, property
layout, and the fast-call feature behavior are unchanged.

Refs: nodejs#66011

Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
Assisted-by: Pi
@mcollina
mcollina force-pushed the fix-ffi-fast-api-snapshot-symbols branch from 9661374 to ac480bc Compare September 14, 2026 19:36
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 15, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 15, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina mcollina added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 15, 2026
@nodejs-github-bot
nodejs-github-bot merged commit cfdb7e6 into nodejs:main Sep 15, 2026
72 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in cfdb7e6

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 15, 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++. needs-ci PRs that need a full CI run.

Projects

None yet

6 participants