Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@node-rs/crc32: rss memory leak #655

Closed
twoeths opened this issue Jan 11, 2023 · 6 comments
Closed

@node-rs/crc32: rss memory leak #655

twoeths opened this issue Jan 11, 2023 · 6 comments
Assignees

Comments

@twoeths
Copy link

twoeths commented Jan 11, 2023

The below code snippet can reproduce the memory leak issue

const nodeRs = require("@node-rs/crc32");
const crc32c = nodeRs.crc32c;

const bytes = Buffer.alloc(1000);


async function memTest() {
  const count = 1_000_000_000;
  for (let i = 0; i < count; i++) {
    crc32c(bytes);
    if (i % 100_000 === 0) {
      await new Promise((resolve) => setTimeout(resolve, 100));
      const {heapTotal, rss} = process.memoryUsage();
      console.log(
        "Memory usage",
        Math.floor((i * 100) / count) + "%",
        "heapTotal",
        toMem(heapTotal),
        "rss",
        toMem(rss)
      );
    }
  }
}

function toMem(n) {
  const bytes = Math.abs(n);
  const sign = n > 0 ? "+" : "-";
  if (bytes < 1e6) return sign + Math.floor(bytes / 10) / 100 + " KB";

  if (bytes < 1e9) return sign + Math.floor(bytes / 1e4) / 100 + " MB";

  return sign + Math.floor(bytes / 1e7) / 100 + " GB";
}

console.log("Start node-rs crc32c mem test");

memTest().then(() => console.log("Done"));

Heap memory is kept stable but rss keeps increasing, some sample outputs:

Memory usage 1% heapTotal +7.63 MB rss +2.27 GB
Memory usage 1% heapTotal +7.63 MB rss +2.25 GB
Memory usage 1% heapTotal +7.63 MB rss +2.23 GB

This happens even with the latest version (1.5.1)

Found it from ChainSafe/node-snappy-stream#16

@twoeths twoeths changed the title rss memory leak @node-rs/crc32: rss memory leak Jan 11, 2023
@Brooooooklyn Brooooooklyn self-assigned this Jan 11, 2023
@Brooooooklyn
Copy link
Member

@tuyennhv these are two separate issues, memory leak could be resolved by upgrade to latest NAPI-RS: napi-rs/napi-rs#1331, I can confirm it was fixed in local.

The second is globally declared Buffer would cause memory leak too in your code snippets, it's the expected Node.js behavior: nodejs/node#39915

the memory goes to normal if I move the const bytes = Buffer.alloc(1000) into the loop:

- const bytes = Buffer.alloc(1000);


async function memTest() {
  const count = 1_000_000_000;
  for (let i = 0; i < count; i++) {
+   const bytes = Buffer.alloc(1000);
    crc32c(bytes);
    if (i % 100_000 === 0) {
      await new Promise((resolve) => setTimeout(resolve, 100));
      const {heapTotal, rss} = process.memoryUsage();
      console.log(
        "Memory usage",
        Math.floor((i * 100) / count) + "%",
        "heapTotal",
        toMem(heapTotal),
        "rss",
        toMem(rss)
      );
    }
  }
}

@twoeths
Copy link
Author

twoeths commented Jan 11, 2023

@Brooooooklyn thanks for your quick response and investigation, looking forward to seeing your new release!

@BlackGlory
Copy link

I've noticed memory leaks on @node-rs/xxhash too, when will a new version be released?

@Brooooooklyn
Copy link
Member

Publishing

@Brooooooklyn
Copy link
Member

Published

@twoeths
Copy link
Author

twoeths commented Jan 12, 2023

thanks @Brooooooklyn 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants