feat: enable Wasm weak references for automatic garbage collection #318
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
yisibl
force-pushed
the
fix-wasm-memory-leak
branch
from
87135ef
to
d2a6359
Compare
|
@mizdra Please check if this PR fixes the problem. |
|
Using WeakRef is a good idea! I'll try it and see if it solves the problem. |
|
@mizdra I'm looking forward to your test results, I'm ready to release them later. |
|
I just finished work. I'm testing now. |
| @@ -136,16 +137,21 @@ function handleError(f, args) { | |||
| wasm.__wbindgen_exn_store(addHeapObject(e)); | |||
| } | |||
| } | |||
| var BBoxFinalization = typeof FinalizationRegistry === "undefined" ? { register: () => { | |||
| }, unregister: () => { | |||
| } } : new FinalizationRegistry((ptr) => wasm.__wbg_bbox_free(ptr >>> 0)); | |||
There was a problem hiding this comment.
FinalizationRegistry requires Node.js v14.6.0 and Safari 14.1. Is this okay? What versions of Node.js/Safari does @resvg/resvg-wasm support?
|
@yisibl Test Codes// wasm-memory-leak-test.mjs
import { readFile } from 'fs/promises'
import { join } from 'path'
import { Resvg, initWasm } from './wasm/index.mjs'
const wasmPath = join(import.meta.dirname, './wasm/index_bg.wasm')
if (!global.gc) {
throw new Error('You must run this script with --expose-gc')
}
function logMemoryUsage(i, rss, heapTotal, heapUsed, external) {
console.log(
`${i.padStart(8, ' ')},` +
`${rss.padStart(12, ' ')},` +
`${heapTotal.padStart(12, ' ')},` +
`${heapUsed.padStart(12, ' ')},` +
`${external.padStart(12, ' ')},`,
)
}
await initWasm(readFile(wasmPath))
const svg = await readFile(join(import.meta.dirname, './example/text.svg'))
const opts = {
background: 'rgba(238, 235, 230, .9)',
fitTo: {
mode: 'width',
value: 1200,
},
font: {
fontFiles: ['./example/SourceHanSerifCN-Light-subset.ttf'],
loadSystemFonts: false,
},
}
logMemoryUsage('i', 'rss', 'heapTotal', 'heapUsed', 'external')
for (let i = 0; i < 1000000; i++) {
const resvg = new Resvg(svg, opts)
const pngData = resvg.render()
const pngBuffer = pngData.asPng()
if (i % 10 === 0) {
// NOTE: Do GC before measuring memory usage because the garbage is not measured.
global.gc()
const usage = process.memoryUsage()
logMemoryUsage(
i.toString(),
usage.rss.toString(),
usage.heapTotal.toString(),
usage.heapUsed.toString(),
usage.external.toString(),
)
}
}Test environment
Before$ git switch main
$ node --expose-gc wasm-memory-leak-test.mjs
i, rss, heapTotal, heapUsed, external,
0, 131956736, 9895936, 4704664, 26211296,
10, 178569216, 9895936, 4714272, 55639141,
20, 213336064, 6750208, 4714864, 87424101,
30, 244957184, 6750208, 4453344, 116128869,
40, 275021824, 6750208, 4452648, 149552229,
50, 305004544, 6750208, 4469672, 176770687,
60, 334856192, 6750208, 4456840, 209889867,
70, 366247936, 6750208, 4471112, 237412505,
80, 396165120, 6750208, 4471896, 268893285,
90, 427704320, 6750208, 4458584, 302164555,
100, 464650240, 6750208, 4473336, 329383013,
110, 495779840, 6750208, 4464816, 362654283,
120, 525713408, 6750208, 4464272, 391359051,
130, 557056000, 6750208, 4479936, 423144011,
140, 586940416, 6750208, 4480656, 454928971,
150, 618479616, 6750208, 4481376, 482147429,
160, 648380416, 6750208, 4468152, 510700107,
170, 678297600, 6750208, 4471016, 544123467,
180, 708263936, 6750208, 4485288, 571494015,
190, 738131968, 6750208, 4471368, 604613195,
200, 769474560, 7012352, 4486856, 636398155,After |
|
Thanks to your detailed testing, I confirm that the problem still exists, for which I have raised an issue: rustwasm/wasm-bindgen#3903 |
|
I found that my method of measuring memory leaks was incorrect. In Node.js, when --- a/wasm-memory-leak-test.mjs
+++ b/wasm-memory-leak-test.mjs
@@ -1,5 +1,6 @@
import { readFile } from 'fs/promises'
import { join } from 'path'
+import { setTimeout } from 'timers/promises'
import { Resvg, initWasm } from './wasm/index.mjs'
@@ -39,6 +40,7 @@ for (let i = 0; i < 1000000; i++) {
const resvg = new Resvg(svg, opts)
const pngData = resvg.render()
const pngBuffer = pngData.asPng()
+ await setTimeout(0)
if (i % 10 === 0) {
// NOTE: Do GC before measuring memory usage because the garbage is not measured.
global.gc()Log$ node --expose-gc wasm-memory-leak-test.mjs
i, rss, heapTotal, heapUsed, external,
0, 123404288, 9895936, 4732256, 26217689,
10, 166232064, 9895936, 4779880, 55645521,
20, 182583296, 10158080, 5044056, 62154603,
30, 185434112, 7012352, 4791968, 63371323,
40, 184434688, 7012352, 4800976, 63371323,
50, 185597952, 7012352, 4801072, 69424135,
60, 185647104, 7012352, 4802312, 69728315,
70, 185614336, 7012352, 4808896, 69728315,
80, 185696256, 7012352, 4864728, 69728315,
90, 185729024, 7012352, 4863800, 69728315,
100, 200720384, 7012352, 4873792, 69728315,
110, 201932800, 7012352, 4878392, 69728315,
120, 204947456, 7012352, 4878760, 69728315,
130, 204931072, 7012352, 4875680, 69728315,
140, 204996608, 7012352, 4875680, 69728315,
150, 204980224, 7012352, 4875680, 69728315,
160, 204963840, 7012352, 4875680, 69728315,
170, 205012992, 7012352, 4877560, 69728315,
180, 208011264, 7012352, 4877560, 69728315,
190, 207994880, 7012352, 4877560, 69728315,
200, 208044032, 7012352, 4877560, 69728315,
210, 208044032, 7012352, 4877560, 69728315,
220, 208060416, 7012352, 4877560, 69728315,
230, 208027648, 7012352, 4877560, 69728315,
240, 208076800, 7012352, 4877560, 69728315,
250, 211025920, 7012352, 4877416, 75781127,
260, 211124224, 7012352, 4877560, 76085307,
270, 211107840, 7012352, 4877560, 76085307,
280, 211107840, 7012352, 4877560, 76085307,
290, 211075072, 7012352, 4877560, 76085307,
300, 211107840, 7012352, 4877560, 76085307,
310, 214089728, 7012352, 4877560, 76085307,
320, 216760320, 7012352, 4876968, 80921399,
330, 216743936, 7012352, 4877864, 82442299,
340, 216760320, 7012352, 4879032, 82442299,
350, 216776704, 7012352, 4879032, 82442299,
360, 216760320, 7012352, 4879032, 82442299,
370, 216793088, 7012352, 4879032, 82442299,
380, 219807744, 7012352, 4883816, 82442299,
390, 219807744, 7012352, 4883816, 82442299,
400, 219824128, 7012352, 4883816, 82442299,
^CHowever, the heap allocated by the The following patch seems to free the heap. --- a/wasm/index.mjs
+++ b/wasm/index.mjs
@@ -281,6 +281,7 @@ var Resvg = class {
throw takeObject(r1);
}
this.__wbg_ptr = r0 >>> 0;
+ ResvgFinalization.register(this, this.__wbg_ptr, this);
return this;
} finally {
wasm.__wbindgen_add_to_stack_pointer(16);Log$ node --expose-gc wasm-memory-leak-test.mjs
i, rss, heapTotal, heapUsed, external,
0, 131252224, 10158080, 4732504, 26217689,
10, 169410560, 10158080, 4780640, 55645521,
20, 183549952, 10158080, 5003136, 57014331,
30, 185958400, 7012352, 4788232, 63488823,
40, 186580992, 7012352, 4802240, 65009723,
50, 186646528, 7012352, 4803008, 65009723,
60, 186630144, 7012352, 4804248, 65009723,
70, 186679296, 7012352, 4810832, 65009723,
80, 186712064, 7012352, 4866448, 65009723,
90, 186728448, 7012352, 4865736, 65009723,
100, 189267968, 7012352, 4875728, 65009723,
110, 190545920, 7012352, 4880328, 65009723,
120, 190627840, 7012352, 4880696, 65009723,
130, 193495040, 7012352, 4877616, 65009723,
140, 193658880, 7012352, 4877616, 65009723,
150, 193642496, 7012352, 4877616, 65009723,
160, 193675264, 7012352, 4877616, 65009723,
170, 193691648, 7012352, 4879496, 65009723,
180, 194265088, 7012352, 4879496, 65009723,
190, 194232320, 7012352, 4879496, 65009723,
200, 194265088, 7012352, 4879496, 65009723,
210, 194887680, 7012352, 4879496, 65009723,
220, 195051520, 7012352, 4879496, 65009723,
230, 195084288, 7012352, 4879496, 65009723,
240, 194805760, 7012352, 4879496, 65009723,
250, 194805760, 7012352, 4879496, 65009723,
260, 194772992, 7012352, 4879496, 65009723,
270, 194936832, 7012352, 4879496, 65009723,
280, 195149824, 7012352, 4879496, 65009723,
290, 195166208, 7012352, 4879496, 65009723,
300, 195149824, 7012352, 4879496, 65009723,
310, 195149824, 7012352, 4879496, 65009723,
320, 195182592, 7012352, 4879496, 65009723,
330, 195182592, 7012352, 4879496, 65009723,
340, 195248128, 7012352, 4880968, 65009723,
350, 195231744, 7012352, 4880968, 65009723,
360, 195264512, 7012352, 4880968, 65009723,
370, 195231744, 7012352, 4880968, 65009723,
380, 195248128, 7012352, 4885752, 65009723,
390, 195264512, 7012352, 4885752, 65009723,
400, 195313664, 7012352, 4885752, 65009723,
^C
|
|
@mizdra Yes, I remembered, there has to |
|
Hmm. It seems that even without This behavior seems to come from the implementation of V8 (the JavaScript engine used internally by Node.js). When an object registered with In other words, on Chrome, Edge, and Node.js using V8, if you use @resvg/resvg-wasm in fully synchronous code, cleanup callback will not be called and will leak memory. In such cases, the Furthermore, the ECMAScript specification does not define strictly when to call cleanup callback of
Therefore, with non-V8 JavaScript engines, even code with |
|
IMO: I agree with supporting automatic freeing using FinalizationRegistry. It prevents common user mistakes. It does not cover some edge cases, but I think that is acceptable. Edge cases can still be avoided by calling |
Yes, I agree that both can be used at the same time so that they are compatible with more environments. |
|
I reported the behavior of memory leaks in the synchronous code to wasm-bindgen: rustwasm/wasm-bindgen#3917 |
Enable Weak References in the Wasm bindings. This ensures that Wasm memory is freed automatically by the garbage collector, otherwise, it will continue to grow unless the developer explicitly calls
.free()on every exported instance or the instance is passed back to a function that takes ownership of it, allowing Rust to free it.Without weak references your JS integration may be susceptible to memory leaks in Rust, for example:
.free()on a JS object, leaving the Rust memory allocated.Closuretype) may not be executed and cleaned up.Closure::{into_js_value,forget}methods which explicitly do not free the underlying memory.Note: Weak References are enabled by default since wasm-bindgen v2.9.1, before that you need to add the
--weak-refsoption.e.g.
wasm-pack build --target web --out-name index --out-dir wasm/dist --release --weak-refsTest
https://github.com/yisibl/repro-resvg-wasm-memory-leak
Resolved: #216 (comment)