Minor optimization (rust-lang#544)

* Optimization: Rm `debug!` in `find_version`
   printing all version iterated obviously doesn't help much in debugging
   in the problem but rather just confusing.
   
   Also this makes it hard for the compiler to optimize the iterators.
* Use let-else in `ManifestVisitor`
* Optimize `BinFile::preview_{bin, link}` for zero-copy
   Return `impl Display` that lazily format instead of allocating a `String`
* Optimize `infer_bin_dir_template`: Generate dir lazily
* Optimize `find_version`: Lazily clone `version_req` only on err
* Refactor `find_version`: Use `bool::then_some`
* Add dep either v1.8.0 to binstalk
* Optimize `GhCrateMeta::find`: Avoid cloning and `Vec` creation
   by using `Either`
* Optimize `ops::install::install_from_package`: Make it a regular fn
   instead of async fn since it does not `.await` on any async fn.
* Optimize `QuickInstall`: Rm field `target`
   since `Arc<Data>` already contains that field.
* Optimize `Fetcher`s: Extract new struct `TargetData`
   so that `Data` can be shared by all fetchers, regardless of the target.
* Optimize `QuickInstall`: Rm unused field `data`
* Optimize `Resolution::print`: Replace branching with conditional move

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>

crates/binstalk/Cargo.toml

@@ -17,6 +17,7 @@ cargo_toml = "0.13.0"
 compact_str = { version = "0.6.0", features = ["serde"] }
 crates_io_api = { version = "0.8.1", default-features = false }
 detect-targets = { version = "0.1.2", path = "../detect-targets" }
+either = "1.8.0"
 futures-util = { version = "0.3.25", default-features = false, features = ["std"] }
 home = "0.5.4"
 itertools = "0.10.5"

crates/binstalk/src/bins.rs

@@ -1,7 +1,7 @@
 use std::{
     borrow::Cow,
-    fs,
-    path::{Component, Path, PathBuf},
+    fmt, fs,
+    path::{self, Component, Path, PathBuf},
 };
 
 use compact_str::CompactString;
@@ -31,29 +31,30 @@ fn is_valid_path(path: &Path) -> bool {
 /// Must be called after the archive is downloaded and extracted.
 /// This function might uses blocking I/O.
 pub fn infer_bin_dir_template(data: &Data) -> Cow<'static, str> {
-    let name = &data.name;
-    let target = &data.target;
-    let version = &data.version;
+    let name = data.name;
+    let target = data.target;
+    let version = data.version;
 
     // Make sure to update
     // fetchers::gh_crate_meta::hosting::{FULL_FILENAMES,
     // NOVERSION_FILENAMES} if you update this array.
-    let possible_dirs = [
-        format!("{name}-{target}-v{version}"),
-        format!("{name}-{target}-{version}"),
-        format!("{name}-{version}-{target}"),
-        format!("{name}-v{version}-{target}"),
-        format!("{name}-{target}"),
+    let gen_possible_dirs: [for<'r> fn(&'r str, &'r str, &'r str) -> String; 8] = [
+        |name, target, version| format!("{name}-{target}-v{version}"),
+        |name, target, version| format!("{name}-{target}-{version}"),
+        |name, target, version| format!("{name}-{version}-{target}"),
+        |name, target, version| format!("{name}-v{version}-{target}"),
+        |name, target, _version| format!("{name}-{target}"),
         // Ignore the following when updating hosting::{FULL_FILENAMES, NOVERSION_FILENAMES}
-        format!("{name}-{version}"),
-        format!("{name}-v{version}"),
-        name.to_string(),
+        |name, _target, version| format!("{name}-{version}"),
+        |name, _target, version| format!("{name}-v{version}"),
+        |name, _target, _version| name.to_string(),
     ];
 
     let default_bin_dir_template = Cow::Borrowed("{ bin }{ binary-ext }");
 
-    possible_dirs
+    gen_possible_dirs
         .into_iter()
+        .map(|gen_possible_dir| gen_possible_dir(name, target, version))
         .find(|dirname| data.bin_path.join(dirname).is_dir())
         .map(|mut dir| {
             dir.reserve_exact(1 + default_bin_dir_template.len());
@@ -138,26 +139,20 @@ impl BinFile {
         })
     }
 
-    pub fn preview_bin(&self) -> String {
-        format!(
-            "{} ({} -> {})",
-            self.base_name,
-            self.source.file_name().unwrap().to_string_lossy(),
-            self.dest.display()
-        )
+    pub fn preview_bin(&self) -> impl fmt::Display + '_ {
+        LazyFormat {
+            base_name: &self.base_name,
+            source: self.source.file_name().unwrap().to_string_lossy(),
+            dest: self.dest.display(),
+        }
     }
 
-    pub fn preview_link(&self) -> String {
-        if let Some(link) = &self.link {
-            format!(
-                "{} ({} -> {})",
-                self.base_name,
-                link.display(),
-                self.link_dest().display()
-            )
-        } else {
-            String::new()
-        }
+    pub fn preview_link(&self) -> impl fmt::Display + '_ {
+        OptionalLazyFormat(self.link.as_ref().map(|link| LazyFormat {
+            base_name: &self.base_name,
+            source: link.display(),
+            dest: self.link_dest().display(),
+        }))
     }
 
     /// Return `Ok` if the source exists, otherwise `Err`.
@@ -253,3 +248,27 @@ impl<'c> Context<'c> {
         Ok(tt.render("path", self)?)
     }
 }
+
+struct LazyFormat<'a, S: fmt::Display> {
+    base_name: &'a str,
+    source: S,
+    dest: path::Display<'a>,
+}
+
+impl<S: fmt::Display> fmt::Display for LazyFormat<'_, S> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{} ({} -> {})", self.base_name, self.source, self.dest)
+    }
+}
+
+struct OptionalLazyFormat<'a, S: fmt::Display>(Option<LazyFormat<'a, S>>);
+
+impl<S: fmt::Display> fmt::Display for OptionalLazyFormat<'_, S> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if let Some(lazy_format) = self.0.as_ref() {
+            fmt::Display::fmt(lazy_format, f)
+        } else {
+            Ok(())
+        }
+    }
+}

