From 66abf1b8c17a8c96f51f70c19cd0712744400d3c Mon Sep 17 00:00:00 2001
From: Jiahao XU <Jiahao_XU@outlook.com>
Date: Tue, 22 Nov 2022 17:35:16 +1100
Subject: [PATCH] More mionr optimizations (#553)

* Optimize `BinFile::preview_bin`: Use `Path::display` instead of `to_string_lossy` to avoid allocation.
* Refactor: Rm useless `AsRef::as_ref` call
* Optimize `GhCrateMeta::find`: Reduce fut size by converting `Url` to `String`
   `Url` is much larger than `String`.
* Refactor `PackageInfo::resolve`: Destruct `Meta::binstall`
* Optimize `resolve::load_manifest_path`: Avoid allocation

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
---
 crates/binstalk/src/bins.rs                   | 12 ++++++------
 crates/binstalk/src/drivers/crates_io.rs      | 15 ++++++++-------
 crates/binstalk/src/fetchers/gh_crate_meta.rs |  5 ++++-
 crates/binstalk/src/ops/resolve.rs            | 16 ++++++++++------
 4 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/crates/binstalk/src/bins.rs b/crates/binstalk/src/bins.rs
index 6ea20b9ce..8349337ea 100644
--- a/crates/binstalk/src/bins.rs
+++ b/crates/binstalk/src/bins.rs
@@ -142,7 +142,7 @@ impl BinFile {
     pub fn preview_bin(&self) -> impl fmt::Display + '_ {
         LazyFormat {
             base_name: &self.base_name,
-            source: self.source.file_name().unwrap().to_string_lossy(),
+            source: Path::new(self.source.file_name().unwrap()).display(),
             dest: self.dest.display(),
         }
     }
@@ -249,21 +249,21 @@ impl<'c> Context<'c> {
     }
 }
 
-struct LazyFormat<'a, S: fmt::Display> {
+struct LazyFormat<'a> {
     base_name: &'a str,
-    source: S,
+    source: path::Display<'a>,
     dest: path::Display<'a>,
 }
 
-impl<S: fmt::Display> fmt::Display for LazyFormat<'_, S> {
+impl fmt::Display for LazyFormat<'_> {
     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>>);
+struct OptionalLazyFormat<'a>(Option<LazyFormat<'a>>);
 
-impl<S: fmt::Display> fmt::Display for OptionalLazyFormat<'_, S> {
+impl fmt::Display for OptionalLazyFormat<'_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if let Some(lazy_format) = self.0.as_ref() {
             fmt::Display::fmt(lazy_format, f)
diff --git a/crates/binstalk/src/drivers/crates_io.rs b/crates/binstalk/src/drivers/crates_io.rs
index 8202d9065..367871484 100644
--- a/crates/binstalk/src/drivers/crates_io.rs
+++ b/crates/binstalk/src/drivers/crates_io.rs
@@ -33,13 +33,14 @@ pub async fn fetch_crate_cratesio(
     debug!("Looking up crate information");
 
     // Fetch online crate information
-    let base_info = crates_io_api_client
-        .get_crate(name.as_ref())
-        .await
-        .map_err(|err| BinstallError::CratesIoApi {
-            crate_name: name.into(),
-            err: Box::new(err),
-        })?;
+    let base_info =
+        crates_io_api_client
+            .get_crate(name)
+            .await
+            .map_err(|err| BinstallError::CratesIoApi {
+                crate_name: name.into(),
+                err: Box::new(err),
+            })?;
 
     // Locate matching version
     let version_iter = base_info.versions.iter().filter(|v| !v.yanked);
diff --git a/crates/binstalk/src/fetchers/gh_crate_meta.rs b/crates/binstalk/src/fetchers/gh_crate_meta.rs
index 7091183f3..257e2630b 100644
--- a/crates/binstalk/src/fetchers/gh_crate_meta.rs
+++ b/crates/binstalk/src/fetchers/gh_crate_meta.rs
@@ -124,7 +124,10 @@ impl super::Fetcher for GhCrateMeta {
             return Ok(false);
         };
 
-        let repo = repo.as_ref().map(|u| u.as_str().trim_end_matches('/'));
+        // Convert Option<Url> to Option<String> to reduce size of future.
+        let repo = repo.map(String::from);
+        let repo = repo.as_deref().map(|u| u.trim_end_matches('/'));
+
         let launch_baseline_find_tasks = |pkg_fmt| {
             match &pkg_urls {
                 Either::Left(pkg_url) => Either::Left(iter::once(*pkg_url)),
diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs
index 7cc8e5bb6..09784b7b7 100644
--- a/crates/binstalk/src/ops/resolve.rs
+++ b/crates/binstalk/src/ops/resolve.rs
@@ -239,7 +239,10 @@ async fn resolve_inner(
     }
 }
 
-///  * `fetcher` - `fetcher.find()` must return `Ok(true)`.
+///  * `fetcher` - `fetcher.find()` must have returned `Ok(true)`.
+///
+/// Can return empty Vec if all `BinFile` is optional and does not exist
+/// in the archive downloaded.
 async fn download_extract_and_verify(
     fetcher: &dyn Fetcher,
     bin_path: &Path,
@@ -278,7 +281,7 @@ async fn download_extract_and_verify(
         }
     }
 
-    // Verify that all the bin_files exist
+    // Verify that all non-optional bin_files exist
     block_in_place(|| {
         let bin_files = collect_bin_files(
             fetcher,
@@ -430,7 +433,7 @@ impl PackageInfo {
             package
                 .metadata
                 .take()
-                .and_then(|mut m| m.binstall.take())
+                .and_then(|m| m.binstall)
                 .unwrap_or_default(),
             manifest
                 .bin
@@ -465,12 +468,13 @@ impl PackageInfo {
 pub fn load_manifest_path<P: AsRef<Path>>(
     manifest_path: P,
 ) -> Result<Manifest<Meta>, BinstallError> {
+    let manifest_path = manifest_path.as_ref();
+
     block_in_place(|| {
-        let manifest_path = manifest_path.as_ref();
         let manifest_path = if manifest_path.is_dir() {
-            manifest_path.join("Cargo.toml")
+            Cow::Owned(manifest_path.join("Cargo.toml"))
         } else if manifest_path.is_file() {
-            manifest_path.into()
+            Cow::Borrowed(manifest_path)
         } else {
             return Err(BinstallError::CargoManifestPath);
         };