Repository: sagebind/isahc
Pull request #397 (2022-05-14 12:14:14)

feat(benches): Add bench with reqwest blocking
Signed-off-by: Xuanwo <github@xuanwo.io>

---

This PR will add `reqwest` (with the `blocking` feature enabled) to benchmarks so that we can measure the perf with reqwest(hyper).


Patch:
diff --git a/Cargo.toml b/Cargo.toml
index ab9b7fc..76dfbc4 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -9,7 +9,7 @@ categories = ["web-programming::http-client"]
 repository = "https://github.com/sagebind/isahc"
 documentation = "https://docs.rs/isahc/"
 readme = "README.md"
-edition = "2018"
+edition = "2021"
 exclude = [".*", "benchmarks", "media", "src/cookies/psl/list/*/"]
 
 [package.metadata.docs.rs]
diff --git a/benchmarks/Cargo.toml b/benchmarks/Cargo.toml
index 4119e4f..200a6c2 100644
--- a/benchmarks/Cargo.toml
+++ b/benchmarks/Cargo.toml
@@ -9,6 +9,7 @@ criterion = "0.3"
 curl = "0.4.42"
 rayon = "1"
 rouille = "3"
+reqwest = { version = "0.11.10", features = ["blocking"] }
 
 [dependencies.isahc]
 path = ".."
diff --git a/benchmarks/benches/download.rs b/benchmarks/benches/download.rs
index 553e389..a2c5007 100644
--- a/benchmarks/benches/download.rs
+++ b/benchmarks/benches/download.rs
@@ -48,6 +48,22 @@ fn benchmark(c: &mut Criterion) {
             BatchSize::SmallInput,
         )
     });
+
+    c.bench_function("download 64K: reqwest", move |b| {
+        use reqwest::blocking::Client;
+
+        let server = TestServer::static_response(&DATA);
+        let endpoint = server.endpoint();
+
+        b.iter_batched(
+            Client::new,
+            |client| {
+                let req = client.get(&endpoint).build().unwrap();
+                let _ = client.execute(req).unwrap().copy_to(&mut sink()).unwrap();
+            },
+            BatchSize::SmallInput,
+        )
+    });
 }
 
 criterion_group!(benches, benchmark);
diff --git a/examples/cookies.rs b/examples/cookies.rs
index c2a4241..5c9632b 100644
--- a/examples/cookies.rs
+++ b/examples/cookies.rs
@@ -31,7 +31,7 @@ fn main() -> Result<(), Box<dyn Error>> {
 
     // Send another request. The cookies previously set will be sent.
     let mut response = Request::get("http://httpbin.org/cookies")
-        .cookie_jar(cookie_jar.clone())
+        .cookie_jar(cookie_jar)
         .body(())?
         .send()?;
 
diff --git a/examples/head_length.rs b/examples/head_length.rs
index a3b9cdf..b23ae84 100644
--- a/examples/head_length.rs
+++ b/examples/head_length.rs
@@ -3,8 +3,9 @@ use std::io::Cursor;
 use isahc::{Body, Request};
 
 fn main() -> Result<(), isahc::Error> {
-    let weird_request = Request::head("http://download.opensuse.org/update/tumbleweed/repodata/repomd.xml")
-        .body(Body::from_reader(Cursor::new(b"")))?;
+    let weird_request =
+        Request::head("http://download.opensuse.org/update/tumbleweed/repodata/repomd.xml")
+            .body(Body::from_reader(Cursor::new(b"")))?;
 
     let error = isahc::send(weird_request).unwrap_err();
     eprintln!("{}", error);
diff --git a/examples/json.rs b/examples/json.rs
index add329e..a9ba31e 100644
--- a/examples/json.rs
+++ b/examples/json.rs
@@ -6,8 +6,7 @@
 //! isahc = { version = "*", features = ["json"]}
 //! ```
 
-use isahc::Request;
-use isahc::prelude::*;
+use isahc::{prelude::*, Request};
 
 #[derive(Debug, serde::Serialize)]
 #[serde(rename_all = "UPPERCASE")]
diff --git a/rustfmt.toml b/rustfmt.toml
index 3fcae1d..caaad68 100644
--- a/rustfmt.toml
+++ b/rustfmt.toml
@@ -1,7 +1,7 @@
-edition = "2018"
+edition = "2021"
 imports_layout = "HorizontalVertical"
-merge_imports = true
 overflow_delimited_expr = true
 struct_lit_single_line = false
 use_field_init_shorthand = true
+imports_granularity="Crate"
 version = "Two"
diff --git a/src/agent/selector.rs b/src/agent/selector.rs
index 2f78987..b29b096 100644
--- a/src/agent/selector.rs
+++ b/src/agent/selector.rs
@@ -305,7 +305,7 @@ impl Hasher for IntHasher {
             }
         }
 
-        (&mut self.0[..bytes.len()]).copy_from_slice(bytes);
+        (self.0[..bytes.len()]).copy_from_slice(bytes);
     }
 
     #[inline]
