Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 3e6688c

Browse files
trivikraduh95
authored andcommitted
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 PR-URL: #64971Fixes: #64970 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 8b0bdd9 commit 3e6688c

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

β€Ždoc/api/ffi.mdβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ The returned function has a `.pointer` property containing the native function
363363
address as a `bigint`.
364364

365365
If the same symbol has already been resolved, requesting it again with a
366-
different signature throws.
366+
different signature throws. Requesting it again with the same signature returns
367+
the same function, as does reading it from [`library.functions`][].
367368

368369
```cjs
369370
const { DynamicLibrary, suffix } =require('node:ffi');
@@ -766,5 +767,6 @@ and keep callback and pointer lifetimes explicit on the native side.
766767
[Permission Model]: permissions.md#permission-model
767768
[`--allow-ffi`]: cli.md#--allow-ffi
768769
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
770+
[`library.functions`]: #libraryfunctions
769771
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
770772
[type names]: #type-names

β€Žlib/ffi.jsβ€Ž

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
ObjectGetOwnPropertyDescriptor,
88
ObjectKeys,
99
ObjectPrototypeToString,
10+
SafeWeakMap,
11+
SafeWeakRef,
1012
SymbolDispose,
1113
}=primordials;
1214
const{ Buffer }=require('buffer');
@@ -80,23 +82,36 @@ function makeSignature(argumentTypes, returnType) {
8082
};
8183
}
8284

85+
// The native layer hands out one raw function per resolved symbol, so the
86+
// wrapper composed around it is reused too, otherwise every read of
87+
// `library.functions` would return callables that are not identical to the
88+
// previous read's. The entry holds a WeakRef because V8 can keep a raw function
89+
// alive after user code drops the wrapper, and a strong value would then pin
90+
// every wrapper for the lifetime of the library.
91+
constwrappedFunctions=newSafeWeakMap();
92+
8393
functionwrapFFIFunction(rawFn,owner){
84-
letargumentTypes;
94+
if(rawFn===undefined||rawFn===null){
95+
returnrawFn;
96+
}
97+
constcached=wrappedFunctions.get(rawFn)?.deref();
98+
if(cached!==undefined){
99+
returncached;
100+
}
85101
letreturnType;
86-
if(rawFn!==undefined&&rawFn!==null){
87-
constsbArguments=rawFn[kSbArguments];
88-
argumentTypes=sbArguments??rawFn[kFastArguments];
89-
if(sbArguments!==undefined){
90-
returnType=rawFn[kSbReturn];
91-
}
102+
constsbArguments=rawFn[kSbArguments];
103+
constargumentTypes=sbArguments??rawFn[kFastArguments];
104+
if(sbArguments!==undefined){
105+
returnType=rawFn[kSbReturn];
92106
}
93-
constwrapped=wrapWithSharedBuffer(
107+
letwrapped=wrapWithSharedBuffer(
94108
rawFn,
95109
argumentTypes===undefined ? undefined : makeSignature(argumentTypes,returnType));
96-
if(wrapped!==rawFn){
97-
returnwrapped;
110+
if(wrapped===rawFn){
111+
wrapped=wrapWithRawPointerConversions(rawFn,argumentTypes,owner);
98112
}
99-
returnwrapWithRawPointerConversions(rawFn,argumentTypes,owner);
113+
wrappedFunctions.set(rawFn,newSafeWeakRef(wrapped));
114+
returnwrapped;
100115
}
101116

102117
constrawGetFunction=DynamicLibrary.prototype.getFunction;

β€Žsrc/node_ffi.ccβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
8383
tracker->TrackFieldWithSize(
8484
"symbols", symbols_size, "std::unordered_map<std::string, void*>");
8585

86+
tracker->TrackFieldWithSize(
87+
"function_wrappers",
88+
function_wrappers_.size() *
89+
sizeof(decltype(function_wrappers_)::value_type),
90+
"std::unordered_map<std::string, v8::Global<v8::Function>>");
91+
8692
// FFIFunctionInfo instances and their sb_backing ArrayBuffers are
8793
// owned by V8 function wrappers and reachable only via weak references,
8894
// so they are deliberately not counted here.
@@ -108,6 +114,7 @@ void DynamicLibrary::Close() {
108114

109115
symbols_.clear();
110116
functions_.clear();
117+
function_wrappers_.clear();
111118
callbacks_.clear();
112119
}
113120

@@ -259,6 +266,19 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
259266
Isolate* isolate = env->isolate();
260267
Local<Context> context = env->context();
261268

269+
// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
270+
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
271+
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
272+
// a different signature, so a hit always describes the same signature. An
273+
// empty handle means the wrapper was collected; fall through and rebuild.
274+
auto cached = function_wrappers_.find(name);
275+
if (cached != function_wrappers_.end()) {
276+
if (!cached->second.IsEmpty()) {
277+
return cached->second.Get(isolate);
278+
}
279+
function_wrappers_.erase(cached);
280+
}
281+
262282
auto info = FFIFunctionInfo::Create(env, fn, this);
263283

264284
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
@@ -454,6 +474,14 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
454474
}
455475
}
456476

477+
// A strong handle would root the callable, which holds the library object
478+
// through FFIFunctionInfo, so neither could ever be collected. Weaken the
479+
// stored handle instead, so the cache lasts exactly as long as user code
480+
// keeps a reference. SetWeak() runs after the move into the map because
481+
// moving a handle relocates the underlying slot.
482+
function_wrappers_.emplace(name, Global<Function>(isolate, ret))
483+
.first->second.SetWeak();
484+
457485
return ret;
458486
}
459487

β€Žsrc/node_ffi.hβ€Ž

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ class DynamicLibrary : public BaseObject {
169169
std::string path_;
170170
std::unordered_map<std::string, void*> symbols_;
171171
std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;
172+
// Callables created for `functions_`, so repeated resolution of the same
173+
// symbol reuses one wrapper instead of emitting another trampoline. The
174+
// handles are weak: an entry disappears once user code drops the wrapper,
175+
// which keeps the map from rooting the library through the wrapper's
176+
// FFIFunctionInfo.
177+
std::unordered_map<std::string, v8::Global<v8::Function>> function_wrappers_;
172178
std::unordered_map<void*, std::unique_ptr<FFICallback>> callbacks_;
173179
};
174180

β€Žtest/ffi/test-ffi-dynamic-library.jsβ€Ž

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ test('getFunction caches signatures consistently', () => {
176176
}
177177
});
178178

179+
test('resolving the same symbol reuses one function',()=>{
180+
constlib=newffi.DynamicLibrary(libraryPath);
181+
constdefinitions={add_i32: fixtureSymbols.add_i32};
182+
183+
try{
184+
// Every resolution used to build a new callable, allocating another
185+
// trampoline and making `lib.functions.add_i32` a different function on
186+
// each read.
187+
constfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
188+
assert.strictEqual(lib.getFunction('add_i32',fixtureSymbols.add_i32),fn);
189+
assert.strictEqual(lib.functions.add_i32,fn);
190+
assert.strictEqual(lib.getFunctions().add_i32,fn);
191+
assert.strictEqual(lib.getFunctions(definitions).add_i32,fn);
192+
assert.strictEqual(fn(20,22),42);
193+
}finally{
194+
lib.close();
195+
}
196+
});
197+
198+
test('a dropped function wrapper is collectable',async()=>{
199+
constlib=newffi.DynamicLibrary(libraryPath);
200+
201+
try{
202+
// Caching the wrapper must not pin it, so that dropping the last user
203+
// reference still releases the wrapper and the trampoline it owns.
204+
letfn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
205+
constref=newWeakRef(fn);
206+
fn=null;
207+
208+
awaitgcUntil('a dropped function wrapper is collectable',()=>{
209+
returnref.deref()===undefined;
210+
});
211+
212+
fn=lib.getFunction('add_i32',fixtureSymbols.add_i32);
213+
assert.strictEqual(fn(20,22),42);
214+
}finally{
215+
lib.close();
216+
}
217+
});
218+
179219
test('FFI functions keep their owning library alive',async()=>{
180220
letlib=newffi.DynamicLibrary(libraryPath);
181221
constaddI32=lib.getFunction('add_i32',fixtureSymbols.add_i32);

0 commit comments

Comments
Β (0)