From b8d2184ee9d2ca4f87284fd1abfde9104a93a3de Mon Sep 17 00:00:00 2001
From: Jiahao XU <Jiahao_XU@outlook.com>
Date: Mon, 26 Sep 2022 16:05:14 +1000
Subject: [PATCH] Optimize applying `PkgOverride`s into `PkgMeta` (#429)

* Refactor: Rm `PkgMeta::clone_without_overrides`
* Impl new fn `PkgMeta::merge_overrides` to avoid unnecessary clone

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
---
 .../src/manifests/cargo_toml_binstall.rs      | 40 ++++++++++++++-----
 crates/binstalk/src/ops/resolve.rs            | 13 +++---
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/crates/binstalk/src/manifests/cargo_toml_binstall.rs b/crates/binstalk/src/manifests/cargo_toml_binstall.rs
index a8b023b83..9e785f201 100644
--- a/crates/binstalk/src/manifests/cargo_toml_binstall.rs
+++ b/crates/binstalk/src/manifests/cargo_toml_binstall.rs
@@ -43,16 +43,6 @@ pub struct PkgMeta {
 }
 
 impl PkgMeta {
-    pub fn clone_without_overrides(&self) -> Self {
-        Self {
-            pkg_url: self.pkg_url.clone(),
-            pkg_fmt: self.pkg_fmt,
-            bin_dir: self.bin_dir.clone(),
-            pub_key: self.pub_key.clone(),
-            overrides: BTreeMap::new(),
-        }
-    }
-
     /// Merge configuration overrides into object
     pub fn merge(&mut self, pkg_override: &PkgOverride) {
         if let Some(o) = &pkg_override.pkg_url {
@@ -65,6 +55,36 @@ impl PkgMeta {
             self.bin_dir = Some(o.clone());
         }
     }
+
+    /// Merge configuration overrides into object
+    ///
+    ///  * `pkg_overrides` - ordered in preference
+    pub fn merge_overrides<'a, It>(&self, pkg_overrides: It) -> Self
+    where
+        It: IntoIterator<Item = &'a PkgOverride> + Clone,
+    {
+        Self {
+            pkg_url: pkg_overrides
+                .clone()
+                .into_iter()
+                .find_map(|pkg_override| pkg_override.pkg_url.clone())
+                .or_else(|| self.pkg_url.clone()),
+
+            pkg_fmt: pkg_overrides
+                .clone()
+                .into_iter()
+                .find_map(|pkg_override| pkg_override.pkg_fmt)
+                .or(self.pkg_fmt),
+
+            bin_dir: pkg_overrides
+                .into_iter()
+                .find_map(|pkg_override| pkg_override.bin_dir.clone())
+                .or_else(|| self.bin_dir.clone()),
+
+            pub_key: self.pub_key.clone(),
+            overrides: Default::default(),
+        }
+    }
 }
 
 /// Target specific overrides for binary installation
diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs
index 8310072ec..d5a16b992 100644
--- a/crates/binstalk/src/ops/resolve.rs
+++ b/crates/binstalk/src/ops/resolve.rs
@@ -1,6 +1,7 @@
 use std::{
     borrow::Cow,
     collections::BTreeSet,
+    iter, mem,
     path::{Path, PathBuf},
     sync::Arc,
 };
@@ -169,7 +170,7 @@ async fn resolve_inner(
         }
     }
 
-    let (meta, mut binaries) = (
+    let (mut meta, mut binaries) = (
         package
             .metadata
             .as_ref()
@@ -189,19 +190,17 @@ async fn resolve_inner(
 
     let mut handles: Vec<(Arc<dyn Fetcher>, _)> = Vec::with_capacity(desired_targets.len() * 2);
 
+    let overrides = mem::take(&mut meta.overrides);
+
     handles.extend(
         desired_targets
             .iter()
             .map(|target| {
                 debug!("Building metadata for target: {target}");
-                let mut target_meta = meta.clone_without_overrides();
 
-                // Merge any overrides
-                if let Some(o) = meta.overrides.get(target) {
-                    target_meta.merge(o);
-                }
+                let target_meta = meta
+                    .merge_overrides(iter::once(&opts.cli_overrides).chain(overrides.get(target)));
 
-                target_meta.merge(&opts.cli_overrides);
                 debug!("Found metadata: {target_meta:?}");
 
                 Arc::new(Data {