diff --git a/src/agent/timer.rs b/src/agent/timer.rs
index 85359de..0c5e632 100644
--- a/src/agent/timer.rs
+++ b/src/agent/timer.rs
@@ -1,37 +1,40 @@
-use std::time::{Duration, Instant};
-
-use crossbeam_utils::atomic::AtomicCell;
+use std::{
+    sync::Mutex,
+    time::{Duration, Instant},
+};
 
 #[derive(Debug)]
 pub(crate) struct Timer {
-    timeout: AtomicCell<Option<Instant>>,
+    timeout: Mutex<Option<Instant>>,
 }
 
 impl Timer {
     pub(crate) fn new() -> Self {
         Self {
-            timeout: AtomicCell::new(None),
+            timeout: Mutex::new(None),
         }
     }
 
     pub(crate) fn is_expired(&self, now: Instant) -> bool {
         self.timeout
-            .load()
+            .lock()
+            .unwrap()
             .map(|timeout| now >= timeout)
             .unwrap_or(false)
     }
 
     pub(crate) fn get_remaining(&self, now: Instant) -> Option<Duration> {
         self.timeout
-            .load()
+            .lock()
+            .unwrap()
             .map(|timeout| timeout.saturating_duration_since(now))
     }
 
     pub(crate) fn start(&self, timeout: Duration) {
-        self.timeout.store(Some(Instant::now() + timeout));
+        *self.timeout.lock().unwrap() = Some(Instant::now() + timeout);
     }
 
     pub(crate) fn stop(&self) {
-        self.timeout.store(None);
+        *self.timeout.lock().unwrap() = None;
     }
 }
