Skip to content

Commit

Permalink
rewrite.rs: optimization for new_parents
Browse files Browse the repository at this point in the history
As suggested by @yuja in
jj-vcs#2646 (comment)
  • Loading branch information
ilyagr committed Dec 13, 2023
1 parent d5ef639 commit 668bec5
Showing 1 changed file with 25 additions and 47 deletions.
72 changes: 25 additions & 47 deletions lib/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,56 +386,34 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
}

/// Panics if `parent_mapping` contains cycles
fn new_parents(&self, old_ids: &[CommitId]) -> Vec<CommitId> {
fn single_substitution_round(
parent_mapping: &HashMap<CommitId, Vec<CommitId>>,
ids: Vec<CommitId>,
) -> (Vec<CommitId>, bool) {
let mut made_replacements = false;
let mut new_ids = vec![];
// TODO(ilyagr): (Maybe?) optimize common case of replacements all
// being singletons. If CommitId-s were Copy. no allocations would be needed in
// that case, but it probably doesn't matter much while they are Vec<u8>-s.
for id in ids.into_iter() {
match parent_mapping.get(&id) {
None => new_ids.push(id),
Some(replacements) => {
assert!(
// Each commit must have a parent, so a parent can
// not just be mapped to nothing. This assertion
// could be removed if this function is used for
// mapping something other than a commit's parents.
!replacements.is_empty(),
"Found empty value for key {id:?} in the parent mapping",
);
made_replacements = true;
new_ids.extend(replacements.iter().cloned())
}
};
}
(new_ids, made_replacements)
}

let mut new_ids: Vec<CommitId> = old_ids.to_vec();
// The longest possible non-cycle substitution sequence goes through each key of
// parent_mapping once.
let mut allowed_iterations = 0..self.parent_mapping.len();
loop {
let made_replacements;
(new_ids, made_replacements) = single_substitution_round(&self.parent_mapping, new_ids);
if !made_replacements {
break;
fn new_parents(&self, mut ids: Vec<CommitId>) -> Vec<CommitId> {
let mut i = 0;
while i < ids.len() {
// The longest possible non-cycle substitution sequence goes through each key of
// parent_mapping once.
let mut allowed_iterations = 0..self.parent_mapping.len();
while let Some(replacements) = self.parent_mapping.get(&ids[i]) {
allowed_iterations
.next()
.expect("cycle detected in the parent mapping");
assert!(
// Each commit must have a parent, so a parent can
// not just be mapped to nothing. If this assertion is
// removed, adjust the looping logic.
!replacements.is_empty(),
"Found empty value for key {:?} in the parent mapping",
&ids[i]
);
ids.splice(i..i + 1, replacements.iter().cloned());
}
allowed_iterations
.next()
.expect("cycle detected in the parent mapping");
i += 1;
}
match new_ids.as_slice() {
match ids.as_slice() {
// The first two cases are an optimization for the common case of commits with <=2
// parents
[_singleton] => new_ids,
[a, b] if a != b => new_ids,
_ => new_ids.into_iter().unique().collect(),
[_singleton] => ids,
[a, b] if a != b => ids,
_ => ids.into_iter().unique().collect(),
}
}

Expand Down Expand Up @@ -539,7 +517,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
continue;
}
let old_parent_ids = old_commit.parent_ids();
let new_parent_ids = self.new_parents(old_parent_ids);
let new_parent_ids = self.new_parents(old_parent_ids.to_vec());
if self.abandoned.contains(&old_commit_id) {
// Update the `new_parents` map so descendants are rebased correctly.
self.parent_mapping
Expand Down

0 comments on commit 668bec5

Please sign in to comment.