From 5aec8b9d77325e0fb0b4a18a7b3cfc3382400e5a Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 16 Mar 2021 22:51:05 -0700 Subject: [PATCH] evolution: use index for filtering out ancestors of candidates in new_parent() This speeds up `jj evolve` of 100 linear commits of the "what's cooking" branch in the git.git repo further, from ~700 ms to ~400 ms. --- lib/src/evolution.rs | 38 ++++++++++++++++++------------------- lib/tests/test_evolution.rs | 20 +++++++++---------- src/commands.rs | 2 +- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/lib/src/evolution.rs b/lib/src/evolution.rs index 4e10d8035..74a851067 100644 --- a/lib/src/evolution.rs +++ b/lib/src/evolution.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use crate::commit::Commit; use crate::commit_builder::CommitBuilder; -use crate::dag_walk::{bfs, closest_common_node, leaves, walk_ancestors}; +use crate::dag_walk::{bfs, closest_common_node, leaves}; use crate::repo::{MutableRepo, ReadonlyRepo, RepoRef}; use crate::repo_path::DirRepoPath; use crate::rewrite::{merge_commit_trees, rebase_commit}; @@ -228,7 +228,8 @@ impl State { } } - pub fn new_parent(&self, store: &StoreWrapper, old_parent_id: &CommitId) -> HashSet { + pub fn new_parent(&self, repo: RepoRef, old_parent_id: &CommitId) -> HashSet { + let store = repo.store(); let mut new_parents = HashSet::new(); if let Some(successor_ids) = self.successors.get(old_parent_id) { let old_parent = store.get_commit(old_parent_id).unwrap(); @@ -286,22 +287,20 @@ impl State { ); for candidate in candidates { - all_candidates.insert(candidate.clone()); + all_candidates.insert(candidate.id().clone()); } } - // Filter out candidates that are ancestors of or other candidates. - let non_heads: Vec<_> = all_candidates - .iter() - .flat_map(|commit| commit.parents()) + // Filter out candidates that are ancestors of other candidates. + let all_candidates: Vec = repo + .index() + .heads(all_candidates.iter()) + .into_iter() .collect(); - for commit in walk_ancestors(non_heads) { - all_candidates.remove(&commit); - } for candidate in all_candidates { // TODO: Make this not recursive - for effective_successor in self.new_parent(store, candidate.id()) { + for effective_successor in self.new_parent(repo, &candidate) { new_parents.insert(effective_successor); } } @@ -363,10 +362,10 @@ impl EvolutionRef<'_> { /// change id as A). Then C is rebased to somewhere else and becomes C'. /// We will choose that C' as effective successor even though it has a /// different change id and is not a descendant of one that does. - pub fn new_parent(&self, store: &StoreWrapper, old_parent_id: &CommitId) -> HashSet { + pub fn new_parent(&self, repo: RepoRef, old_parent_id: &CommitId) -> HashSet { match self { - EvolutionRef::Readonly(evolution) => evolution.new_parent(store, old_parent_id), - EvolutionRef::Mutable(evolution) => evolution.new_parent(store, old_parent_id), + EvolutionRef::Readonly(evolution) => evolution.new_parent(repo, old_parent_id), + EvolutionRef::Mutable(evolution) => evolution.new_parent(repo, old_parent_id), } } } @@ -421,8 +420,8 @@ impl ReadonlyEvolution { self.state.is_divergent(change_id) } - pub fn new_parent(&self, store: &StoreWrapper, old_parent_id: &CommitId) -> HashSet { - self.state.new_parent(store, old_parent_id) + pub fn new_parent(&self, repo: RepoRef, old_parent_id: &CommitId) -> HashSet { + self.state.new_parent(repo, old_parent_id) } } @@ -453,8 +452,8 @@ impl MutableEvolution { self.state.is_divergent(change_id) } - pub fn new_parent(&self, store: &StoreWrapper, old_parent_id: &CommitId) -> HashSet { - self.state.new_parent(store, old_parent_id) + pub fn new_parent(&self, repo: RepoRef, old_parent_id: &CommitId) -> HashSet { + self.state.new_parent(repo, old_parent_id) } pub fn add_commit(&mut self, commit: &Commit) { @@ -521,7 +520,8 @@ pub fn evolve( let mut ambiguous_new_parents = false; let evolution = mut_repo.evolution(); for old_parent in &old_parents { - let new_parent_candidates = evolution.new_parent(&store, old_parent.id()); + let new_parent_candidates = + evolution.new_parent(mut_repo.as_repo_ref(), old_parent.id()); if new_parent_candidates.len() > 1 { ambiguous_new_parents = true; break; diff --git a/lib/tests/test_evolution.rs b/lib/tests/test_evolution.rs index 7e75195d4..ca3897b7f 100644 --- a/lib/tests/test_evolution.rs +++ b/lib/tests/test_evolution.rs @@ -174,7 +174,7 @@ fn test_new_parent_rewritten(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![rewritten.id().clone()].into_iter().collect() ); tx.discard(); @@ -197,7 +197,7 @@ fn test_new_parent_cherry_picked(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![original.id().clone()].into_iter().collect() ); tx.discard(); @@ -224,7 +224,7 @@ fn test_new_parent_is_pruned(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![new_parent.id().clone()].into_iter().collect() ); tx.discard(); @@ -256,7 +256,7 @@ fn test_new_parent_divergent(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![ rewritten1.id().clone(), rewritten2.id().clone(), @@ -300,7 +300,7 @@ fn test_new_parent_divergent_one_not_pruned(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![ rewritten1.id().clone(), parent2.id().clone(), @@ -346,7 +346,7 @@ fn test_new_parent_divergent_all_pruned(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![ parent1.id().clone(), parent2.id().clone(), @@ -385,7 +385,7 @@ fn test_new_parent_split(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![rewritten3.id().clone()].into_iter().collect() ); tx.discard(); @@ -422,7 +422,7 @@ fn test_new_parent_split_pruned_descendant(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![rewritten2.id().clone()].into_iter().collect() ); tx.discard(); @@ -459,7 +459,7 @@ fn test_new_parent_split_forked(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![rewritten2.id().clone(), rewritten3.id().clone()] .into_iter() .collect() @@ -498,7 +498,7 @@ fn test_new_parent_split_forked_pruned(use_git: bool) { assert_eq!( mut_repo .evolution() - .new_parent(mut_repo.store(), original.id()), + .new_parent(mut_repo.as_repo_ref(), original.id()), vec![rewritten3.id().clone()].into_iter().collect() ); tx.discard(); diff --git a/src/commands.rs b/src/commands.rs index 609c50e85..0f957f411 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -258,7 +258,7 @@ fn update_checkout_after_rewrite(ui: &mut Ui, mut_repo: &mut MutableRepo) { // TODO: Perhaps this method should be in MutableRepo. let new_checkout_candidates = mut_repo .evolution() - .new_parent(mut_repo.store(), mut_repo.view().checkout()); + .new_parent(mut_repo.as_repo_ref(), mut_repo.view().checkout()); if new_checkout_candidates.is_empty() { return; }