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 <Jiahao_XU@outlook.com>

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<usize> {
         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<usize> {
         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(())

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| {
+            let pkg_fmts = if let Some(pkg_fmt) = self.target_data.meta.pkg_fmt {
-                match &pkg_urls {
+                Either::Left(iter::once(pkg_fmt))
-                    Either::Left(pkg_url) => Either::Left(iter::once(*pkg_url)),
+            } else {
-                    Either::Right(pkg_urls) => Either::Right(pkg_urls.iter().map(Deref::deref)),
+                Either::Right(PkgFmt::iter())
-                }
-                .flat_map(move |pkg_url| this.launch_baseline_find_tasks(pkg_fmt, pkg_url, repo))
             };
 
-            let mut handles: FuturesUnordered<_> =
+            let mut handles = FuturesUnordered::new();
-                if let Some(pkg_fmt) = self.target_data.meta.pkg_fmt {
+
-                    launch_baseline_find_tasks(pkg_fmt).collect()
+            // Iterate over pkg_urls first to avoid String::clone.
-                } else {
+            for pkg_url in pkg_urls {
-                    PkgFmt::iter()
+                //             Clone iter pkg_fmts to ensure all pkg_fmts is
-                        .flat_map(launch_baseline_find_tasks)
+                //             iterated over for each pkg_url, which is
-                        .collect()
+                //             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? {

crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs

@@ -44,7 +44,9 @@ impl RepositoryHost {
         }
     }
 
-    pub fn get_default_pkg_url_template(self) -> Option<Vec<String>> {
+    pub fn get_default_pkg_url_template(
+        self,
+    ) -> Option<impl Iterator<Item = String> + Clone + 'static> {
         use RepositoryHost::*;
 
         match self {
@@ -82,11 +84,14 @@ impl RepositoryHost {
     }
 }
 
-fn apply_filenames_to_paths(paths: &[&str], filenames: &[&[&str]], suffix: &str) -> Vec<String> {
+fn apply_filenames_to_paths(
+    paths: &'static [&'static str],
+    filenames: &'static [&'static [&'static str]],
+    suffix: &'static str,
+) -> impl Iterator<Item = String> + Clone + 'static {
     filenames
         .iter()
         .flat_map(|fs| fs.iter())
         .cartesian_product(paths.iter())
-        .map(|(filename, path)| format!("{path}/{filename}{suffix}"))
+        .map(move |(filename, path)| format!("{path}/{filename}{suffix}"))
-        .collect()
 }