Skip to content

Commit

Permalink
merged_tree: micro-optimize Merge<Tree> entries iterator to return &T…
Browse files Browse the repository at this point in the history
…reeValue

try_resolve_file_conflict() is also updated. It could be a generic function,
but there are only two callers, and the legacy tree one is used only in tests.
  • Loading branch information
yuja committed Aug 12, 2024
1 parent 6d6f599 commit 88018e8
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
12 changes: 6 additions & 6 deletions lib/src/merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ fn all_tree_entries(
/// than `tree.entries_non_recursive()`.
fn all_merged_tree_entries(
trees: &Merge<Tree>,
) -> impl Iterator<Item = (&RepoPathComponent, MergedTreeValue)> {
) -> impl Iterator<Item = (&RepoPathComponent, Merge<Option<&TreeValue>>)> {
let mut entries_iters = trees
.iter()
.map(|tree| tree.entries_non_recursive().peekable())
Expand All @@ -404,7 +404,7 @@ fn all_merged_tree_entries(
.iter_mut()
.map(|iter| {
let entry = iter.next_if(|entry| entry.name() == next_name)?;
Some(entry.value().clone())
Some(entry.value())
})
.collect();
Some((next_name, values.build()))
Expand Down Expand Up @@ -456,7 +456,7 @@ fn merge_trees(merge: &Merge<Tree>) -> BackendResult<Merge<Tree>> {
let mut conflicts = vec![];
for (basename, path_merge) in all_merged_tree_entries(merge) {
let path = dir.join(basename);
let path_merge = merge_tree_values(store, &path, path_merge)?;
let path_merge = merge_tree_values(store, &path, &path_merge)?;
match path_merge.into_resolved() {
Ok(value) => {
new_tree.set_or_remove(basename, value);
Expand Down Expand Up @@ -493,10 +493,10 @@ fn merge_trees(merge: &Merge<Tree>) -> BackendResult<Merge<Tree>> {
fn merge_tree_values(
store: &Arc<Store>,
path: &RepoPath,
values: MergedTreeValue,
values: &Merge<Option<&TreeValue>>,
) -> BackendResult<MergedTreeValue> {
if let Some(resolved) = values.resolve_trivial() {
return Ok(Merge::resolved(resolved.clone()));
return Ok(Merge::resolved(resolved.cloned()));
}

if let Some(trees) = values.to_tree_merge(store, path)? {
Expand All @@ -517,7 +517,7 @@ fn merge_tree_values(
Ok(Merge::normal(resolved))
} else {
// Failed to merge the files, or the paths are not files
Ok(values)
Ok(values.map(|value| value.cloned()))
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions lib/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::backend::{
};
use crate::files::MergeResult;
use crate::matchers::{EverythingMatcher, Matcher};
use crate::merge::{trivial_merge, Merge, MergedTreeValue};
use crate::merge::{trivial_merge, Merge};
use crate::object_id::ObjectId;
use crate::repo_path::{RepoPath, RepoPathBuf, RepoPathComponent};
use crate::store::Store;
Expand Down Expand Up @@ -382,8 +382,9 @@ fn merge_tree_value(
match merge.into_resolved() {
Ok(value) => value,
Err(conflict) => {
let conflict_borrowed = conflict.map(|value| value.as_ref());
if let Some(tree_value) =
try_resolve_file_conflict(store, &filename, &conflict)?
try_resolve_file_conflict(store, &filename, &conflict_borrowed)?
{
Some(tree_value)
} else {
Expand All @@ -403,7 +404,7 @@ fn merge_tree_value(
pub fn try_resolve_file_conflict(
store: &Store,
filename: &RepoPath,
conflict: &MergedTreeValue,
conflict: &Merge<Option<&TreeValue>>,
) -> BackendResult<Option<TreeValue>> {
// If there are any non-file or any missing parts in the conflict, we can't
// merge it. We check early so we don't waste time reading file contents if
Expand Down

0 comments on commit 88018e8

Please sign in to comment.