From 31b7439a69a2ff550e060e2e2c1a5ce7f57d516e Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 15:16:10 +1000 Subject: [PATCH 1/8] Mod trait `Fetcher::new` to return `Arc` Signed-off-by: Jiahao XU --- src/fetchers.rs | 7 ++++--- src/fetchers/gh_crate_meta.rs | 5 +++-- src/fetchers/quickinstall.rs | 5 +++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/fetchers.rs b/src/fetchers.rs index b7b25795..77daaf68 100644 --- a/src/fetchers.rs +++ b/src/fetchers.rs @@ -1,4 +1,5 @@ use std::path::Path; +use std::sync::Arc; pub use gh_crate_meta::*; pub use log::debug; @@ -12,7 +13,7 @@ mod quickinstall; #[async_trait::async_trait] pub trait Fetcher { /// Create a new fetcher from some data - async fn new(data: &Data) -> Box + async fn new(data: &Data) -> Arc where Self: Sized; @@ -44,11 +45,11 @@ pub struct Data { #[derive(Default)] pub struct MultiFetcher { - fetchers: Vec>, + fetchers: Vec>, } impl MultiFetcher { - pub fn add(&mut self, fetcher: Box) { + pub fn add(&mut self, fetcher: Arc) { self.fetchers.push(fetcher); } diff --git a/src/fetchers/gh_crate_meta.rs b/src/fetchers/gh_crate_meta.rs index f048bb87..12bb22ab 100644 --- a/src/fetchers/gh_crate_meta.rs +++ b/src/fetchers/gh_crate_meta.rs @@ -1,4 +1,5 @@ use std::path::Path; +use std::sync::Arc; use log::{debug, info, warn}; use reqwest::Method; @@ -22,8 +23,8 @@ impl GhCrateMeta { #[async_trait::async_trait] impl super::Fetcher for GhCrateMeta { - async fn new(data: &Data) -> Box { - Box::new(Self { data: data.clone() }) + async fn new(data: &Data) -> Arc { + Arc::new(Self { data: data.clone() }) } async fn check(&self) -> Result { diff --git a/src/fetchers/quickinstall.rs b/src/fetchers/quickinstall.rs index 61871e5c..96487851 100644 --- a/src/fetchers/quickinstall.rs +++ b/src/fetchers/quickinstall.rs @@ -1,4 +1,5 @@ use std::path::Path; +use std::sync::Arc; use log::info; use reqwest::Method; @@ -17,11 +18,11 @@ pub struct QuickInstall { #[async_trait::async_trait] impl super::Fetcher for QuickInstall { - async fn new(data: &Data) -> Box { + async fn new(data: &Data) -> Arc { let crate_name = &data.name; let version = &data.version; let target = &data.target; - Box::new(Self { + Arc::new(Self { package: format!("{crate_name}-{version}-{target}"), }) } From d373ad5145ed72bac402b9f71987856739414c9a Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 15:28:48 +1000 Subject: [PATCH 2/8] Require `Send` and `Sync` for trait `Fetcher` Signed-off-by: Jiahao XU --- src/fetchers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fetchers.rs b/src/fetchers.rs index 77daaf68..08b43d1e 100644 --- a/src/fetchers.rs +++ b/src/fetchers.rs @@ -11,7 +11,7 @@ mod gh_crate_meta; mod quickinstall; #[async_trait::async_trait] -pub trait Fetcher { +pub trait Fetcher: Send + Sync { /// Create a new fetcher from some data async fn new(data: &Data) -> Arc where From c393270899f90eb2862d2293fc577f09bc0feb7c Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 15:29:09 +1000 Subject: [PATCH 3/8] Run fetchers in parallel in `MultiFetcher.first_available` Signed-off-by: Jiahao XU --- src/fetchers.rs | 56 ++++++++++++++++++++++++++++++++++++++----------- src/main.rs | 2 +- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/fetchers.rs b/src/fetchers.rs index 08b43d1e..a7384985 100644 --- a/src/fetchers.rs +++ b/src/fetchers.rs @@ -4,6 +4,7 @@ use std::sync::Arc; pub use gh_crate_meta::*; pub use log::debug; pub use quickinstall::*; +use tokio::task::JoinHandle; use crate::{BinstallError, PkgFmt, PkgMeta}; @@ -53,22 +54,53 @@ impl MultiFetcher { self.fetchers.push(fetcher); } - pub async fn first_available(&self) -> Option<&dyn Fetcher> { - for fetcher in &self.fetchers { - let available = fetcher.check().await.unwrap_or_else(|err| { - debug!( - "Error while checking fetcher {}: {}", - fetcher.source_name(), - err - ); - false - }); + pub async fn first_available(&self) -> Option> { + let handles: Vec<_> = self + .fetchers + .iter() + .cloned() + .map(|fetcher| { + let fetcher_cloned = fetcher.clone(); - if available { - return Some(&**fetcher); + ( + AutoAbortJoinHandle(tokio::spawn(async move { + fetcher.check().await.unwrap_or_else(|err| { + debug!( + "Error while checking fetcher {}: {}", + fetcher.source_name(), + err + ); + false + }) + })), + fetcher_cloned, + ) + }) + .collect(); + + for (mut handle, fetcher) in handles { + match (&mut handle.0).await { + Ok(true) => return Some(fetcher), + Err(join_err) => { + debug!( + "Error while checking fetcher {}: {}", + fetcher.source_name(), + join_err + ); + } + _ => (), } } None } } + +#[derive(Debug)] +struct AutoAbortJoinHandle(JoinHandle); + +impl Drop for AutoAbortJoinHandle { + fn drop(&mut self) { + self.0.abort(); + } +} diff --git a/src/main.rs b/src/main.rs index a9479a2d..5c913dc5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -234,7 +234,7 @@ async fn entry() -> Result<()> { Some(fetcher) => { install_from_package( binaries, - fetcher, + &*fetcher, install_path, meta, opts, From 3d6679fd7d8fe801ec23e47400f51db648139b64 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 15:35:21 +1000 Subject: [PATCH 4/8] Refactor `MultiFetcher.first_available` Signed-off-by: Jiahao XU --- src/fetchers.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/fetchers.rs b/src/fetchers.rs index a7384985..b532d1e3 100644 --- a/src/fetchers.rs +++ b/src/fetchers.rs @@ -63,16 +63,7 @@ impl MultiFetcher { let fetcher_cloned = fetcher.clone(); ( - AutoAbortJoinHandle(tokio::spawn(async move { - fetcher.check().await.unwrap_or_else(|err| { - debug!( - "Error while checking fetcher {}: {}", - fetcher.source_name(), - err - ); - false - }) - })), + AutoAbortJoinHandle(tokio::spawn(async move { fetcher.check().await })), fetcher_cloned, ) }) @@ -80,7 +71,15 @@ impl MultiFetcher { for (mut handle, fetcher) in handles { match (&mut handle.0).await { - Ok(true) => return Some(fetcher), + Ok(Ok(true)) => return Some(fetcher), + Ok(Ok(false)) => (), + Ok(Err(err)) => { + debug!( + "Error while checking fetcher {}: {}", + fetcher.source_name(), + err + ); + } Err(join_err) => { debug!( "Error while checking fetcher {}: {}", @@ -88,7 +87,6 @@ impl MultiFetcher { join_err ); } - _ => (), } } @@ -97,7 +95,7 @@ impl MultiFetcher { } #[derive(Debug)] -struct AutoAbortJoinHandle(JoinHandle); +struct AutoAbortJoinHandle(JoinHandle>); impl Drop for AutoAbortJoinHandle { fn drop(&mut self) { From b2a533dbdbf2c7aaf8f56b4c6ec05672d33f5530 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 15:40:55 +1000 Subject: [PATCH 5/8] Optimize `GhCrateMeta::check`: Avoid converting url to str Only to convert it back to `Url` in `helpers::remote_exists` Signed-off-by: Jiahao XU --- src/fetchers/gh_crate_meta.rs | 2 +- src/fetchers/quickinstall.rs | 2 +- src/helpers.rs | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/fetchers/gh_crate_meta.rs b/src/fetchers/gh_crate_meta.rs index f048bb87..d55109a6 100644 --- a/src/fetchers/gh_crate_meta.rs +++ b/src/fetchers/gh_crate_meta.rs @@ -36,7 +36,7 @@ impl super::Fetcher for GhCrateMeta { } info!("Checking for package at: '{url}'"); - remote_exists(url.as_str(), Method::HEAD).await + remote_exists(url, Method::HEAD).await } async fn fetch(&self, dst: &Path) -> Result<(), BinstallError> { diff --git a/src/fetchers/quickinstall.rs b/src/fetchers/quickinstall.rs index 61871e5c..4611e5f5 100644 --- a/src/fetchers/quickinstall.rs +++ b/src/fetchers/quickinstall.rs @@ -30,7 +30,7 @@ impl super::Fetcher for QuickInstall { let url = self.package_url(); self.report().await?; info!("Checking for package at: '{url}'"); - remote_exists(&url, Method::HEAD).await + remote_exists(Url::parse(&url)?, Method::HEAD).await } async fn fetch(&self, dst: &Path) -> Result<(), BinstallError> { diff --git a/src/helpers.rs b/src/helpers.rs index e0d85b99..9471eb4f 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -32,8 +32,7 @@ pub fn load_manifest_path>( Ok(manifest) } -pub async fn remote_exists(url: &str, method: Method) -> Result { - let url = Url::parse(url)?; +pub async fn remote_exists(url: Url, method: Method) -> Result { let req = reqwest::Client::new() .request(method.clone(), url.clone()) .send() From b6245bcf4be9e78fe04a784667faa679961fff6e Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 16:13:38 +1000 Subject: [PATCH 6/8] Spawn `entry()` in `main` to improve parallelism Using `rt.block_on`, the future returned by `entry` can only be run on the main thread. Buf if we use `tokio::spawn`, then it can be run on any thread. Signed-off-by: Jiahao XU --- src/main.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5c913dc5..87f63063 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,7 +11,7 @@ use miette::{miette, IntoDiagnostic, Result, WrapErr}; use simplelog::{ColorChoice, ConfigBuilder, TermLogger, TerminalMode}; use structopt::StructOpt; use tempfile::TempDir; -use tokio::{process::Command, runtime::Runtime}; +use tokio::{process::Command, runtime::Runtime, task::JoinError}; use cargo_binstall::{ bins, @@ -84,6 +84,7 @@ enum MainExit { Success(Duration), Error(BinstallError), Report(miette::Report), + JoinErr(JoinError), } impl Termination for MainExit { @@ -99,6 +100,11 @@ impl Termination for MainExit { eprintln!("{err:?}"); ExitCode::from(16) } + Self::JoinErr(err) => { + error!("Fatal error:"); + eprintln!("{err:?}"); + ExitCode::from(16) + } } } } @@ -107,19 +113,22 @@ fn main() -> MainExit { let start = Instant::now(); let rt = Runtime::new().unwrap(); - let result = rt.block_on(entry()); + let handle = rt.spawn(entry()); + let result = rt.block_on(handle); drop(rt); let done = start.elapsed(); debug!("run time: {done:?}"); result - .map(|_| MainExit::Success(done)) - .unwrap_or_else(|err| { - err.downcast::() - .map(MainExit::Error) - .unwrap_or_else(MainExit::Report) + .map(|res| { + res.map(|_| MainExit::Success(done)).unwrap_or_else(|err| { + err.downcast::() + .map(MainExit::Error) + .unwrap_or_else(MainExit::Report) + }) }) + .unwrap_or_else(MainExit::JoinErr) } async fn entry() -> Result<()> { From 903c9f559171ecd5fe07f36a59da325c2a270066 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 16:19:07 +1000 Subject: [PATCH 7/8] Refactor: Use `Result::map_or_else` in `main` Signed-off-by: Jiahao XU --- src/main.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/main.rs b/src/main.rs index 87f63063..5d578545 100644 --- a/src/main.rs +++ b/src/main.rs @@ -120,15 +120,13 @@ fn main() -> MainExit { let done = start.elapsed(); debug!("run time: {done:?}"); - result - .map(|res| { - res.map(|_| MainExit::Success(done)).unwrap_or_else(|err| { - err.downcast::() - .map(MainExit::Error) - .unwrap_or_else(MainExit::Report) - }) + result.map_or_else(MainExit::JoinErr, |res| { + res.map(|_| MainExit::Success(done)).unwrap_or_else(|err| { + err.downcast::() + .map(MainExit::Error) + .unwrap_or_else(MainExit::Report) }) - .unwrap_or_else(MainExit::JoinErr) + }) } async fn entry() -> Result<()> { From 456e8964833f31bd62a15cd3e98afb6d5e8cc2b9 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 7 Jun 2022 16:22:01 +1000 Subject: [PATCH 8/8] Use code 17 for `MainExit::JoinErr` Signed-off-by: Jiahao XU --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 5d578545..0a6a10e3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -103,7 +103,7 @@ impl Termination for MainExit { Self::JoinErr(err) => { error!("Fatal error:"); eprintln!("{err:?}"); - ExitCode::from(16) + ExitCode::from(17) } } }