shawn-mcginty / naboris

Simple, fast, minimalist http server for OCaml/ReasonML
https://naboris.dev
MIT License
71 stars 4 forks source link

Clean up file descriptors in Res.static #37

Closed shawn-mcginty closed 4 years ago

shawn-mcginty commented 4 years ago

Relates to #34

fds need to be closed in Res.static, there may be other perf benefits we can get there as well, some testing should be done.

gtach2o commented 4 years ago

I have tried with let _ = Unix.close(fd); and it did help.

vegeta -cpus 16 attack -http2=false -duration=60s -targets veg.txt | tee results.bin | vegeta report
Requests      [total, rate, throughput]  3000, 50.01, 50.01
Duration      [total, attack, wait]      59.984438332s, 59.98206917s, 2.369162ms
Latencies     [mean, 50, 95, 99, max]    4.344825ms, 2.572458ms, 9.694918ms, 29.371967ms, 121.457504ms
Bytes In      [total, mean]              404104200, 134701.40
Bytes Out     [total, mean]              0, 0.00
Success       [ratio]                    100.00%
Status Codes  [code:count]               200:3000  
gtach2o commented 4 years ago
vegeta -cpus 4 attack -http2=false -duration=60s -targets veg.txt | tee results.bin | vegeta report

Some interesting numbers of Latencies with Unix.close(fd);

mean 50 95 99 max
4.239174ms 2.278331ms 9.283131ms 26.988654ms 130.551781ms

with let _ = Unix.close(fd);

mean 50 95 99 max
3.23765ms 2.039496ms 6.237305ms 8.247941ms 122.032702ms
shawn-mcginty commented 4 years ago

@gtach2o great! Is this with conf-libev as well? I found performance to be much better with libel. You can make a PR if you’d like. I’m working on getting writing an automated test for this.

I will probably add a recommendation to install libev in the readme as well.

gtach2o commented 4 years ago

I can see you already have it in clean-up-fds branch. Yes, I tested it with libev. The problem is the conf-libev depends on libev C library. I think you need to add

sudo apt-get update
sudo apt-get install libev-dev

to before_install in travis config.

shawn-mcginty commented 4 years ago

I think i might run 2 test suites, one with libev and without, since it is optional. Thanks for all of the help with this!

shawn-mcginty commented 4 years ago

I was able to get much better perf from static files. Thanks a ton @gtach2o. I also added a libev test suite and a vegeta load test suite (these will only run on master PRs). 0.1.1 should be much more performant than 0.1.0!

shawn-mcginty commented 4 years ago

fixed in 0.1.1

shawn-mcginty commented 4 years ago

Actually i'll leave it open until this goes into master :-)