Conversation
Two fixes applied to uniffi-bindgen-node/templates/sys.ts: 1. Call status pool — makeRustCall() now reuses a depth-indexed pool of createCallStatus externals instead of allocating a new one per FFI call. Eliminated ~3.4 KB/call. 2. Byte buffer pool — allocateWithBytes() now reuses a single pre-allocated Buffer + cached External instead of createPointer(U8Array) per call. Eliminated ~1.0 KB/call. Both pools are nulled in _uniffiUnload for GC on shutdown. Cleanup: - Removed RSS instrumentation from enhancer.rs (the PROCESS_CALL_COUNT, rss_bytes() function, and logging block) - Removed JS-side RSS logging from the test harness Net result: 5.6 KB/call → ~1.2 KB/call (79% reduction). The remaining ~1.2 KB/call is inherent ffi-rs FFI overhead that would require upstream ffi-rs changes to address.
| paramsValue: [buffer], | ||
| }); | ||
| const unwrappedExternal = unwrapPointer([external])[0]; | ||
| _nativeByteBufPool = { buffer, unwrappedExternal, capacity }; |
There was a problem hiding this comment.
do we re-create the _nativeByteBufPool more than once ?
and do we need to release the previous buffer before assigning _nativeByteBufPool to a new one ?
There was a problem hiding this comment.
My understanding from reading the LLM analysis is that it is potentially created more than once, and that no, the old buffers are not being freed because of a bug in ffi-rs makes it impossible for these buffers to actually be freed.
So this still results in a memory leak (it doesn't seem like there is actually a way to do this in a fully correct way due to the ffi-rs issues), but a much much smaller one.
|
I opened an issue on the ffi-rs repo with a characterization of the issue on that end: zhangyuang/node-ffi-rs#134. My understanding is that it is impossible to implement the uniffi call semantics in fully non leaking way properly at the moment, and fixing of these upstream issues would be required to do this. |
2 participants
Warning
This pull request was LLM generated and has only been lightly reviewed by a human. The author has tested this and confirms it works in the happy path, but no other validation has been done.
A more thorough review of this needs to occur before it could be merged.
Two fixes applied to uniffi-bindgen-node/templates/sys.ts:
Both pools are nulled in _uniffiUnload for GC on shutdown.
Cleanup:
Net result: 5.6 KB/call → ~1.2 KB/call (79% reduction). The remaining ~1.2 KB/call is inherent ffi-rs FFI overhead that would require upstream ffi-rs changes to address.