From eb95c5b799c2991a87cfc5bfcb620a3e6f1308dc Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 24 Jan 2023 23:58:07 +1100 Subject: [PATCH] Minor optimization for bin and binstalk (#646) * Refactor `logging`: Extract `report_err` * Optimize `get_default_pkg_url_template`: Return iter instead of `Vec` to avoid heap allocation. * Refactor `GhCrateMeta::find`: Rm `launch_baseline_find_tasks` * Optimize `GhCrateMeta::find`: Avoid cloning `Cow<'_, str>` * Improve `report_err` output: Print newline after msg Signed-off-by: Jiahao XU --- crates/bin/src/logging.rs | 18 +++++++--- crates/binstalk/src/fetchers/gh_crate_meta.rs | 36 ++++++++++--------- .../src/fetchers/gh_crate_meta/hosting.rs | 13 ++++--- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/crates/bin/src/logging.rs b/crates/bin/src/logging.rs index 377c7c4b..2db20e2f 100644 --- a/crates/bin/src/logging.rs +++ b/crates/bin/src/logging.rs @@ -1,4 +1,8 @@ -use std::{cmp::min, io, iter::repeat}; +use std::{ + cmp::min, + io::{self, Write}, + iter::repeat, +}; use log::{LevelFilter, Log, STATIC_MAX_LEVEL}; use once_cell::sync::Lazy; @@ -135,10 +139,14 @@ impl Log for Logger { struct ErrorFreeWriter; +fn report_err(err: io::Error) { + writeln!(io::stderr(), "Failed to write to stdout: {err}").ok(); +} + impl io::Write for &ErrorFreeWriter { fn write(&mut self, buf: &[u8]) -> io::Result { io::stdout().write(buf).or_else(|err| { - write!(io::stderr(), "Failed to write to stdout: {err}").ok(); + report_err(err); // Behave as if writing to /dev/null so that logging system // would keep working. Ok(buf.len()) @@ -147,7 +155,7 @@ impl io::Write for &ErrorFreeWriter { fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { io::stdout().write_all(buf).or_else(|err| { - write!(io::stderr(), "Failed to write to stdout: {err}").ok(); + report_err(err); // Behave as if writing to /dev/null so that logging system // would keep working. Ok(()) @@ -156,7 +164,7 @@ impl io::Write for &ErrorFreeWriter { fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { io::stdout().write_vectored(bufs).or_else(|err| { - write!(io::stderr(), "Failed to write to stdout: {err}").ok(); + report_err(err); // Behave as if writing to /dev/null so that logging system // would keep working. Ok(bufs.iter().map(|io_slice| io_slice.len()).sum()) @@ -165,7 +173,7 @@ impl io::Write for &ErrorFreeWriter { fn flush(&mut self) -> io::Result<()> { io::stdout().flush().or_else(|err| { - write!(io::stderr(), "Failed to write to stdout: {err}").ok(); + report_err(err); // Behave as if writing to /dev/null so that logging system // would keep working. Ok(()) diff --git a/crates/binstalk/src/fetchers/gh_crate_meta.rs b/crates/binstalk/src/fetchers/gh_crate_meta.rs index 790ef084..72c467ea 100644 --- a/crates/binstalk/src/fetchers/gh_crate_meta.rs +++ b/crates/binstalk/src/fetchers/gh_crate_meta.rs @@ -1,4 +1,4 @@ -use std::{future::Future, iter, ops::Deref, path::Path, sync::Arc}; +use std::{borrow::Cow, future::Future, iter, path::Path, sync::Arc}; use compact_str::{CompactString, ToCompactString}; use either::Either; @@ -92,12 +92,12 @@ impl super::Fetcher for GhCrateMeta { }; let pkg_urls = if let Some(pkg_url) = self.target_data.meta.pkg_url.as_deref() { - Either::Left(pkg_url) + Either::Left(iter::once(Cow::Borrowed(pkg_url))) } else if let Some(repo) = repo.as_ref() { if let Some(pkg_urls) = RepositoryHost::guess_git_hosting_services(repo)?.get_default_pkg_url_template() { - Either::Right(pkg_urls) + Either::Right(pkg_urls.map(Cow::Owned)) } else { warn!( concat!( @@ -129,22 +129,24 @@ impl super::Fetcher for GhCrateMeta { // launch_baseline_find_tasks which moves `this` let this = &self; - let launch_baseline_find_tasks = |pkg_fmt| { - match &pkg_urls { - Either::Left(pkg_url) => Either::Left(iter::once(*pkg_url)), - Either::Right(pkg_urls) => Either::Right(pkg_urls.iter().map(Deref::deref)), - } - .flat_map(move |pkg_url| this.launch_baseline_find_tasks(pkg_fmt, pkg_url, repo)) + let pkg_fmts = if let Some(pkg_fmt) = self.target_data.meta.pkg_fmt { + Either::Left(iter::once(pkg_fmt)) + } else { + Either::Right(PkgFmt::iter()) }; - let mut handles: FuturesUnordered<_> = - if let Some(pkg_fmt) = self.target_data.meta.pkg_fmt { - launch_baseline_find_tasks(pkg_fmt).collect() - } else { - PkgFmt::iter() - .flat_map(launch_baseline_find_tasks) - .collect() - }; + let mut handles = FuturesUnordered::new(); + + // Iterate over pkg_urls first to avoid String::clone. + for pkg_url in pkg_urls { + // Clone iter pkg_fmts to ensure all pkg_fmts is + // iterated over for each pkg_url, which is + // basically cartesian product. + // | + for pkg_fmt in pkg_fmts.clone() { + handles.extend(this.launch_baseline_find_tasks(pkg_fmt, &pkg_url, repo)); + } + } while let Some(res) = handles.next().await { if let Some((url, pkg_fmt)) = res? { diff --git a/crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs b/crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs index 7cb26b85..00a030fa 100644 --- a/crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs +++ b/crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs @@ -44,7 +44,9 @@ impl RepositoryHost { } } - pub fn get_default_pkg_url_template(self) -> Option> { + pub fn get_default_pkg_url_template( + self, + ) -> Option + Clone + 'static> { use RepositoryHost::*; match self { @@ -82,11 +84,14 @@ impl RepositoryHost { } } -fn apply_filenames_to_paths(paths: &[&str], filenames: &[&[&str]], suffix: &str) -> Vec { +fn apply_filenames_to_paths( + paths: &'static [&'static str], + filenames: &'static [&'static [&'static str]], + suffix: &'static str, +) -> impl Iterator + Clone + 'static { filenames .iter() .flat_map(|fs| fs.iter()) .cartesian_product(paths.iter()) - .map(|(filename, path)| format!("{path}/{filename}{suffix}")) - .collect() + .map(move |(filename, path)| format!("{path}/{filename}{suffix}")) }