swindon-rs / tk-http

Full featured HTTP and Websockets library for rust/tokio
Apache License 2.0
136 stars 10 forks source link

Consider convenience method to stamp Date header in response #32

Closed nayato closed 6 years ago

nayato commented 6 years ago

Date can be cached thread-locally and formatted only when formatted value is expected to change (seconds-level granularity). e.g. Encoder.add_date() or Encoder.add_auto_date()

tailhook commented 6 years ago

It might be a good idea to add a method, it's not there because it needs another dependency and it's oneliner. But I'm not sure about caching.

Is it really justified by benchmarks or it's just an idea?

nayato commented 6 years ago

Just compared (code here: https://github.com/nayato/hyper-test/tree/a618125e2ee34900742cdec54d92c28ef021ea34) and in such a basic setup difference is ~18% (154K req/sec without caching Date vs 182K with).

tailhook commented 6 years ago

Hm, that's a pretty large difference. Can you also try httpdate insead of time crate? (httpdate has a hardcoded date format, so might be faster). We might also just need to add inline(always) to format_header (because it's in the different crate it might not get inlined).

BTW, how tk-http compares to hyper? (I see example in your repo)

nayato commented 6 years ago

Hyper - 223K, same functionality.

nayato commented 6 years ago

tried non-cached httpdate and re-ran benchmark on a different VM. hyper: 212K tk-http, cached date using time crate: 201K tk-http using httpdate (no caching added): 186K

nayato commented 6 years ago

194K with httpdate, no caching when with this change https://github.com/nayato/hyper-test/commit/5590d994f9ff4bc7bad008f6770fbd7b79c15599

tailhook commented 6 years ago

Okay with this kind of difference there is much less sense for caching. In more real workload it should not be noticeable at all.

Still it's unfortunate that tk-http is slower than hyper :) How do you benchmark? It would be nice that I could replicate the test and see if something might need to be fixed in tk-http. Also, typical benchmarks like ab tend to send two request headers, while normal browser send ~10, and that may be important since the two libraries handle headers in a very different way.

nayato commented 6 years ago

I benchmarked with wrk, without pipelining, on two Ubuntu 16.04 VMs in Azure - 4 core server (D3v2), 16 core client (F16, but may go lower here). I agree on real life perf diff not being big. On dependency mgmt, considering how ubiquitous Date header in HTTP is, I sure wouldn't mind having OOTB support for stamping it. Also, considering how small and focused httpdate is... 😉

tailhook commented 6 years ago

Done. Releasing in v0.3.6 (waiting on travis).

I have some motivation to tackle the overall performance difference, but not sure if I'll have spare time for that. Feel free to open issue if you have another idea or just to discuss performance.

tailhook commented 6 years ago

We're also going to improve the performance of httpdate itself: https://github.com/pyfisch/httpdate/pull/2