Optimize use of tinytemplate (#647)

* Optimize `GhCrateMeta::find`: Cache compiled template `pkg_url`
* Optimize `collect_bin_files`: Cache compiled template `bin_dir`
* Refactor: Improve readability of `gh_crate_meta`
* Optimize `GhCrateMeta::launch_baseline_find_tasks`: Dedup urls before checking
* Optimize `BinFile::new`: Avoid one heap alloc when creating `dest`
* Improve `warn!` msg in `GhCrateMeta::launch_baseline_find_tasks`

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

crates/binstalk/src/bins.rs

@@ -74,10 +74,11 @@ pub struct BinFile {
 }
 
 impl BinFile {
+    /// * `tt` - must have a template with name "bin_dir"
     pub fn new(
         data: &Data<'_>,
         base_name: &str,
-        bin_dir: &str,
+        tt: &TinyTemplate,
         no_symlinks: bool,
     ) -> Result<Self, BinstallError> {
         let binary_ext = if data.target.contains("windows") {
@@ -101,7 +102,7 @@ impl BinFile {
         } else {
             // Generate install paths
             // Source path is the download dir + the generated binary path
-            let path = ctx.render(bin_dir)?;
+            let path = ctx.render_with_compiled_tt(tt)?;
 
             let path_normalized = Path::new(&path).normalize();
 
@@ -119,13 +120,21 @@ impl BinFile {
         };
 
         // Destination at install dir + base-name{.extension}
-        let dest = data.install_path.join(ctx.render("{ bin }{ binary-ext }")?);
+        let mut dest = data.install_path.join(ctx.bin);
+        if !binary_ext.is_empty() {
+            let binary_ext = binary_ext.strip_prefix('.').unwrap();
+
+            // PathBuf::set_extension returns false if Path::file_name
+            // is None, but we know that the file name must be Some,
+            // thus we assert! the return value here.
+            assert!(dest.set_extension(binary_ext));
+        }
 
         let (dest, link) = if no_symlinks {
             (dest, None)
         } else {
             // Destination path is the install dir + base-name-version{.extension}
-            let dest_file_path_with_ver = ctx.render("{ bin }-v{ version }{ binary-ext }")?;
+            let dest_file_path_with_ver = format!("{}-v{}{}", ctx.bin, ctx.version, ctx.binary_ext);
             let dest_with_ver = data.install_path.join(dest_file_path_with_ver);
 
             (dest_with_ver, Some(dest))
@@ -235,10 +244,9 @@ struct Context<'c> {
 }
 
 impl<'c> Context<'c> {
-    fn render(&self, template: &str) -> Result<String, BinstallError> {
-        let mut tt = TinyTemplate::new();
-        tt.add_template("path", template)?;
-        Ok(tt.render("path", self)?)
+    /// * `tt` - must have a template with name "bin_dir"
+    fn render_with_compiled_tt(&self, tt: &TinyTemplate) -> Result<String, BinstallError> {
+        Ok(tt.render("bin_dir", self)?)
     }
 }
 

crates/binstalk/src/fetchers/gh_crate_meta.rs

@@ -3,6 +3,7 @@ use std::{borrow::Cow, future::Future, iter, path::Path, sync::Arc};
 use compact_str::{CompactString, ToCompactString};
 use either::Either;
 use futures_util::stream::{FuturesUnordered, StreamExt};
+use itertools::Itertools;
 use once_cell::sync::OnceCell;
 use serde::Serialize;
 use strum::IntoEnumIterator;
@@ -35,23 +36,30 @@ pub struct GhCrateMeta {
 type FindTaskRes = Result<Option<(Url, PkgFmt)>, BinstallError>;
 
 impl GhCrateMeta {
+    /// * `tt` - must have added a template named "pkg_url".
     fn launch_baseline_find_tasks<'a>(
         &'a self,
         pkg_fmt: PkgFmt,
+        tt: &'a TinyTemplate,
         pkg_url: &'a str,
         repo: Option<&'a str>,
     ) -> impl Iterator<Item = impl Future<Output = FindTaskRes> + 'static> + 'a {
         // build up list of potential URLs
-        let urls = pkg_fmt.extensions().iter().filter_map(move |ext| {
-            let ctx = Context::from_data_with_repo(&self.data, &self.target_data.target, ext, repo);
-            match ctx.render_url(pkg_url) {
-                Ok(url) => Some(url),
-                Err(err) => {
-                    warn!("Failed to render url for {ctx:#?}: {err:#?}");
-                    None
+        let urls = pkg_fmt
+            .extensions()
+            .iter()
+            .filter_map(move |ext| {
+                let ctx =
+                    Context::from_data_with_repo(&self.data, &self.target_data.target, ext, repo);
+                match ctx.render_url_with_compiled_tt(tt, pkg_url) {
+                    Ok(url) => Some(url),
+                    Err(err) => {
+                        warn!("Failed to render url for {ctx:#?}: {err}");
+                        None
+                    }
                 }
-            }
-        });
+            })
+            .dedup();
 
         // go check all potential URLs at once
         urls.map(move |url| {
@@ -139,12 +147,16 @@ impl super::Fetcher for GhCrateMeta {
 
             // Iterate over pkg_urls first to avoid String::clone.
             for pkg_url in pkg_urls {
+                let mut tt = TinyTemplate::new();
+
+                tt.add_template("pkg_url", &pkg_url)?;
+
                 //             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));
+                    handles.extend(this.launch_baseline_find_tasks(pkg_fmt, &tt, &pkg_url, repo));
                 }
             }
 
@@ -266,12 +278,22 @@ impl<'c> Context<'c> {
         Self::from_data_with_repo(data, target, archive_format, data.repo.as_deref())
     }
 
-    pub(self) fn render_url(&self, template: &str) -> Result<Url, BinstallError> {
-        debug!("Render {template:?} using context: {:?}", self);
+    /// * `tt` - must have added a template named "pkg_url".
+    pub(self) fn render_url_with_compiled_tt(
+        &self,
+        tt: &TinyTemplate,
+        template: &str,
+    ) -> Result<Url, BinstallError> {
+        debug!("Render {template} using context: {self:?}");
 
+        Ok(Url::parse(&tt.render("pkg_url", self)?)?)
+    }
+
+    #[cfg(test)]
+    pub(self) fn render_url(&self, template: &str) -> Result<Url, BinstallError> {
         let mut tt = TinyTemplate::new();
-        tt.add_template("path", template)?;
-        Ok(Url::parse(&tt.render("path", self)?)?)
+        tt.add_template("pkg_url", template)?;
+        self.render_url_with_compiled_tt(&tt, template)
     }
 }
 

crates/binstalk/src/ops/resolve.rs

@@ -12,6 +12,7 @@ use crates_io_api::AsyncClient as CratesIoApiClient;
 use itertools::Itertools;
 use maybe_owned::MaybeOwned;
 use semver::{Version, VersionReq};
+use tinytemplate::TinyTemplate;
 use tokio::task::block_in_place;
 use tracing::{debug, info, instrument, warn};
 
@@ -297,11 +298,15 @@ fn collect_bin_files(
         .map(Cow::Borrowed)
         .unwrap_or_else(|| bins::infer_bin_dir_template(&bin_data));
 
+    let mut tt = TinyTemplate::new();
+
+    tt.add_template("bin_dir", &bin_dir)?;
+
     // Create bin_files
     let bin_files = package_info
         .binaries
         .iter()
-        .map(|bin| bins::BinFile::new(&bin_data, bin.name.as_str(), &bin_dir, no_symlinks))
+        .map(|bin| bins::BinFile::new(&bin_data, bin.name.as_str(), &tt, no_symlinks))
         .collect::<Result<Vec<_>, BinstallError>>()?;
 
     let mut source_set = BTreeSet::new();