ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

MutableEvolution: make MutableRepo responsible for lazy calculation

Browse source 
This patch continues the work from the previous pathc. From this
patch, we no longer calculate the evolution state just because a
transaction starts. We still unnecessarily calculate it when adding a
commit within the transaction, however. I'll fix that next.
This commit is contained in:
Martin von Zweigbergk 2021-03-15 04:15:35 -07:00 committed by Martin von Zweigbergk
parent 61acee52f4
commit f0619c07ac
4 changed files with 68 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
lib/src/evolution.rs
View file

@ -315,12 +315,12 @@ impl State {
}
}
pub enum EvolutionRef<'a, 'm: 'a, 'r: 'm> {
pub enum EvolutionRef<'a> {
Readonly(Arc<ReadonlyEvolution>),
Mutable(&'a MutableEvolution<'m, 'r>),
Mutable(&'a MutableEvolution),
}
impl EvolutionRef<'_, '_, '_> {
impl EvolutionRef<'_> {
pub fn successors(&self, commit_id: &CommitId) -> HashSet<CommitId> {
match self {
EvolutionRef::Readonly(evolution) => evolution.successors(commit_id),
@ -367,7 +367,7 @@ impl EvolutionRef<'_, '_, '_> {
pub fn new_parent(&self, store: &StoreWrapper, old_parent_id: &CommitId) -> HashSet<CommitId> {
match self {
EvolutionRef::Readonly(evolution) => evolution.new_parent(store, old_parent_id),
EvolutionRef::Mutable(evolution) => evolution.new_parent(old_parent_id),
EvolutionRef::Mutable(evolution) => evolution.new_parent(store, old_parent_id),
}
}
}
@ -400,12 +400,8 @@ impl ReadonlyEvolution {
}
}
pub fn start_modification<'r: 'm, 'm>(
&self,
repo: &'m MutableRepo<'r>,
) -> MutableEvolution<'r, 'm> {
pub fn start_modification(&self) -> MutableEvolution {
MutableEvolution {
repo,
state: self.state.clone(),
}
}
@ -431,12 +427,17 @@ impl ReadonlyEvolution {
}
}
pub struct MutableEvolution<'r: 'm, 'm> {
repo: &'m MutableRepo<'r>,
pub struct MutableEvolution {
state: State,
}
impl MutableEvolution<'_, '_> {
impl MutableEvolution {
pub fn new(repo: &MutableRepo) -> MutableEvolution {
MutableEvolution {
state: State::calculate(repo.as_repo_ref()),
}
}
pub fn successors(&self, commit_id: &CommitId) -> HashSet<CommitId> {
self.state.successors(commit_id)
}
@ -453,17 +454,13 @@ impl MutableEvolution<'_, '_> {
self.state.is_divergent(change_id)
}
pub fn new_parent(&self, old_parent_id: &CommitId) -> HashSet<CommitId> {
self.state.new_parent(self.repo.store(), old_parent_id)
pub fn new_parent(&self, store: &StoreWrapper, old_parent_id: &CommitId) -> HashSet<CommitId> {
self.state.new_parent(store, old_parent_id)
}
pub fn add_commit(&mut self, commit: &Commit) {
self.state.add_commit(commit);
}
pub fn invalidate(&mut self) {
self.state = State::calculate(self.repo.as_repo_ref());
}
}
pub fn evolve(
@ -527,7 +524,7 @@ pub fn evolve(
let mut ambiguous_new_parents = false;
let evolution = tx.mut_repo().evolution();
for old_parent in &old_parents {
let new_parent_candidates = evolution.new_parent(old_parent.id());
let new_parent_candidates = evolution.new_parent(&store, old_parent.id());
if new_parent_candidates.len() > 1 {
ambiguous_new_parents = true;
break;

67
lib/src/repo.rs
View file

@ -97,7 +97,7 @@ impl<'a, 'r> RepoRef<'a, 'r> {
}
}
pub fn evolution(&self) -> EvolutionRef<'a, 'a, 'r> {
pub fn evolution(&self) -> EvolutionRef {
match self {
RepoRef::Readonly(repo) => EvolutionRef::Readonly(repo.evolution()),
RepoRef::Mutable(repo) => EvolutionRef::Mutable(repo.evolution()),
@ -346,7 +346,8 @@ impl ReadonlyRepo {
}
pub fn start_transaction(&self, description: &str) -> Transaction {
let mut_repo = MutableRepo::new(self, self.index(), &self.view, self.evolution());
let locked_evolution = self.evolution.lock().unwrap();
let mut_repo = MutableRepo::new(self, self.index(), &self.view, locked_evolution.as_ref());
Transaction::new(mut_repo, description)
}
@ -482,7 +483,7 @@ pub struct MutableRepo<'r> {
repo: &'r ReadonlyRepo,
index: MutableIndex,
view: MutableView,
evolution: Option<MutableEvolution<'static, 'static>>,
evolution: Mutex<Option<MutableEvolution>>,
}
impl<'r> MutableRepo<'r> {
@ -490,24 +491,17 @@ impl<'r> MutableRepo<'r> {
repo: &'r ReadonlyRepo,
index: Arc<ReadonlyIndex>,
view: &ReadonlyView,
evolution: Arc<ReadonlyEvolution>,
evolution: Option<&Arc<ReadonlyEvolution>>,
) -> Arc<MutableRepo<'r>> {
let mut_view = view.start_modification();
let mut_index = MutableIndex::incremental(index);
let mut mut_repo = Arc::new(MutableRepo {
let mut_evolution = evolution.map(|evolution| evolution.start_modification());
Arc::new(MutableRepo {
repo,
index: mut_index,
view: mut_view,
evolution: None,
});
let repo_ref: &MutableRepo = mut_repo.as_ref();
let static_lifetime_repo: &'static MutableRepo = unsafe { std::mem::transmute(repo_ref) };
let mut_evolution: MutableEvolution<'_, '_> =
evolution.start_modification(static_lifetime_repo);
let static_lifetime_mut_evolution: MutableEvolution<'static, 'static> =
unsafe { std::mem::transmute(mut_evolution) };
Arc::get_mut(&mut mut_repo).unwrap().evolution = Some(static_lifetime_mut_evolution);
mut_repo
evolution: Mutex::new(mut_evolution),
})
}
pub fn as_repo_ref(&self) -> RepoRef {
@ -538,10 +532,31 @@ impl<'r> MutableRepo<'r> {
(self.index, self.view)
}
pub fn evolution<'m>(&'m self) -> &MutableEvolution<'r, 'm> {
let evolution: &MutableEvolution<'static, 'static> = self.evolution.as_ref().unwrap();
let evolution: &MutableEvolution<'r, 'm> = unsafe { std::mem::transmute(evolution) };
evolution
pub fn evolution(&self) -> &MutableEvolution {
let mut locked_evolution = self.evolution.lock().unwrap();
if locked_evolution.is_none() {
locked_evolution.replace(MutableEvolution::new(self));
}
let evolution = locked_evolution.as_ref().unwrap();
// Extend lifetime from lifetime of MutexGuard to lifetime of self. Safe because
// the value won't change again except for by invalidate_evolution(), which
// requires a mutable reference.
unsafe { std::mem::transmute(evolution) }
}
pub fn evolution_mut(&mut self) -> &mut MutableEvolution {
let mut locked_evolution = self.evolution.lock().unwrap();
if locked_evolution.is_none() {
locked_evolution.replace(MutableEvolution::new(self));
}
// Extend lifetime from lifetime of MutexGuard to lifetime of self. Safe because
// the value won't change again except for by invalidate_evolution(), which
// requires a mutable reference.
unsafe { std::mem::transmute(locked_evolution.as_mut().unwrap()) }
}
pub fn invalidate_evolution(&mut self) {
self.evolution.lock().unwrap().take();
}
pub fn write_commit(&mut self, commit: store::Commit) -> Commit {
@ -610,7 +625,7 @@ impl<'r> MutableRepo<'r> {
{
self.index.add_commit(head);
self.view.add_head(head);
self.evolution.as_mut().unwrap().add_commit(head);
self.evolution_mut().add_commit(head);
} else {
let missing_commits = topo_order_reverse(
vec![head.clone()],
@ -627,23 +642,23 @@ impl<'r> MutableRepo<'r> {
self.index.add_commit(missing_commit);
}
self.view.add_head(head);
self.evolution.as_mut().unwrap().invalidate();
self.invalidate_evolution();
}
}
pub fn remove_head(&mut self, head: &Commit) {
self.view.remove_head(head);
self.evolution.as_mut().unwrap().invalidate();
self.invalidate_evolution();
}
pub fn add_public_head(&mut self, head: &Commit) {
self.view.add_public_head(head);
self.evolution.as_mut().unwrap().add_commit(head);
self.evolution_mut().add_commit(head);
}
pub fn remove_public_head(&mut self, head: &Commit) {
self.view.remove_public_head(head);
self.evolution.as_mut().unwrap().invalidate();
self.invalidate_evolution();
}
pub fn insert_git_ref(&mut self, name: String, commit_id: CommitId) {
@ -656,7 +671,7 @@ impl<'r> MutableRepo<'r> {
pub fn set_view(&mut self, data: op_store::View) {
self.view.set_view(data);
self.evolution.as_mut().unwrap().invalidate();
self.invalidate_evolution();
}
pub fn merge(&mut self, base_repo: &ReadonlyRepo, other_repo: &ReadonlyRepo) {
@ -674,6 +689,6 @@ impl<'r> MutableRepo<'r> {
);
self.view.set_view(merged_view);
self.evolution.as_mut().unwrap().invalidate();
self.invalidate_evolution();
}
}

20
lib/tests/test_evolution.rs
View file

@ -164,7 +164,7 @@ fn test_new_parent_rewritten(use_git: bool) {
.set_change_id(original.change_id().clone())
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![rewritten.id().clone()].into_iter().collect()
);
tx.discard();
@ -184,7 +184,7 @@ fn test_new_parent_cherry_picked(use_git: bool) {
.set_predecessors(vec![original.id().clone()])
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![original.id().clone()].into_iter().collect()
);
tx.discard();
@ -208,7 +208,7 @@ fn test_new_parent_is_pruned(use_git: bool) {
.set_change_id(original.change_id().clone())
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![new_parent.id().clone()].into_iter().collect()
);
tx.discard();
@ -237,7 +237,7 @@ fn test_new_parent_divergent(use_git: bool) {
.set_change_id(original.change_id().clone())
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![
rewritten1.id().clone(),
rewritten2.id().clone(),
@ -278,7 +278,7 @@ fn test_new_parent_divergent_one_not_pruned(use_git: bool) {
.set_pruned(true)
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![
rewritten1.id().clone(),
parent2.id().clone(),
@ -321,7 +321,7 @@ fn test_new_parent_divergent_all_pruned(use_git: bool) {
.set_pruned(true)
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![
parent1.id().clone(),
parent2.id().clone(),
@ -357,7 +357,7 @@ fn test_new_parent_split(use_git: bool) {
.set_predecessors(vec![original.id().clone()])
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![rewritten3.id().clone()].into_iter().collect()
);
tx.discard();
@ -391,7 +391,7 @@ fn test_new_parent_split_pruned_descendant(use_git: bool) {
.set_predecessors(vec![original.id().clone()])
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![rewritten2.id().clone()].into_iter().collect()
);
tx.discard();
@ -425,7 +425,7 @@ fn test_new_parent_split_forked(use_git: bool) {
.set_predecessors(vec![original.id().clone()])
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![rewritten2.id().clone(), rewritten3.id().clone()]
.into_iter()
.collect()
@ -461,7 +461,7 @@ fn test_new_parent_split_forked_pruned(use_git: bool) {
.set_predecessors(vec![original.id().clone()])
.write_to_transaction(&mut tx);
assert_eq!(
tx.evolution().new_parent(original.id()),
tx.evolution().new_parent(tx.store(), original.id()),
vec![rewritten3.id().clone()].into_iter().collect()
);
tx.discard();

2
src/commands.rs
View file

@ -257,7 +257,7 @@ fn update_working_copy(
fn update_checkout_after_rewrite(ui: &mut Ui, tx: &mut Transaction) {
// TODO: Perhaps this method should be in Transaction.
let new_checkout_candidates = tx.evolution().new_parent(tx.view().checkout());
let new_checkout_candidates = tx.evolution().new_parent(tx.store(), tx.view().checkout());
if new_checkout_candidates.is_empty() {
return;
}