Benchmark of isahc and hyper

[Xuanwo]

Hi, there!

OpenDAL is a rust library that focuses on accessing different storage services painless and efficiently. I worked out a demo to migrate hyper to isahc. Here is my conclusion to share with the isahc community, hoping it will be helpful!

apache/opendal#471

• isahc uses the same request struct with hyper, making migration easier.
• isahc's ResponseFuture has a lifetime. We have to Box::pin again. (Can we improve?)
• All tests passed (isahc is OK to use for opendal)
• Benchmark seems a bit slow (not sure if related to runtime)

---

I have mainly two questions:

• Can we have a 'static ResponseFuture?
• Is this benchmark as expected? Or I'm using isahc in the wrong way?

Thanks in advance!

[sagebind]

Thanks for trying out Isahc, always appreciate feedback!

Can we have a 'static ResponseFuture?

Well ResponseFuture is currently not 'static because it borrows the client instead of doing an implicit clone, though I can see how that could be annoying to use with a program structure like that one where you don't pass around the client. I think it might be possible to do some internal refactoring to allow this without excessive internal cloning though, but the API change would probably be breaking. We're planning on releasing version 2.0 sometime this year with a couple minor breaking changes and this would probably be a good thing for us to include.

Is this benchmark as expected? Or I'm using isahc in the wrong way?

Hyper is pretty darn fast so I'm pretty pleased to see Isahc being at least within the same ballpark performance as Hyper! Generally Isahc has weaker throughput with lots of concurrent requests since HttpClient isn't fully parallelized and doesn't always use multiple CPU cores as well as it could. There's room for improvement here definitely, though since we use libcurl under the hood there's a couple things that are constrained.

That said, I do have a couple notes on your PR where there's probably some performance being left on the table:

    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        let mut data = vec![0; 1024];
        match ready!(Pin::new(&mut self.body).poll_read(cx, &mut data)) {
            Ok(0) => Poll::Ready(Error::new(

https://github.com/datafuselabs/opendal/blob/79f0029b0ecb0e546519fd83c273b62daf6f4575/src/io_util/http_body.rs#L195-L198

This is allocating a buffer every time the response stream is polled, which is pretty inefficient. It would be better to allocate a buffer once and reuse the buffer every call. Also, 1 KiB is a pretty small buffer which will probably lower throughput; Isahc streams are internally buffered at 64 KiB so using a buffer closer to that size might yield better throughput. Also, I'm not sure if the project has any additional buffering layers higher up in the stack but generally we recommend using the response streams directly if possible since they are pre-buffered, and additional buffering layers can also reduce throughput.

Edit: Based on another pass I think you could simply read into ParseErrorResponse.buf directly instead of reading into an intermediate buffer and copying.

Overall looks pretty correct to me.

[Xuanwo]

Thanks for the quick response! And thanks for the review; I will address the comments you raised.

We're planning on releasing version 2.0 sometime this year with a couple minor breaking changes and this would probably be a good thing for us to include.

Oh, that's nice!

There's room for improvement here definitely, though since we use libcurl under the hood there's a couple things that are constrained.

Great! Looking forward to this!

---

Overall, isahc is excellent, and you are a fantastic maintainer! I am very happy to work with you.

[sagebind]

I created an issue to track making ResponseFuture 'static here: #403

[suryavirkapur]

Hey @Xuanwo, did you end up switching to isahc?

[Xuanwo]

No now. We will evaluate isahc again while releasing 2.0.

[Xuanwo]

Updates: OpenDAL decides to use isahc for a try!

We do want the blocking API support, and implementing that against hyper is boring and easy to go wrong with nest tokio runtime. Compared to that, the slight performance difference can be ignored.

Contributors from OpenDAL can also make some contributions to improving the overall performance!