From 16a5d6e27c6a02f5ef63cf5fa4d5fbb913ac2d21 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Sun, 2 Aug 2026 23:08:03 +0000 Subject: [PATCH] ffi: reuse the callable created per symbol 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 --- doc/api/ffi.md | 4 ++- lib/ffi.js | 37 +++++++++++++++++-------- src/node_ffi.cc | 28 +++++++++++++++++++ src/node_ffi.h | 6 +++++ test/ffi/test-ffi-dynamic-library.js | 40 ++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 12 deletions(-) diff --git a/doc/api/ffi.md b/doc/api/ffi.md index e7e5f74e23c3..634afa757e2b 100644 --- a/doc/api/ffi.md +++ b/doc/api/ffi.md @@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function address as a `bigint`. If the same symbol has already been resolved, requesting it again with a -different signature throws. +different signature throws. Requesting it again with the same signature returns +the same function, as does reading it from [`library.functions`][]. ```cjs const { DynamicLibrary, suffix } = require('node:ffi'); @@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side. [Permission Model]: permissions.md#permission-model [`--allow-ffi`]: cli.md#--allow-ffi [`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy +[`library.functions`]: #libraryfunctions [`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations [type names]: #type-names diff --git a/lib/ffi.js b/lib/ffi.js index cde6cca7a86e..edb12ed4ed45 100644 --- a/lib/ffi.js +++ b/lib/ffi.js @@ -7,6 +7,8 @@ const { ObjectGetOwnPropertyDescriptor, ObjectKeys, ObjectPrototypeToString, + SafeWeakMap, + SafeWeakRef, SymbolDispose, } = primordials; const { Buffer } = require('buffer'); @@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) { }; } +// The native layer hands out one raw function per resolved symbol, so the +// wrapper composed around it is reused too, otherwise every read of +// `library.functions` would return callables that are not identical to the +// previous read's. The entry holds a WeakRef because V8 can keep a raw function +// alive after user code drops the wrapper, and a strong value would then pin +// every wrapper for the lifetime of the library. +const wrappedFunctions = new SafeWeakMap(); + function wrapFFIFunction(rawFn, owner) { - let argumentTypes; + if (rawFn === undefined || rawFn === null) { + return rawFn; + } + const cached = wrappedFunctions.get(rawFn)?.deref(); + if (cached !== undefined) { + return cached; + } let returnType; - if (rawFn !== undefined && rawFn !== null) { - const sbArguments = rawFn[kSbArguments]; - argumentTypes = sbArguments ?? rawFn[kFastArguments]; - if (sbArguments !== undefined) { - returnType = rawFn[kSbReturn]; - } + const sbArguments = rawFn[kSbArguments]; + const argumentTypes = sbArguments ?? rawFn[kFastArguments]; + if (sbArguments !== undefined) { + returnType = rawFn[kSbReturn]; } - const wrapped = wrapWithSharedBuffer( + let wrapped = wrapWithSharedBuffer( rawFn, argumentTypes === undefined ? undefined : makeSignature(argumentTypes, returnType)); - if (wrapped !== rawFn) { - return wrapped; + if (wrapped === rawFn) { + wrapped = wrapWithRawPointerConversions(rawFn, argumentTypes, owner); } - return wrapWithRawPointerConversions(rawFn, argumentTypes, owner); + wrappedFunctions.set(rawFn, new SafeWeakRef(wrapped)); + return wrapped; } const rawGetFunction = DynamicLibrary.prototype.getFunction; diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 6cce5e38e8e3..da0b8d2bac2b 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -72,6 +72,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackFieldWithSize( "symbols", symbols_size, "std::unordered_map"); + tracker->TrackFieldWithSize( + "function_wrappers", + function_wrappers_.size() * + sizeof(decltype(function_wrappers_)::value_type), + "std::unordered_map>"); + // FFIFunctionInfo instances and their sb_backing ArrayBuffers are // owned by V8 function wrappers and reachable only via weak references, // so they are deliberately not counted here. @@ -97,6 +103,7 @@ void DynamicLibrary::Close() { symbols_.clear(); functions_.clear(); + function_wrappers_.clear(); callbacks_.clear(); } @@ -242,6 +249,19 @@ MaybeLocal DynamicLibrary::CreateFunction( Isolate* isolate = env->isolate(); Local context = env->context(); + // Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and + // on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already + // handed out for this symbol. `PrepareFunction()` rejects a request that uses + // a different signature, so a hit always describes the same signature. An + // empty handle means the wrapper was collected; fall through and rebuild. + auto cached = function_wrappers_.find(name); + if (cached != function_wrappers_.end()) { + if (!cached->second.IsEmpty()) { + return cached->second.Get(isolate); + } + function_wrappers_.erase(cached); + } + auto info = FFIFunctionInfo::Create(env, fn, this); DCHECK_EQ(fn->args.size(), fn->arg_type_names.size()); @@ -437,6 +457,14 @@ MaybeLocal DynamicLibrary::CreateFunction( } } + // A strong handle would root the callable, which holds the library object + // through FFIFunctionInfo, so neither could ever be collected. Weaken the + // stored handle instead, so the cache lasts exactly as long as user code + // keeps a reference. SetWeak() runs after the move into the map because + // moving a handle relocates the underlying slot. + function_wrappers_.emplace(name, Global(isolate, ret)) + .first->second.SetWeak(); + return ret; } diff --git a/src/node_ffi.h b/src/node_ffi.h index a55cb74fc619..3e823134e166 100644 --- a/src/node_ffi.h +++ b/src/node_ffi.h @@ -148,6 +148,12 @@ class DynamicLibrary : public BaseObject { std::string path_; std::unordered_map symbols_; std::unordered_map> functions_; + // Callables created for `functions_`, so repeated resolution of the same + // symbol reuses one wrapper instead of emitting another trampoline. The + // handles are weak: an entry disappears once user code drops the wrapper, + // which keeps the map from rooting the library through the wrapper's + // FFIFunctionInfo. + std::unordered_map> function_wrappers_; std::unordered_map> callbacks_; }; diff --git a/test/ffi/test-ffi-dynamic-library.js b/test/ffi/test-ffi-dynamic-library.js index 2a6aa4129e95..82400335a12f 100644 --- a/test/ffi/test-ffi-dynamic-library.js +++ b/test/ffi/test-ffi-dynamic-library.js @@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => { } }); +test('resolving the same symbol reuses one function', () => { + const lib = new ffi.DynamicLibrary(libraryPath); + const definitions = { add_i32: fixtureSymbols.add_i32 }; + + try { + // Every resolution used to build a new callable, allocating another + // trampoline and making `lib.functions.add_i32` a different function on + // each read. + const fn = lib.getFunction('add_i32', fixtureSymbols.add_i32); + assert.strictEqual(lib.getFunction('add_i32', fixtureSymbols.add_i32), fn); + assert.strictEqual(lib.functions.add_i32, fn); + assert.strictEqual(lib.getFunctions().add_i32, fn); + assert.strictEqual(lib.getFunctions(definitions).add_i32, fn); + assert.strictEqual(fn(20, 22), 42); + } finally { + lib.close(); + } +}); + +test('a dropped function wrapper is collectable', async () => { + const lib = new ffi.DynamicLibrary(libraryPath); + + try { + // Caching the wrapper must not pin it, so that dropping the last user + // reference still releases the wrapper and the trampoline it owns. + let fn = lib.getFunction('add_i32', fixtureSymbols.add_i32); + const ref = new WeakRef(fn); + fn = null; + + await gcUntil('a dropped function wrapper is collectable', () => { + return ref.deref() === undefined; + }); + + fn = lib.getFunction('add_i32', fixtureSymbols.add_i32); + assert.strictEqual(fn(20, 22), 42); + } finally { + lib.close(); + } +}); + test('FFI functions keep their owning library alive', async () => { let lib = new ffi.DynamicLibrary(libraryPath); const addI32 = lib.getFunction('add_i32', fixtureSymbols.add_i32);