napi

[nstepien]

Still needs more work on error handling and memory management.

[nstepien]

Seems like there is a memory leak somewhere. I was Hoping Node 10.2.0 would fix it as it fixes a memory leak with napi according to its changelog, but that does not seem to be the case.

[nstepien]

@addaleax @gabrielschulhof
Would you happen to have any clues when it comes to n-api memory management?

Perf and memory looks good, but it does look like there might be a small memory leak somewhere. It does look like we make the right calls to napi_adjust_external_memory, so I'm not sure where to look.
The current NAN version always drops down to the same RAM usage, while the n-api version seems to slowly increase over time.

I run a benchmark using a tiny server + wrk:

const fs = require('fs');
const http = require('http');
const iltorb = require('.');

const filename = 'lorem_ipsum.txt';

function onRequest(req, res) {
  res.setHeader('Content-Encoding', 'br');
  fs.createReadStream(filename).pipe(iltorb.compressStream()).pipe(res);
}

http.createServer(onRequest).listen(3000);

$ wrk -c 100 -t 6 -d 60s -H 'Accept-Encoding: br' http://localhost:3000

[mhdawson]

@MayhemYDG can you list the steps need to recreate, starting from nothing? ie what all needs to be installed etc.

[nstepien]

@mhdawson I think I fixed the remaining issue, I needed to manually delete the object I was wrapping in the destructor.

If you want to try and see for yourself if there are any remaining memory leaks:

1. clone the repo, including the submodule, cd into it
2. run npm install --build-from-source
3. check that all works well by running the tests: npm test
4. create bench.js, paste in the following:

const fs = require('fs');
const http = require('http');
const iltorb = require('.');
const buf = fs.readFileSync('test/fixtures/data.txt');

function onRequest(req, res) {
  res.setHeader('Content-Encoding', 'br');
  const stream = iltorb.compressStream();
  stream.pipe(res);
  stream.end(buf);
}

http.createServer(onRequest).listen(3000);

5. run node bench.js
6. you can test in your browser if localhost:3000 loads fine
7. run wrk -c 100 -t 6 -d 60s -H 'Accept-Encoding: br' http://localhost:3000, this will essentially stress-test the server, then when it's over wait about 10 seconds, and check the process' memory usage
8. repeat the step above to see if the memory usage keeps increasing or not.

On my machine the process typically stabilizes around 40-45MB when idle.

[mhdawson]

If you believe its good, then I'm ok. We have looks of work to do on the N-API/node-addon-api front so unless there is an issue I'll concentrate on that other work.