Closed multivoltage closed 6 months ago
I tried to edit all calback
After that I tried npm run exaple && autocannon -c 100 -d 5 -p 10 localhost:3000
In master
┌─────────┬───────┬────────┬────────┬────────┬───────────┬─────────┬────────┐
│ Stat │ 2.5% │ 50% │ 97.5% │ 99% │ Avg │ Stdev │ Max │
├─────────┼───────┼────────┼────────┼────────┼───────────┼─────────┼────────┤
│ Latency │ 98 ms │ 101 ms │ 161 ms │ 174 ms │ 107.03 ms │ 16.1 ms │ 198 ms │
└─────────┴───────┴────────┴────────┴────────┴───────────┴─────────┴────────┘
┌───────────┬─────────┬─────────┬─────────┬────────┬─────────┬────────┬─────────┐
│ Stat │ 1% │ 2.5% │ 50% │ 97.5% │ Avg │ Stdev │ Min │
├───────────┼─────────┼─────────┼─────────┼────────┼─────────┼────────┼─────────┤
│ Req/Sec │ 7703 │ 7703 │ 9543 │ 9687 │ 9190 │ 747.26 │ 7703 │
├───────────┼─────────┼─────────┼─────────┼────────┼─────────┼────────┼─────────┤
│ Bytes/Sec │ 2.07 MB │ 2.07 MB │ 2.56 MB │ 2.6 MB │ 2.46 MB │ 200 kB │ 2.06 MB │
└───────────┴─────────┴─────────┴─────────┴────────┴─────────┴────────┴─────────┘
Req/Bytes counts sampled once per second.
# of samples: 5
47k requests in 5.03s, 12.3 MB read
In my commit:
┌─────────┬────────┬────────┬────────┬────────┬──────────┬─────────┬────────┐
│ Stat │ 2.5% │ 50% │ 97.5% │ 99% │ Avg │ Stdev │ Max │
├─────────┼────────┼────────┼────────┼────────┼──────────┼─────────┼────────┤
│ Latency │ 140 ms │ 146 ms │ 165 ms │ 176 ms │ 147.6 ms │ 7.62 ms │ 210 ms │
└─────────┴────────┴────────┴────────┴────────┴──────────┴─────────┴────────┘
┌───────────┬─────────┬─────────┬─────────┬─────────┬─────────┬─────────┬─────────┐
│ Stat │ 1% │ 2.5% │ 50% │ 97.5% │ Avg │ Stdev │ Min │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Req/Sec │ 6003 │ 6003 │ 6783 │ 6995 │ 6632.4 │ 363.27 │ 6000 │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Bytes/Sec │ 1.61 MB │ 1.61 MB │ 1.82 MB │ 1.87 MB │ 1.78 MB │ 97.5 kB │ 1.61 MB │
└───────────┴─────────┴─────────┴─────────┴─────────┴─────────┴─────────┴─────────┘
Req/Bytes counts sampled once per second.
# of samples: 5
34k requests in 5.03s, 8.89 MB read
i think the performance decreased, maybe using async await is not the best way or maybe i made some mistakes
Hopefully performance will not be degraded when we are finished :/.
how are the benchmarks?
at my commit:
┌─────────┬────────┬────────┬────────┬────────┬───────────┬─────────┬────────┐
│ Stat │ 2.5% │ 50% │ 97.5% │ 99% │ Avg │ Stdev │ Max │
├─────────┼────────┼────────┼────────┼────────┼───────────┼─────────┼────────┤
│ Latency │ 141 ms │ 153 ms │ 286 ms │ 324 ms │ 161.49 ms │ 32.4 ms │ 344 ms │
└─────────┴────────┴────────┴────────┴────────┴───────────┴─────────┴────────┘
┌───────────┬─────────┬─────────┬─────────┬─────────┬─────────┬────────┬─────────┐
│ Stat │ 1% │ 2.5% │ 50% │ 97.5% │ Avg │ Stdev │ Min │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Req/Sec │ 5003 │ 5003 │ 6463 │ 6995 │ 6090 │ 890.18 │ 5000 │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Bytes/Sec │ 1.34 MB │ 1.34 MB │ 1.73 MB │ 1.87 MB │ 1.63 MB │ 239 kB │ 1.34 MB │
└───────────┴─────────┴─────────┴─────────┴─────────┴─────────┴────────┴─────────┘
Req/Bytes counts sampled once per second.
# of samples: 5
31k requests in 5.03s, 8.16 MB read
I start reading this issue
Using
const readFile = require("util").promisify(require("fs").readFile)
is really impressive and performance come back to the same than master commit. All my test are made with node 18. @mcollina what do you think about ?
@mcollina @multivoltage
What about this PR?
@Uzlopak Hi. Since there was a lot of code new after some release I decided to create new branch from new release and did same commit from 0. So I have a ready branch but not pushed. That because after my last reply I did not find a method different from node utils promisify which keep same perfomance as original method readFile
@multivoltage are you still in continuing with this work? I am interested in picking it up where you left off.
@mweberxyz nope in this moment. But in 2/3 days I will update my PR and maybe we can help each other if you want
@mweberxyz I updated my branch if you want to start from that. Two main points that are important for me:
const { readFile } = require("node:fs").promises
is VERY slowconst readFile = require("node:util").promisify(require("node:fs").readFile);
witch is similar do sync version
, but a little bit slowerthank @mweberxyz, I will close the issue :)
Whats the point of using promisify on a sync function? You will anyway block the Event loop.
Only because async version of readFile seem to be slower than sync version. But nevermind since I closed PR :) in favor of new one by @mweberxyz
Checklist
npm run test
andnpm run benchmark
Why this PR?
I added support for html-minifier-terser in a separate PR. To to that I wrote some "await" and "async". Matteo Collina advised that it's better to have all "await" or all callback for promises. So I tried to remove
.then() and .catch()
in favour ofawait asynk