appears as if gc not freeing values created via node-api & napi-rs

[universalmind303]

What version of Bun is running?

1.0.1+31aec4ebe325982fc0ef27498984b0ad9969162b

What platform is your computer?

Darwin 22.4.0 arm64 arm

What steps can reproduce the bug?

originally opened in nodejs-polars, but seems to behave a bit differently in bun.

const pl = require("nodejs-polars");

function runBuggyCode(entries) {
  let df = pl.DataFrame(entries);
}

async function main() {
  const data = Array(50)
    .fill(null)
    .map((_, i) =>
      Array(10000)
        .fill(0)
        .map((_, ii) => i * 100 + ii)
    )

  let count = 0;
  let peakMemUsage = 0;
  while (count < 500) {
    const start = Date.now();
    runBuggyCode(data);
    console.log("process time:", Date.now() - start, "ms");
    peakMemUsage = Math.max(process.memoryUsage().rss, peakMemUsage);
    console.log(
      "peak rss mem usage:",
      Math.round(peakMemUsage / 1024 ** 2),
      "MB"
    );
    console.log("run:", count++);
    await new Promise((res) => setTimeout(res, 100));
  }
}


  void main().catch((err) => console.error(err));

What is the expected behavior?

memory usage doesn't keep growing

What do you see instead?

memory grows on every iteration

Additional information

Nodejs: peak rss mem usage: 207 MB
Bun: peak rss mem usage: 2111 MB

mem stats

heap stats:    peak      total      freed    current       unit      count   
  reserved:   32.0 MiB   32.0 MiB      0       32.0 MiB                        
 committed:   32.0 MiB   32.0 MiB   20.0 MiB   11.9 MiB                        
     reset:      0          0          0          0                            ok
   touched:   64.2 KiB   64.2 KiB  458.6 KiB -394.3 KiB                        ok
  segments:      1          1          0          1                            not all freed!
-abandoned:      0          0          0          0                            ok
   -cached:      0          0          0          0                            ok
     pages:      0          0         22        -22                            ok
-abandoned:      0          0          0          0                            ok
 -extended:      0    
 -noretire:      0    
     mmaps:      0    
   commits:      0    
   threads:      0          0          0          0                            ok
  searches:     0.0 avg
numa nodes:       1
   elapsed:      59.317 s
   process: user: 7.516 s, system: 1.837 s, faults: 42, rss: 2.0 GiB, commit: 32.0 MiB

I tried adding a custom finalize and it seems like it'll never get called in bun, but it does get called in nodejs.

impl ObjectFinalize for JsSeries {
    fn finalize(self, mut env: Env) -> Result<()> {
        println!("finalize series");
        std::mem::drop(self);
        Ok(())
    }
}

Also wanted to say great work on the FFI. For nodejs-polars, which needs to iterate over very large arrays, it is >10x faster for creating dataframes & series! while also using less memory!! Great work 🚀 🚀 !!

pola-rs/nodejs-polars#69

[Jarred-Sumner]

I spent some time investigating and didn't make much progress. The repro is very helpful though.

I do think there is a memory leak in napi_ref, it needs to be deleted. But that is not the cause of this bug. There are finalizers being called. But the one we care about here -- the one holding the memory for the big byte array -- is not being freed and I'm not sure why. It's likely something to do with our implementation of Napi classes. It's not something to do with napi_wrap or napi_unwrap.

[Jarred-Sumner]

In #7765, it should now free the values as expected.

It looks like we still use lots of ram though.

if the code is changed to move the data array to be inside the inner while loop, memory usage becomes stable:

const pl = require("nodejs-polars");

function runBuggyCode(entries) {
  let df = pl.DataFrame(entries);
}

async function main() {
  let count = 0;
  let peakMemUsage = 0;
  while (count < 500) {
    const start = Date.now();
    const data = Array(50)
      .fill(null)
      .map((_, i) =>
        Array(10000)
          .fill(0)
          .map((_, ii) => i * 100 + ii)
      );

    runBuggyCode(data);
    console.log("process time:", Date.now() - start, "ms");
    peakMemUsage = Math.max(process.memoryUsage().rss, peakMemUsage);
    console.log(
      "peak rss mem usage:",
      Math.round(peakMemUsage / 1024 ** 2),
      "MB"
    );
    console.log("run:", count++);
    await new Promise((res) => setTimeout(res, 100));
  }
}

void main().catch((err) => console.error(err));

I suspect it's a timer execution order issue

[universalmind303]

if the code is changed to move the data array to be inside the inner while loop, memory usage becomes stable:

I'd expect the former to use less memory. in your example, we are creating the array inside the loop. It seems highly unexpected that it'd use less ram creating a large array in a loop than creating it outside.