Skip to content

Commit

Permalink
templater: fast-path empty and conflict to not read trees
Browse files Browse the repository at this point in the history
When there's a single parent, we can determine if a commit is empty by
just comparing the tree ids. Also, when using tree-level conflicts, we
don't need to read the trees to determine if there's a conflict. This
patch adds both of those fast paths, speeding up `jj log -r ::main`
from 317 ms to 227 ms (-28.4%). It has much larger impact with our
cloud-based backend at Google (~5x faster).

I made the same fix in the revset engine and the Git push code (thanks
to Yuya for the suggestion).
  • Loading branch information
martinvonz committed Sep 27, 2023
1 parent e5ad32e commit e50f6ac
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 5 deletions.
2 changes: 1 addition & 1 deletion cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ fn cmd_git_push(
{
reasons.push("it has no author and/or committer set");
}
if commit.tree()?.has_conflict() {
if commit.has_conflict()? {
reasons.push("it has conflicts");
}
if !reasons.is_empty() {
Expand Down
9 changes: 6 additions & 3 deletions cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,13 @@ fn build_commit_keyword_opt<'repo>(
let maybe_entries = repo.resolve_change_id(commit.change_id());
maybe_entries.map_or(true, |entries| !entries.contains(commit.id()))
})),
"conflict" => language.wrap_boolean(wrap_fn(property, |commit| {
commit.tree().unwrap().has_conflict()
})),
"conflict" => {
language.wrap_boolean(wrap_fn(property, |commit| commit.has_conflict().unwrap()))
}
"empty" => language.wrap_boolean(wrap_fn(property, |commit| {
if let [parent] = &commit.parents()[..] {
return parent.tree_id() == commit.tree_id();
}
let parent_tree = rewrite::merge_commit_trees(repo, &commit.parents()).unwrap();
*commit.tree_id() == parent_tree.id()
})),
Expand Down
8 changes: 8 additions & 0 deletions lib/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ impl Commit {
&self.data.root_tree
}

pub fn has_conflict(&self) -> Result<bool, BackendError> {
if let MergedTreeId::Merge(tree_ids) = self.tree_id() {
Ok(!tree_ids.is_resolved())
} else {
Ok(self.tree()?.has_conflict())
}
}

pub fn change_id(&self) -> &ChangeId {
&self.data.change_id
}
Expand Down
2 changes: 1 addition & 1 deletion lib/src/default_revset_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ fn build_predicate_fn<'index>(
}
RevsetFilterPredicate::HasConflict => pure_predicate_fn(move |entry| {
let commit = store.get_commit(&entry.commit_id()).unwrap();
commit.tree().unwrap().has_conflict()
commit.has_conflict().unwrap()
}),
}
}
Expand Down

0 comments on commit e50f6ac

Please sign in to comment.