crates/binstalk/src/drivers/crates_io/visitor.rs

@@ -43,9 +43,8 @@ impl TarEntriesVisitor for ManifestVisitor {
             let path = entry.path()?;
             let path = path.normalize();
 
-            let path = if let Ok(path) = path.strip_prefix(&self.manifest_dir_path) {
-                path
-            } else {
+            let Ok(path) = path.strip_prefix(&self.manifest_dir_path)
+                else {
                 // The path is outside of the curr dir (manifest dir),
                 // ignore it.
                 continue;

crates/binstalk/src/drivers/version.rs

@@ -1,5 +1,4 @@
 use semver::VersionReq;
-use tracing::debug;
 
 use crate::errors::BinstallError;
 
@@ -37,16 +36,11 @@ pub(super) fn find_version<Item: Version, VersionIter: Iterator<Item = Item>>(
             let ver = item.get_version()?;
 
             // Filter by version match
-            if version_req.matches(&ver) {
-                debug!("Version: {:?}", ver);
-                Some((item, ver))
-            } else {
-                None
-            }
+            version_req.matches(&ver).then_some((item, ver))
         })
         // Return highest version
         .max_by(|(_item_x, ver_x), (_item_y, ver_y)| ver_x.cmp(ver_y))
-        .ok_or(BinstallError::VersionMismatch {
+        .ok_or_else(|| BinstallError::VersionMismatch {
             req: version_req.clone(),
         })
 }

crates/binstalk/src/fetchers.rs

@@ -17,7 +17,7 @@ pub(crate) mod quickinstall;
 pub trait Fetcher: Send + Sync {
     /// Create a new fetcher from some data
     #[allow(clippy::new_ret_no_self)]
-    fn new(client: &Client, data: &Arc<Data>) -> Arc<dyn Fetcher>
+    fn new(client: Client, data: Arc<Data>, target_data: Arc<TargetData>) -> Arc<dyn Fetcher>
     where
         Self: Sized;
 
@@ -61,8 +61,13 @@ pub trait Fetcher: Send + Sync {
 #[derive(Clone, Debug)]
 pub struct Data {
     pub name: CompactString,
-    pub target: String,
     pub version: CompactString,
     pub repo: Option<String>,
+}
+
+/// Target specific data required to fetch a package
+#[derive(Clone, Debug)]
+pub struct TargetData {
+    pub target: String,
     pub meta: PkgMeta,
 }