From b22744c8a25a066fd62473496d06cdea25f6cd25 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 4 Sep 2023 23:11:00 +1000 Subject: [PATCH 1/4] Optimize initialization of `TrustDnsResolver` Replace use of `tokio::sync::Mutex` with `once_cell::sync::OnceCell` since there's no need for it to be `async` and thus it can just use `OnceCell` instead of `Mutex`. Signed-off-by: Jiahao XU --- src/dns/trust_dns.rs | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/src/dns/trust_dns.rs b/src/dns/trust_dns.rs index a1f3a1b03..d68f29e57 100644 --- a/src/dns/trust_dns.rs +++ b/src/dns/trust_dns.rs @@ -1,8 +1,7 @@ //! DNS resolution via the [trust_dns_resolver](https://github.com/bluejekyll/trust-dns) crate use hyper::client::connect::dns::Name; -use once_cell::sync::Lazy; -use tokio::sync::Mutex; +use once_cell::sync::{Lazy, OnceCell}; pub use trust_dns_resolver::config::{ResolverConfig, ResolverOpts}; use trust_dns_resolver::{lookup_ip::LookupIpIntoIter, system_conf, TokioAsyncResolver}; @@ -20,19 +19,13 @@ static SYSTEM_CONF: Lazy> = /// Wrapper around an `AsyncResolver`, which implements the `Resolve` trait. #[derive(Debug, Clone)] pub(crate) struct TrustDnsResolver { - state: Arc>, + state: Arc>, } struct SocketAddrs { iter: LookupIpIntoIter, } -#[derive(Debug)] -enum State { - Init, - Ready(SharedResolver), -} - impl TrustDnsResolver { /// Create a new resolver with the default configuration, /// which reads from `/etc/resolve.conf`. @@ -45,7 +38,7 @@ impl TrustDnsResolver { // Tokio Runtime, so we must delay the actual construction of the // resolver. Ok(TrustDnsResolver { - state: Arc::new(Mutex::new(State::Init)), + state: Arc::new(OnceCell::new()), }) } } @@ -54,20 +47,7 @@ impl Resolve for TrustDnsResolver { fn resolve(&self, name: Name) -> Resolving { let resolver = self.clone(); Box::pin(async move { - let mut lock = resolver.state.lock().await; - - let resolver = match &*lock { - State::Init => { - let resolver = new_resolver().await; - *lock = State::Ready(resolver.clone()); - resolver - } - State::Ready(resolver) => resolver.clone(), - }; - - // Don't keep lock once the resolver is constructed, otherwise - // only one lookup could be done at a time. - drop(lock); + let resolver = resolver.state.get_or_init(new_resolver); let lookup = resolver.lookup_ip(name.as_str()).await?; let addrs: Addrs = Box::new(SocketAddrs { @@ -86,7 +66,7 @@ impl Iterator for SocketAddrs { } } -async fn new_resolver() -> SharedResolver { +fn new_resolver() -> SharedResolver { let (config, opts) = SYSTEM_CONF .as_ref() .expect("can't construct TrustDnsResolver if SYSTEM_CONF is error") From 4ec6128bcc3da1231ddb9d92dc75bdf52d620339 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 4 Sep 2023 23:15:48 +1000 Subject: [PATCH 2/4] Optimize `TrustDnsResolver`: Rm use of `Lazy` Signed-off-by: Jiahao XU --- src/dns/trust_dns.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/dns/trust_dns.rs b/src/dns/trust_dns.rs index d68f29e57..951d65a66 100644 --- a/src/dns/trust_dns.rs +++ b/src/dns/trust_dns.rs @@ -1,7 +1,7 @@ //! DNS resolution via the [trust_dns_resolver](https://github.com/bluejekyll/trust-dns) crate use hyper::client::connect::dns::Name; -use once_cell::sync::{Lazy, OnceCell}; +use once_cell::sync::OnceCell; pub use trust_dns_resolver::config::{ResolverConfig, ResolverOpts}; use trust_dns_resolver::{lookup_ip::LookupIpIntoIter, system_conf, TokioAsyncResolver}; @@ -13,9 +13,6 @@ use super::{Addrs, Resolve, Resolving}; type SharedResolver = Arc; -static SYSTEM_CONF: Lazy> = - Lazy::new(|| system_conf::read_system_conf().map_err(io::Error::from)); - /// Wrapper around an `AsyncResolver`, which implements the `Resolve` trait. #[derive(Debug, Clone)] pub(crate) struct TrustDnsResolver { @@ -30,10 +27,6 @@ impl TrustDnsResolver { /// Create a new resolver with the default configuration, /// which reads from `/etc/resolve.conf`. pub fn new() -> io::Result { - SYSTEM_CONF.as_ref().map_err(|e| { - io::Error::new(e.kind(), format!("error reading DNS system conf: {}", e)) - })?; - // At this stage, we might not have been called in the context of a // Tokio Runtime, so we must delay the actual construction of the // resolver. @@ -47,7 +40,7 @@ impl Resolve for TrustDnsResolver { fn resolve(&self, name: Name) -> Resolving { let resolver = self.clone(); Box::pin(async move { - let resolver = resolver.state.get_or_init(new_resolver); + let resolver = resolver.state.get_or_try_init(new_resolver)?; let lookup = resolver.lookup_ip(name.as_str()).await?; let addrs: Addrs = Box::new(SocketAddrs { @@ -66,12 +59,10 @@ impl Iterator for SocketAddrs { } } -fn new_resolver() -> SharedResolver { - let (config, opts) = SYSTEM_CONF - .as_ref() - .expect("can't construct TrustDnsResolver if SYSTEM_CONF is error") - .clone(); - new_resolver_with_config(config, opts) +fn new_resolver() -> io::Result { + let (config, opts) = system_conf::read_system_conf() + .map_err(|e| io::Error::new(e.kind(), format!("error reading DNS system conf: {}", e)))?; + Ok(new_resolver_with_config(config, opts)) } fn new_resolver_with_config(config: ResolverConfig, opts: ResolverOpts) -> SharedResolver { From 330ce57963584369196f397b024ff7b894c3a90d Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 4 Sep 2023 23:17:36 +1000 Subject: [PATCH 3/4] Optimize `TrustDnsResolver`: Rm unnecessary `Arc` Signed-off-by: Jiahao XU --- src/dns/trust_dns.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/dns/trust_dns.rs b/src/dns/trust_dns.rs index 951d65a66..8037871c0 100644 --- a/src/dns/trust_dns.rs +++ b/src/dns/trust_dns.rs @@ -11,12 +11,10 @@ use std::sync::Arc; use super::{Addrs, Resolve, Resolving}; -type SharedResolver = Arc; - /// Wrapper around an `AsyncResolver`, which implements the `Resolve` trait. #[derive(Debug, Clone)] pub(crate) struct TrustDnsResolver { - state: Arc>, + state: Arc>, } struct SocketAddrs { @@ -59,12 +57,8 @@ impl Iterator for SocketAddrs { } } -fn new_resolver() -> io::Result { +fn new_resolver() -> io::Result { let (config, opts) = system_conf::read_system_conf() .map_err(|e| io::Error::new(e.kind(), format!("error reading DNS system conf: {}", e)))?; - Ok(new_resolver_with_config(config, opts)) -} - -fn new_resolver_with_config(config: ResolverConfig, opts: ResolverOpts) -> SharedResolver { - Arc::new(TokioAsyncResolver::tokio(config, opts)) + Ok(TokioAsyncResolver::tokio(config, opts)) } From 57f289a4f09abafda2f5724f89d3bba4efeffecc Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 4 Sep 2023 23:20:56 +1000 Subject: [PATCH 4/4] Derive `Default` on `TrustDnsResolver` instead of implementing `TrustDnsResolver::new()` Signed-off-by: Jiahao XU --- src/async_impl/client.rs | 2 +- src/dns/trust_dns.rs | 20 ++++++-------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs index 44cff22ac..d724055cd 100644 --- a/src/async_impl/client.rs +++ b/src/async_impl/client.rs @@ -266,7 +266,7 @@ impl ClientBuilder { let mut resolver: Arc = match config.trust_dns { false => Arc::new(GaiResolver::new()), #[cfg(feature = "trust-dns")] - true => Arc::new(TrustDnsResolver::new().map_err(crate::error::builder)?), + true => Arc::new(TrustDnsResolver::default()), #[cfg(not(feature = "trust-dns"))] true => unreachable!("trust-dns shouldn't be enabled unless the feature is"), }; diff --git a/src/dns/trust_dns.rs b/src/dns/trust_dns.rs index 8037871c0..3a5abda81 100644 --- a/src/dns/trust_dns.rs +++ b/src/dns/trust_dns.rs @@ -12,8 +12,11 @@ use std::sync::Arc; use super::{Addrs, Resolve, Resolving}; /// Wrapper around an `AsyncResolver`, which implements the `Resolve` trait. -#[derive(Debug, Clone)] +#[derive(Debug, Default, Clone)] pub(crate) struct TrustDnsResolver { + /// Since we might not have been called in the context of a + /// Tokio Runtime in initialization, so we must delay the actual + /// construction of the resolver. state: Arc>, } @@ -21,19 +24,6 @@ struct SocketAddrs { iter: LookupIpIntoIter, } -impl TrustDnsResolver { - /// Create a new resolver with the default configuration, - /// which reads from `/etc/resolve.conf`. - pub fn new() -> io::Result { - // At this stage, we might not have been called in the context of a - // Tokio Runtime, so we must delay the actual construction of the - // resolver. - Ok(TrustDnsResolver { - state: Arc::new(OnceCell::new()), - }) - } -} - impl Resolve for TrustDnsResolver { fn resolve(&self, name: Name) -> Resolving { let resolver = self.clone(); @@ -57,6 +47,8 @@ impl Iterator for SocketAddrs { } } +/// Create a new resolver with the default configuration, +/// which reads from `/etc/resolve.conf`. fn new_resolver() -> io::Result { let (config, opts) = system_conf::read_system_conf() .map_err(|e| io::Error::new(e.kind(), format!("error reading DNS system conf: {}", e)))?;