Implement persistent connections?

[richarddd]

I don't see this in the docs but are connections persisted? Or are they dropped on response?

It seems we connect again for each request:

minreq/src/connection.rs

Line 244 in 1533698

let tcp = self.connect()?;

If connections are not persisted, a connection-pool or a connection-cache would greatly improve additional connections (especially for https)

[neonmoe]

They're dropped on response, by design. Minreq isn't really supposed to be extremely fast or efficient*, but minimal in code size (including dependencies).

* We'll take what we can get though, if you feel like you could very simply add a connection pool, feel free to make a PR. But I don't personally think it's worth it, I'd rather use another http client if I needed the speed.

[12932]

I definitely think this is something that's important to have. Since the most expensive part of any request is the TLS handshake, it's a bit of a waste to perform this every request, especially on lower-end hardware where minreq is more appealing than other Rust http crates

[w-flo]

I do believe a connection pool would be out of scope of the crate, but what about a persistent connection that is not pooled, but kept alive between requests? Like this:

// This should probably have a better name ([new_]persistent_connection()? minreq::Connection::new()?)
let mut connection = minreq::keepalive();
// The following line would keep the connection alive if the server suggests it through the relevant HTTP headers
let response = connection.get(url).send().unwrap();
// do some stuff or maybe not
// This would re-use the connection if it was kept alive and the URL refers to the same host, or re-connect if the connection could not be kept alive since the server didn't want to, and throw an error (or possibly disconnect and re-connect?) if it refers to a different host
let other_response = connection.post(url).send().unwrap();
// This would close the connection if it is still alive
drop(connection);

I suspect it would add a bit of complexity though, like tracking connection state, the timeout and request limits indicated by the "Keep-Alive" HTTP header, making sure requests are not getting sent to the wrong host if the previous connection is still alive, and so on. So maybe this is out of scope of the crate, too. I do like the idea though. Existing minreq::get() etc. calls should be very simple to offer on top of this API.

Obviously, HTTP pipelining would not work when using a simple sync API like minreq is designed to offer, and an async API generally seems more fitting for pipelining. However, this simple "connection keep-alive" approach would reduce the number of expensive TLS handshakes when sending multiple requests to the same host in quick succession.

[neonmoe]

Yeah, I think that would work. There's already some accommodation for making multiple requests with one connection, for the redirection functionality.