From ccabb118907ee0fb65df228190857c89b90bd9f4 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 30 Apr 2024 17:56:02 +0900 Subject: [PATCH] templater: wrap RefName with Rc to copy OnceCell around Because template is declarative language, and is evaluated as a tree, there will be multiple copies of the same RefName object. This patch allows us to cache ahead/behind counts which will be lazily calculated. --- cli/src/commit_templater.rs | 61 ++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index e74b83a1d..cc9ee5bf3 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -217,19 +217,19 @@ impl<'repo> CommitTemplateLanguage<'repo> { } pub fn wrap_ref_name( - property: impl TemplateProperty + 'repo, + property: impl TemplateProperty> + 'repo, ) -> CommitTemplatePropertyKind<'repo> { CommitTemplatePropertyKind::RefName(Box::new(property)) } pub fn wrap_ref_name_opt( - property: impl TemplateProperty> + 'repo, + property: impl TemplateProperty>> + 'repo, ) -> CommitTemplatePropertyKind<'repo> { CommitTemplatePropertyKind::RefNameOpt(Box::new(property)) } pub fn wrap_ref_name_list( - property: impl TemplateProperty> + 'repo, + property: impl TemplateProperty>> + 'repo, ) -> CommitTemplatePropertyKind<'repo> { CommitTemplatePropertyKind::RefNameList(Box::new(property)) } @@ -252,9 +252,9 @@ pub enum CommitTemplatePropertyKind<'repo> { Commit(Box + 'repo>), CommitOpt(Box> + 'repo>), CommitList(Box> + 'repo>), - RefName(Box + 'repo>), - RefNameOpt(Box> + 'repo>), - RefNameList(Box> + 'repo>), + RefName(Box> + 'repo>), + RefNameOpt(Box>> + 'repo>), + RefNameList(Box>> + 'repo>), CommitOrChangeId(Box + 'repo>), ShortestIdPrefix(Box + 'repo>), } @@ -340,7 +340,7 @@ pub type CommitTemplateBuildMethodFnMap<'repo, T> = pub struct CommitTemplateBuildFnTable<'repo> { pub core: CoreTemplateBuildFnTable<'repo, CommitTemplateLanguage<'repo>>, pub commit_methods: CommitTemplateBuildMethodFnMap<'repo, Commit>, - pub ref_name_methods: CommitTemplateBuildMethodFnMap<'repo, RefName>, + pub ref_name_methods: CommitTemplateBuildMethodFnMap<'repo, Rc>, pub commit_or_change_id_methods: CommitTemplateBuildMethodFnMap<'repo, CommitOrChangeId>, pub shortest_id_prefix_methods: CommitTemplateBuildMethodFnMap<'repo, ShortestIdPrefix>, } @@ -721,7 +721,7 @@ fn evaluate_user_revset<'repo>( } /// Branch or tag name with metadata. -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct RefName { /// Local name. name: String, @@ -735,26 +735,29 @@ pub struct RefName { } impl RefName { + // RefName is wrapped by Rc to make it cheaply cloned and share + // lazy-evaluation results across clones. + /// Creates local ref representation which might track some of the /// `remote_refs`. pub fn local<'a>( name: impl Into, target: RefTarget, remote_refs: impl IntoIterator, - ) -> Self { + ) -> Rc { let synced = remote_refs .into_iter() .all(|remote_ref| !remote_ref.is_tracking() || remote_ref.target == target); - RefName { + Rc::new(RefName { name: name.into(), remote: None, target, synced, - } + }) } /// Creates local ref representation which doesn't track any remote refs. - pub fn local_only(name: impl Into, target: RefTarget) -> Self { + pub fn local_only(name: impl Into, target: RefTarget) -> Rc { Self::local(name, target, []) } @@ -765,14 +768,14 @@ impl RefName { remote_name: impl Into, remote_ref: RemoteRef, local_target: &RefTarget, - ) -> Self { + ) -> Rc { let synced = remote_ref.is_tracking() && remote_ref.target == *local_target; - RefName { + Rc::new(RefName { name: name.into(), remote: Some(remote_name.into()), target: remote_ref.target, synced, - } + }) } /// Creates remote ref representation which isn't tracked by a local ref. @@ -780,13 +783,13 @@ impl RefName { name: impl Into, remote_name: impl Into, target: RefTarget, - ) -> Self { - RefName { + ) -> Rc { + Rc::new(RefName { name: name.into(), remote: Some(remote_name.into()), target, synced: false, // has no local counterpart - } + }) } fn is_local(&self) -> bool { @@ -807,7 +810,8 @@ impl RefName { } } -impl Template for RefName { +// If wrapping with Rc becomes common, add generic impl for Rc. +impl Template for Rc { fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> { write!(formatter.labeled("name"), "{}", self.name)?; if let Some(remote) = &self.remote { @@ -825,27 +829,28 @@ impl Template for RefName { } } -impl Template for Vec { +impl Template for Vec> { fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> { templater::format_joined(formatter, self, " ") } } -fn builtin_ref_name_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, RefName> { +fn builtin_ref_name_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Rc> { type L<'repo> = CommitTemplateLanguage<'repo>; // Not using maplit::hashmap!{} or custom declarative macro here because // code completion inside macro is quite restricted. - let mut map = CommitTemplateBuildMethodFnMap::::new(); + let mut map = CommitTemplateBuildMethodFnMap::>::new(); map.insert("name", |_language, _build_ctx, self_property, function| { template_parser::expect_no_arguments(function)?; - let out_property = self_property.map(|ref_name| ref_name.name); + let out_property = self_property.map(|ref_name| ref_name.name.clone()); Ok(L::wrap_string(out_property)) }); map.insert( "remote", |_language, _build_ctx, self_property, function| { template_parser::expect_no_arguments(function)?; - let out_property = self_property.map(|ref_name| ref_name.remote.unwrap_or_default()); + let out_property = + self_property.map(|ref_name| ref_name.remote.clone().unwrap_or_default()); Ok(L::wrap_string(out_property)) }, ); @@ -907,11 +912,11 @@ fn builtin_ref_name_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Re /// Cache for reverse lookup refs. #[derive(Clone, Debug, Default)] pub struct RefNamesIndex { - index: HashMap>, + index: HashMap>>, } impl RefNamesIndex { - fn insert<'a>(&mut self, ids: impl IntoIterator, name: RefName) { + fn insert<'a>(&mut self, ids: impl IntoIterator, name: Rc) { for id in ids { let ref_names = self.index.entry(id.clone()).or_default(); ref_names.push(name.clone()); @@ -920,7 +925,7 @@ impl RefNamesIndex { #[allow(unknown_lints)] // XXX FIXME (aseipp): nightly bogons; re-test this occasionally #[allow(clippy::manual_unwrap_or_default)] - pub fn get(&self, id: &CommitId) -> &[RefName] { + pub fn get(&self, id: &CommitId) -> &[Rc] { if let Some(names) = self.index.get(id) { names } else { @@ -962,7 +967,7 @@ fn build_ref_names_index<'a>( index } -fn extract_git_head(repo: &dyn Repo, commit: &Commit) -> Option { +fn extract_git_head(repo: &dyn Repo, commit: &Commit) -> Option> { let target = repo.view().git_head(); target .added_ids()