fix: reduce expensive clones of BaseSettings (#186)

Use `Arc`'s clone-on-write capabilities to only clone data when absolutely necessary.

Author: sbstp

src/request/builder.rs

@@ -2,6 +2,7 @@ use std::borrow::Borrow;
 use std::convert::{From, TryInto};
 use std::fs;
 use std::str;
+use std::sync::Arc;
 use std::time::Duration;
 
 use base64::Engine;
@@ -38,7 +39,10 @@ pub struct RequestBuilder<B = body::Empty> {
     url: Url,
     method: Method,
     body: B,
-    base_settings: BaseSettings,
+    // Headers are always modified by [Self::try_prepare], it's cheaper to keep them
+    // separate than to clone all of BaseSettings through [Arc::make_mut].
+    headers: HeaderMap,
+    base_settings: Arc<BaseSettings>,
 }
 
 impl RequestBuilder {
@@ -61,17 +65,17 @@ impl RequestBuilder {
     where
         U: AsRef<str>,
     {
-        Self::try_with_settings(method, base_url, BaseSettings::default())
+        Self::try_with_settings(method, base_url, Arc::new(BaseSettings::default()))
     }
 
-    pub(crate) fn with_settings<U>(method: Method, base_url: U, base_settings: BaseSettings) -> Self
+    pub(crate) fn with_settings<U>(method: Method, base_url: U, base_settings: Arc<BaseSettings>) -> Self
     where
         U: AsRef<str>,
     {
         Self::try_with_settings(method, base_url, base_settings).expect("invalid url or method")
     }
 
-    pub(crate) fn try_with_settings<U>(method: Method, base_url: U, base_settings: BaseSettings) -> Result<Self>
+    pub(crate) fn try_with_settings<U>(method: Method, base_url: U, base_settings: Arc<BaseSettings>) -> Result<Self>
     where
         U: AsRef<str>,
     {
@@ -85,6 +89,7 @@ impl RequestBuilder {
             url,
             method,
             body: body::Empty,
+            headers: base_settings.headers.clone(),
             base_settings,
         })
     }
@@ -161,6 +166,7 @@ impl<B> RequestBuilder<B> {
             url: self.url,
             method: self.method,
             body,
+            headers: self.headers,
             base_settings: self.base_settings,
         }
     }
@@ -169,8 +175,7 @@ impl<B> RequestBuilder<B> {
     ///
     /// If the `Content-Type` header is unset, it will be set to `text/plain` and the charset to UTF-8.
     pub fn text<B1: AsRef<str>>(mut self, body: B1) -> RequestBuilder<body::Text<B1>> {
-        self.base_settings
-            .headers
+        self.headers
             .entry(http::header::CONTENT_TYPE)
             .or_insert(HeaderValue::from_static("text/plain; charset=utf-8"));
         self.body(body::Text(body))
@@ -180,8 +185,7 @@ impl<B> RequestBuilder<B> {
     ///
     /// If the `Content-Type` header is unset, it will be set to `application/octet-stream`.
     pub fn bytes<B1: AsRef<[u8]>>(mut self, body: B1) -> RequestBuilder<body::Bytes<B1>> {
-        self.base_settings
-            .headers
+        self.headers
             .entry(http::header::CONTENT_TYPE)
             .or_insert(HeaderValue::from_static("application/octet-stream"));
         self.body(body::Bytes(body))
@@ -191,8 +195,7 @@ impl<B> RequestBuilder<B> {
     ///
     /// If the `Content-Type` header is unset, it will be set to `application/octet-stream`.
     pub fn file(mut self, body: fs::File) -> RequestBuilder<body::File> {
-        self.base_settings
-            .headers
+        self.headers
             .entry(http::header::CONTENT_TYPE)
             .or_insert(HeaderValue::from_static("application/octet-stream"));
         self.body(body::File(body))
@@ -204,8 +207,7 @@ impl<B> RequestBuilder<B> {
     #[cfg(feature = "json")]
     pub fn json<T: serde::Serialize>(mut self, value: &T) -> Result<RequestBuilder<body::Bytes<Vec<u8>>>> {
         let body = serde_json::to_vec(value)?;
-        self.base_settings
-            .headers
+        self.headers
             .entry(http::header::CONTENT_TYPE)
             .or_insert(HeaderValue::from_static("application/json; charset=utf-8"));
         Ok(self.body(body::Bytes(body)))
@@ -216,8 +218,7 @@ impl<B> RequestBuilder<B> {
     /// If the `Content-Type` header is unset, it will be set to `application/json` and the charset to UTF-8.
     #[cfg(feature = "json")]
     pub fn json_streaming<T: serde::Serialize>(mut self, value: T) -> RequestBuilder<body::Json<T>> {
-        self.base_settings
-            .headers
+        self.headers
             .entry(http::header::CONTENT_TYPE)
             .or_insert(HeaderValue::from_static("application/json; charset=utf-8"));
         self.body(body::Json(value))
@@ -229,13 +230,22 @@ impl<B> RequestBuilder<B> {
     #[cfg(feature = "form")]
     pub fn form<T: serde::Serialize>(mut self, value: &T) -> Result<RequestBuilder<body::Bytes<Vec<u8>>>> {
         let body = serde_urlencoded::to_string(value)?.into_bytes();
-        self.base_settings
-            .headers
+        self.headers
             .entry(http::header::CONTENT_TYPE)
             .or_insert(HeaderValue::from_static("application/x-www-form-urlencoded"));
         Ok(self.body(body::Bytes(body)))
     }
 
+    /// Get a mutable reference to headers.
+    pub fn headers(&self) -> &HeaderMap {
+        &self.headers
+    }
+
+    /// Get a mutable reference to headers.
+    pub fn headers_mut(&mut self) -> &mut HeaderMap {
+        &mut self.headers
+    }
+
     //
     // Settings
     //
@@ -281,7 +291,7 @@ impl<B> RequestBuilder<B> {
         V: TryInto<HeaderValue>,
         Error: From<V::Error>,
     {
-        header_insert(&mut self.base_settings.headers, header, value)?;
+        header_insert(&mut self.headers, header, value)?;
         Ok(self)
     }
 
@@ -294,68 +304,63 @@ impl<B> RequestBuilder<B> {
         V: TryInto<HeaderValue>,
         Error: From<V::Error>,
     {
-        header_append(&mut self.base_settings.headers, header, value)?;
+        header_append(&mut self.headers, header, value)?;
         Ok(self)
     }
 
     /// Set the maximum number of headers accepted in responses to this request.
     ///
     /// The default is 100.
     pub fn max_headers(mut self, max_headers: usize) -> Self {
-        self.base_settings.max_headers = max_headers;
+        self.base_settings.set_max_headers(max_headers);
         self
     }
 
-    /// Get a mutable reference to headers.
-    pub fn headers_mut(&mut self) -> &mut HeaderMap {
-        &mut self.base_settings.headers
-    }
-
     /// Set the maximum number of redirections this request can perform.
     ///
     /// The default is 5.
     pub fn max_redirections(mut self, max_redirections: u32) -> Self {
-        self.base_settings.max_redirections = max_redirections;
+        self.base_settings.set_max_redirections(max_redirections);
         self
     }
 
     /// Sets if this request should follow redirects, 3xx codes.
     ///
     /// This value defaults to true.
     pub fn follow_redirects(mut self, follow_redirects: bool) -> Self {
-        self.base_settings.follow_redirects = follow_redirects;
+        self.base_settings.set_follow_redirects(follow_redirects);
         self
     }
 
     /// Sets a connect timeout for this request.
     ///
     /// The default is 30 seconds.
-    pub fn connect_timeout(mut self, duration: Duration) -> Self {
-        self.base_settings.connect_timeout = duration;
+    pub fn connect_timeout(mut self, connect_timeout: Duration) -> Self {
+        self.base_settings.set_connect_timeout(connect_timeout);
         self
     }
 
     /// Sets a read timeout for this request.
     ///
     /// The default is 30 seconds.
-    pub fn read_timeout(mut self, duration: Duration) -> Self {
-        self.base_settings.read_timeout = duration;
+    pub fn read_timeout(mut self, read_timeout: Duration) -> Self {
+        self.base_settings.set_read_tmeout(read_timeout);
         self
     }
 
     /// Sets a timeout for the whole request.
     ///
     /// Applies after a TCP connection is established. Defaults to no timeout.
-    pub fn timeout(mut self, duration: Duration) -> Self {
-        self.base_settings.timeout = Some(duration);
+    pub fn timeout(mut self, timeout: Duration) -> Self {
+        self.base_settings.set_timeout(Some(timeout));
         self
     }
 
     /// Sets the proxy settigns for this request.
     ///
     /// If left untouched, the defaults are to use system proxy settings found in environment variables.
-    pub fn proxy_settings(mut self, settings: ProxySettings) -> Self {
-        self.base_settings.proxy_settings = settings;
+    pub fn proxy_settings(mut self, proxy_settings: ProxySettings) -> Self {
+        self.base_settings.set_proxy_settings(proxy_settings);
         self
     }
 
@@ -365,7 +370,7 @@ impl<B> RequestBuilder<B> {
     /// This value defaults to `None`, in which case ISO-8859-1 is used.
     #[cfg(feature = "charsets")]
     pub fn default_charset(mut self, default_charset: Option<Charset>) -> Self {
-        self.base_settings.default_charset = default_charset;
+        self.base_settings.set_default_charset(default_charset);
         self
     }
 
@@ -375,7 +380,7 @@ impl<B> RequestBuilder<B> {
     /// compression, the server might choose not to compress the content.
     #[cfg(feature = "flate2")]
     pub fn allow_compression(mut self, allow_compression: bool) -> Self {
-        self.base_settings.allow_compression = allow_compression;
+        self.base_settings.set_allow_compression(allow_compression);
         self
     }
 
@@ -391,7 +396,7 @@ impl<B> RequestBuilder<B> {
     /// If you are using self signed certificates, it is much safer to add their root CA
     /// to the list of trusted root CAs by your system.
     pub fn danger_accept_invalid_certs(mut self, accept_invalid_certs: bool) -> Self {
-        self.base_settings.accept_invalid_certs = accept_invalid_certs;
+        self.base_settings.set_accept_invalid_certs(accept_invalid_certs);
         self
     }
 
@@ -403,13 +408,14 @@ impl<B> RequestBuilder<B> {
     /// Use this setting with care. This will accept TLS certificates that do not match
     /// the hostname.
     pub fn danger_accept_invalid_hostnames(mut self, accept_invalid_hostnames: bool) -> Self {
-        self.base_settings.accept_invalid_hostnames = accept_invalid_hostnames;
+        self.base_settings
+            .set_accept_invalid_hostnames(accept_invalid_hostnames);
         self
     }
 
     /// Adds a root certificate that will be trusted.
     pub fn add_root_certificate(mut self, cert: Certificate) -> Self {
-        self.base_settings.root_certificates.0.push(cert);
+        self.base_settings.add_root_certificate(cert);
         self
     }
 }
@@ -429,27 +435,29 @@ impl<B: Body> RequestBuilder<B> {
             url: self.url,
             method: self.method,
             body: self.body,
+            headers: self.headers,
             base_settings: self.base_settings,
         };
-
-        header_insert(&mut prepped.base_settings.headers, CONNECTION, "close")?;
         prepped.set_compression()?;
+        let headers = &mut prepped.headers;
+
+        header_insert(headers, CONNECTION, "close")?;
         match prepped.body.kind()? {
             BodyKind::Empty => (),
             BodyKind::KnownLength(len) => {
-                header_insert(&mut prepped.base_settings.headers, CONTENT_LENGTH, len)?;
+                header_insert(headers, CONTENT_LENGTH, len)?;
             }
             BodyKind::Chunked => {
-                header_insert(&mut prepped.base_settings.headers, TRANSFER_ENCODING, "chunked")?;
+                header_insert(headers, TRANSFER_ENCODING, "chunked")?;
             }
         }
 
         if let Some(typ) = prepped.body.content_type()? {
-            header_insert(&mut prepped.base_settings.headers, CONTENT_TYPE, typ)?;
+            header_insert(headers, CONTENT_TYPE, typ)?;
         }
 
-        header_insert_if_missing(&mut prepped.base_settings.headers, ACCEPT, "*/*")?;
-        header_insert_if_missing(&mut prepped.base_settings.headers, USER_AGENT, DEFAULT_USER_AGENT)?;
+        header_insert_if_missing(headers, ACCEPT, "*/*")?;
+        header_insert_if_missing(headers, USER_AGENT, DEFAULT_USER_AGENT)?;
 
         Ok(prepped)
     }
@@ -489,7 +497,7 @@ impl<B> RequestInspector<'_, B> {
 
     /// Acess the current headers
     pub fn headers(&self) -> &HeaderMap {
-        &self.0.base_settings.headers
+        &self.0.headers
     }
 }