diff --git a/src/body/mod.rs b/src/body/mod.rs
index 5830621..35fe137 100644
--- a/src/body/mod.rs
+++ b/src/body/mod.rs
@@ -314,7 +314,7 @@ mod tests {
     fn cannot_reset_reader() {
         let mut body = AsyncBody::from_reader(futures_lite::io::empty());
 
-        assert_eq!(body.reset(), false);
+        assert!(!body.reset());
     }
 
     #[test]
diff --git a/src/body/sync.rs b/src/body/sync.rs
index e035dee..73e1abe 100644
--- a/src/body/sync.rs
+++ b/src/body/sync.rs
@@ -334,6 +334,6 @@ mod tests {
     fn cannot_reset_reader() {
         let mut body = Body::from_reader(std::io::empty());
 
-        assert_eq!(body.reset(), false);
+        assert!(!body.reset());
     }
 }
diff --git a/src/client.rs b/src/client.rs
index 835b1bf..0514e78 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -104,7 +104,7 @@ impl HttpClientBuilder {
                 // and thus the outermost, interceptor. Also note that this does
                 // not enable redirect following, it just implements support for
                 // it, if a request asks for it.
-                InterceptorObj::new(crate::redirect::RedirectInterceptor),
+                crate::redirect::RedirectInterceptor.into(),
             ],
             default_headers: HeaderMap::new(),
             error: None,
@@ -167,7 +167,7 @@ impl HttpClientBuilder {
 
     #[allow(unused)]
     pub(crate) fn interceptor_impl(mut self, interceptor: impl Interceptor + 'static) -> Self {
-        self.interceptors.push(InterceptorObj::new(interceptor));
+        self.interceptors.push(interceptor.into());
         self
     }
 
@@ -1064,16 +1064,14 @@ impl HttpClient {
 
         easy.signal(false)?;
 
-        let request_config = request
-            .extensions()
-            .get::<RequestConfig>()
-            .unwrap();
+        let request_config = request.extensions().get::<RequestConfig>().unwrap();
 
         request_config.set_opt(&mut easy)?;
         self.inner.client_config.set_opt(&mut easy)?;
 
         // Check if we need to disable the Expect header.
-        let disable_expect_header = request_config.expect_continue
+        let disable_expect_header = request_config
+            .expect_continue
             .as_ref()
             .map(|x| x.is_disabled())
             .unwrap_or_default();
@@ -1324,13 +1322,10 @@ mod tests {
 
     #[test]
     fn test_default_header() {
-        let client = HttpClientBuilder::new()
+        HttpClientBuilder::new()
             .default_header("some-key", "some-value")
-            .build();
-        match client {
-            Ok(_) => assert!(true),
-            Err(_) => assert!(false),
-        }
+            .build()
+            .expect("build client succeed");
     }
 
     #[test]
diff --git a/src/config/dial.rs b/src/config/dial.rs
index 2e21136..6d7293b 100644
--- a/src/config/dial.rs
+++ b/src/config/dial.rs
@@ -202,9 +202,7 @@ mod tests {
 
     #[test]
     fn parse_invalid_tcp_uri() {
-        let result = "tcp:127.0.0.1-1200".parse::<Dialer>();
-
-        assert!(result.is_err());
+        "tcp:127.0.0.1-1200".parse::<Dialer>().unwrap_err();
     }
 
     #[test]
diff --git a/src/cookies/cookie.rs b/src/cookies/cookie.rs
index 281f001..0405e74 100644
--- a/src/cookies/cookie.rs
+++ b/src/cookies/cookie.rs
@@ -73,6 +73,10 @@ impl CookieBuilder {
     }
 
     /// Sets the domain the cookie belongs to.
+    ///
+    /// Note that the domain will be preserved exactly as supplied. If the
+    /// string has a `.` prefix that should be ignored (such as allowed by the
+    /// `Set-Cookie` HTTP header), then you must remove it manually first.
     pub fn domain<S>(mut self, domain: S) -> Self
     where
         S: Into<String>,
@@ -438,7 +442,7 @@ mod tests {
     #[test_case("bad_value_comma=bar,")]
     #[test_case("bad_value_space= bar")]
     fn parse_invalid(s: &str) {
-        assert!(Cookie::parse(s).is_err());
+        Cookie::parse(s).unwrap_err();
     }
 
     #[test_case("foo=bar")]
diff --git a/src/cookies/interceptor.rs b/src/cookies/interceptor.rs
index fc7bd20..408c066 100644
--- a/src/cookies/interceptor.rs
+++ b/src/cookies/interceptor.rs
@@ -19,7 +19,9 @@ pub(crate) struct CookieInterceptor {
 
 impl CookieInterceptor {
     pub(crate) fn new(cookie_jar: Option<CookieJar>) -> Self {
-        Self { cookie_jar }
+        Self {
+            cookie_jar,
+        }
     }
 }
 
diff --git a/src/cookies/jar.rs b/src/cookies/jar.rs
index 9aa23e2..46b92d6 100644
--- a/src/cookies/jar.rs
+++ b/src/cookies/jar.rs
@@ -347,24 +347,23 @@ mod tests {
     fn cookie_domain_not_allowed() {
         let jar = CookieJar::default();
 
-        assert!(jar
-            .set(
-                Cookie::parse("foo=bar").unwrap(),
-                &"https://bar.baz.com".parse().unwrap()
-            )
-            .is_ok());
-        assert!(jar
-            .set(
-                Cookie::parse("foo=bar; domain=bar.baz.com").unwrap(),
-                &"https://bar.baz.com".parse().unwrap()
-            )
-            .is_ok());
-        assert!(jar
-            .set(
-                Cookie::parse("foo=bar; domain=baz.com").unwrap(),
-                &"https://bar.baz.com".parse().unwrap()
-            )
-            .is_ok());
+        jar.set(
+            Cookie::parse("foo=bar").unwrap(),
+            &"https://bar.baz.com".parse().unwrap(),
+        )
+        .unwrap();
+
+        jar.set(
+            Cookie::parse("foo=bar; domain=bar.baz.com").unwrap(),
+            &"https://bar.baz.com".parse().unwrap(),
+        )
+        .unwrap();
+
+        jar.set(
+            Cookie::parse("foo=bar; domain=baz.com").unwrap(),
+            &"https://bar.baz.com".parse().unwrap(),
+        )
+        .unwrap();
 
         assert!(
             jar.set(
diff --git a/src/cookies/psl/list b/src/cookies/psl/list
index 172bbfd..3088234 160000
--- a/src/cookies/psl/list
+++ b/src/cookies/psl/list
@@ -1,1 +1,1 @@
-Subproject commit 172bbfd969ea3438858c3d6b13183b36eaa7282d
+Subproject commit 30882342b60b540fd629ed67efd4763322bd47bd
diff --git a/src/cookies/psl/mod.rs b/src/cookies/psl/mod.rs
index c3a3f10..aa5404b 100644
--- a/src/cookies/psl/mod.rs
+++ b/src/cookies/psl/mod.rs
@@ -23,7 +23,10 @@ use crate::{request::RequestExt, ReadResponseExt};
 use once_cell::sync::Lazy;
 use parking_lot::{RwLock, RwLockUpgradableReadGuard};
 use publicsuffix::{List, Psl};
-use std::{error::Error, time::{Duration, SystemTime}};
+use std::{
+    error::Error,
+    time::{Duration, SystemTime},
+};
 
 /// How long should we use a cached list before refreshing?
 static TTL: Lazy<Duration> = Lazy::new(|| Duration::from_secs(24 * 60 * 60));
@@ -79,7 +82,10 @@ impl ListCache {
         let mut request = http::Request::get(publicsuffix::LIST_URL);
 
         if let Some(last_updated) = self.last_updated {
-            request = request.header(http::header::IF_MODIFIED_SINCE, httpdate::fmt_http_date(last_updated));
+            request = request.header(
+                http::header::IF_MODIFIED_SINCE,
+                httpdate::fmt_http_date(last_updated),
+            );
         }
 
         let mut response = request.body(())?.send()?;
@@ -144,8 +150,8 @@ fn with_cache<T>(f: impl FnOnce(&ListCache) -> T) -> T {
             }
         }
 
-        f(&*cache)
+        f(&cache)
     } else {
-        f(&*cache)
+        f(&cache)
     }
 }
diff --git a/src/handler.rs b/src/handler.rs
index 92445f6..27c34a8 100644
--- a/src/handler.rs
+++ b/src/handler.rs
@@ -538,7 +538,8 @@ impl curl::easy::Handler for RequestHandler {
                         if !self.disable_connection_reuse_log
                             && self.response_version < Some(http::Version::HTTP_2)
                         {
-                            tracing::info!("\
+                            tracing::info!(
+                                "\
                                 response dropped without fully consuming the response body, connection won't be reused\n\
                                 Aborting a response without fully consuming the response body can result in sub-optimal \
                                 performance. See https://github.com/sagebind/isahc/wiki/Connection-Reuse#closing-connections-early."
diff --git a/src/interceptor/mod.rs b/src/interceptor/mod.rs
index 0eb32e1..43b27b1 100644
--- a/src/interceptor/mod.rs
+++ b/src/interceptor/mod.rs
@@ -11,15 +11,6 @@
 //!   generic over a lifetime, there's no way to give the return type the
 //!   correct name using current Rust syntax.
 //! - [`InterceptorObj`] wraps the returned future in an extra box.
-//! - If an interceptor returns a custom error, it is stringified and wrapped in
-//!   `Error::Curl`. We should introduce a new error variant that boxes the
-//!   error and also records the type of the interceptor that created the error
-//!   for visibility. But we can't add a new variant right now without a BC
-//!   break. See [#182](https://github.com/sagebind/isahc/issues/182).
-//! - Automatic redirect following currently bypasses interceptors for
-//!   subsequent requests. This will be fixed when redirect handling is
-//!   rewritten as an interceptor itself. See
-//!   [#232](https://github.com/sagebind/isahc/issues/232).
 ///
 /// # Availability
 ///
diff --git a/src/interceptor/obj.rs b/src/interceptor/obj.rs
index e9176e7..d0f0c2f 100644
--- a/src/interceptor/obj.rs
+++ b/src/interceptor/obj.rs
@@ -5,13 +5,16 @@ use http::Request;
 /// Type-erased interceptor object.
 pub(crate) struct InterceptorObj(Box<dyn DynInterceptor>);
 
-impl InterceptorObj {
-    pub(crate) fn new(interceptor: impl Interceptor + 'static) -> Self {
+impl<I> From<I> for InterceptorObj
+where
+    I: Interceptor + 'static,
+{
+    fn from(interceptor: I) -> Self {
         Self(Box::new(interceptor))
     }
 }
 
-impl Interceptor for InterceptorObj {
+impl Interceptor for &InterceptorObj {
     type Err = Error;
 
     fn intercept<'a>(
diff --git a/src/lib.rs b/src/lib.rs
index 1b0a7bd..e9813c9 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -234,7 +234,6 @@
     future_incompatible,
     missing_debug_implementations,
     missing_docs,
-    rust_2018_idioms,
     unreachable_pub,
     unused,
     clippy::all
diff --git a/src/response.rs b/src/response.rs
index 2bc1c51..e2dfd18 100644
--- a/src/response.rs
+++ b/src/response.rs
@@ -520,7 +520,8 @@ fn allocate_buffer<T>(response: &Response<T>) -> Vec<u8> {
 }
 
 fn get_content_length<T>(response: &Response<T>) -> Option<u64> {
-    response.headers()
+    response
+        .headers()
         .get(http::header::CONTENT_LENGTH)?
         .to_str()
         .ok()?


Test patch:
diff --git a/tests/cookies.rs b/tests/cookies.rs
index 6be882d..54eaa68 100644
--- a/tests/cookies.rs
+++ b/tests/cookies.rs
@@ -6,10 +6,7 @@ use testserver::mock;
 #[test]
 fn cookie_lifecycle() {
     let jar = CookieJar::default();
-    let client = HttpClient::builder()
-        .cookie_jar(jar.clone())
-        .build()
-        .unwrap();
+    let client = HttpClient::builder().cookie_jar(jar).build().unwrap();
 
     let m1 = mock! {
         headers {
diff --git a/tests/expect.rs b/tests/expect.rs
index 820df70..94d56d5 100644
--- a/tests/expect.rs
+++ b/tests/expect.rs
@@ -1,4 +1,4 @@
-use isahc::{Body, Request, prelude::*};
+use isahc::{prelude::*, Body, Request};
 use testserver::mock;
 
 #[test]
diff --git a/tests/headers.rs b/tests/headers.rs
index 8414312..07049d5 100644
--- a/tests/headers.rs
+++ b/tests/headers.rs
@@ -221,7 +221,6 @@ fn trailer_headers() {
     let mut body = None;
     let response = isahc::get(url).unwrap().map(|b| {
         body = Some(b);
-        ()
     });
 
     thread::spawn(move || {
@@ -264,7 +263,6 @@ fn trailer_headers_async() {
         let mut body = None;
         let response = isahc::get_async(url).await.unwrap().map(|b| {
             body = Some(b);
-            ()
         });
 
         thread::spawn(move || {
diff --git a/tests/net.rs b/tests/net.rs
index d133dc7..0fb373b 100644
--- a/tests/net.rs
+++ b/tests/net.rs
@@ -111,7 +111,7 @@ fn any_ip_version_uses_ipv4_or_ipv6() {
     let server_v6 = TcpListener::bind((Ipv6Addr::LOCALHOST, port)).unwrap();
 
     fn respond(client: &mut TcpStream, response: &[u8]) -> io::Result<()> {
-        client.read(&mut [0; 8192])?;
+        let _ = client.read(&mut [0; 8192])?;
         client.write_all(response)?;
         client.flush()?;
         client.shutdown(Shutdown::Both)
diff --git a/tests/redirects.rs b/tests/redirects.rs
index 814db25..c4472b3 100644
--- a/tests/redirects.rs
+++ b/tests/redirects.rs
@@ -282,8 +282,14 @@ fn auto_referer_sets_expected_header() {
         .unwrap();
 
     assert_eq!(m1.request().get_header("Referer").count(), 0);
-    assert_eq!(m2.request().get_header("Referer").collect::<Vec<_>>(), vec![m1.url()]);
-    assert_eq!(m3.request().get_header("Referer").collect::<Vec<_>>(), vec![m2.url()]);
+    assert_eq!(
+        m2.request().get_header("Referer").collect::<Vec<_>>(),
+        vec![m1.url()]
+    );
+    assert_eq!(
+        m3.request().get_header("Referer").collect::<Vec<_>>(),
+        vec![m2.url()]
+    );
 }
 
 #[test]
diff --git a/tests/response_body.rs b/tests/response_body.rs
index a21ea75..692ae92 100644
--- a/tests/response_body.rs
+++ b/tests/response_body.rs
@@ -153,7 +153,7 @@ fn consume_unread_response_body() {
     let body = "wow so large ".repeat(1000);
 
     let m = {
-        let body = body.clone();
+        let body = body;
         mock! {
             _ => {
                 body: body.clone(),
@@ -173,7 +173,7 @@ fn consume_unread_response_body_async() {
     let body = "wow so large ".repeat(1000);
 
     let m = {
-        let body = body.clone();
+        let body = body;
         mock! {
             _ => {
                 body: body.clone(),
diff --git a/testserver/src/mock.rs b/testserver/src/mock.rs
index a02adaa..7259efb 100644
--- a/testserver/src/mock.rs
+++ b/testserver/src/mock.rs
@@ -108,8 +108,7 @@ impl Mock {
         if request
             .headers()
             .iter()
-            .find(|h| h.field.as_str() == "host" && h.value == "api.mock.local")
-            .is_some()
+            .any(|h| h.field.as_str() == "host" && h.value == "api.mock.local")
         {
             self.handle_api_request(request);
             return;
diff --git a/testserver/src/pool.rs b/testserver/src/pool.rs
index 2e715e0..cbe820e 100644
--- a/testserver/src/pool.rs
+++ b/testserver/src/pool.rs
@@ -11,9 +11,7 @@ use threadfin::ThreadPool;
 pub(crate) fn pool() -> &'static ThreadPool {
     // Pool that crates pretty much as many threads as needed, while still
     // allowing reuse.
-    static POOL: Lazy<ThreadPool> = Lazy::new(|| ThreadPool::builder()
-        .size(..100)
-        .build());
+    static POOL: Lazy<ThreadPool> = Lazy::new(|| ThreadPool::builder().size(..100).build());
 
-    &*POOL
+    &POOL
 }
diff --git a/testserver/src/socks4.rs b/testserver/src/socks4.rs
index 6d428d2..8aef041 100644
--- a/testserver/src/socks4.rs
+++ b/testserver/src/socks4.rs
@@ -31,19 +31,12 @@ impl Socks4Server {
     }
 
     pub fn run(&self) {
-        for stream in self.listener.incoming() {
-            match stream {
-                Ok(connection) => {
-                    let s = self.clone();
-
-                    pool().execute(move || {
-                        s.handle(connection).unwrap();
-                    });
-                }
-                Err(_) => {
-                    // ignore
-                }
-            }
+        for connection in self.listener.incoming().flatten() {
+            let s = self.clone();
+
+            pool().execute(move || {
+                s.handle(connection).unwrap();
+            });
         }
     }