merged_tree: hold store globally by TreeDiffIterator

Since TreeDiffDirItem is now calculated eagerly, it doesn't make sense to
keep MergedTree in it.
This commit is contained in:
Yuya Nishihara 2024-08-08 18:29:52 +09:00
parent 37c41d0eaf
commit 9378adedb7
2 changed files with 22 additions and 24 deletions

View file

@ -281,9 +281,7 @@ impl MergedTree {
let concurrency = self.store().concurrency(); let concurrency = self.store().concurrency();
if concurrency <= 1 { if concurrency <= 1 {
Box::pin(futures::stream::iter(TreeDiffIterator::new( Box::pin(futures::stream::iter(TreeDiffIterator::new(
self.clone(), self, other, matcher,
other.clone(),
matcher,
))) )))
} else { } else {
Box::pin(TreeDiffStreamImpl::new( Box::pin(TreeDiffStreamImpl::new(
@ -584,13 +582,12 @@ impl Iterator for ConflictIterator {
/// Iterator over the differences between two trees. /// Iterator over the differences between two trees.
pub struct TreeDiffIterator<'matcher> { pub struct TreeDiffIterator<'matcher> {
store: Arc<Store>,
stack: Vec<TreeDiffItem>, stack: Vec<TreeDiffItem>,
matcher: &'matcher dyn Matcher, matcher: &'matcher dyn Matcher,
} }
struct TreeDiffDirItem { struct TreeDiffDirItem {
tree1: MergedTree,
tree2: MergedTree,
entries: Vec<(RepoPathBuf, MergedTreeValue, MergedTreeValue)>, entries: Vec<(RepoPathBuf, MergedTreeValue, MergedTreeValue)>,
} }
@ -605,7 +602,8 @@ enum TreeDiffItem {
impl<'matcher> TreeDiffIterator<'matcher> { impl<'matcher> TreeDiffIterator<'matcher> {
/// Creates a iterator over the differences between two trees. Generally /// Creates a iterator over the differences between two trees. Generally
/// prefer `MergedTree::diff()` of calling this directly. /// prefer `MergedTree::diff()` of calling this directly.
pub fn new(tree1: MergedTree, tree2: MergedTree, matcher: &'matcher dyn Matcher) -> Self { pub fn new(tree1: &MergedTree, tree2: &MergedTree, matcher: &'matcher dyn Matcher) -> Self {
assert!(Arc::ptr_eq(tree1.store(), tree2.store()));
let root_dir = RepoPath::root(); let root_dir = RepoPath::root();
let mut stack = Vec::new(); let mut stack = Vec::new();
if !matcher.visit(root_dir).is_nothing() { if !matcher.visit(root_dir).is_nothing() {
@ -613,7 +611,11 @@ impl<'matcher> TreeDiffIterator<'matcher> {
root_dir, tree1, tree2, matcher, root_dir, tree1, tree2, matcher,
))); )));
}; };
Self { stack, matcher } Self {
store: tree1.store().clone(),
stack,
matcher,
}
} }
fn single_tree( fn single_tree(
@ -629,14 +631,14 @@ impl<'matcher> TreeDiffIterator<'matcher> {
/// Gets the given tree if `value` is a tree, otherwise an empty tree. /// Gets the given tree if `value` is a tree, otherwise an empty tree.
fn tree( fn tree(
tree: &MergedTree, store: &Arc<Store>,
dir: &RepoPath, dir: &RepoPath,
values: &MergedTreeValue, values: &MergedTreeValue,
) -> BackendResult<MergedTree> { ) -> BackendResult<MergedTree> {
let trees = if values.is_tree() { let trees = if values.is_tree() {
values.try_map(|value| Self::single_tree(tree.store(), dir, value.as_ref()))? values.try_map(|value| Self::single_tree(store, dir, value.as_ref()))?
} else { } else {
Merge::resolved(Tree::null(tree.store().clone(), dir.to_owned())) Merge::resolved(Tree::null(store.clone(), dir.to_owned()))
}; };
Ok(MergedTree { trees }) Ok(MergedTree { trees })
} }
@ -645,12 +647,12 @@ impl<'matcher> TreeDiffIterator<'matcher> {
impl TreeDiffDirItem { impl TreeDiffDirItem {
fn from_trees( fn from_trees(
dir: &RepoPath, dir: &RepoPath,
tree1: MergedTree, tree1: &MergedTree,
tree2: MergedTree, tree2: &MergedTree,
matcher: &dyn Matcher, matcher: &dyn Matcher,
) -> Self { ) -> Self {
let mut entries = vec![]; let mut entries = vec![];
for (name, before, after) in merged_tree_entry_diff(&tree1, &tree2) { for (name, before, after) in merged_tree_entry_diff(tree1, tree2) {
let path = dir.join(name); let path = dir.join(name);
let before = before.to_merge(); let before = before.to_merge();
let after = after.to_merge(); let after = after.to_merge();
@ -678,11 +680,7 @@ impl TreeDiffDirItem {
entries.push((path, before, after)); entries.push((path, before, after));
} }
entries.reverse(); entries.reverse();
Self { Self { entries }
tree1,
tree2,
entries,
}
} }
} }
@ -694,9 +692,9 @@ impl Iterator for TreeDiffIterator<'_> {
fn next(&mut self) -> Option<Self::Item> { fn next(&mut self) -> Option<Self::Item> {
while let Some(top) = self.stack.last_mut() { while let Some(top) = self.stack.last_mut() {
let (dir, (path, before, after)) = match top { let (path, before, after) = match top {
TreeDiffItem::Dir(dir) => match dir.entries.pop() { TreeDiffItem::Dir(dir) => match dir.entries.pop() {
Some(entry) => (dir, entry), Some(entry) => entry,
None => { None => {
self.stack.pop().unwrap(); self.stack.pop().unwrap();
continue; continue;
@ -714,16 +712,16 @@ impl Iterator for TreeDiffIterator<'_> {
let tree_before = before.is_tree(); let tree_before = before.is_tree();
let tree_after = after.is_tree(); let tree_after = after.is_tree();
let post_subdir = if tree_before || tree_after { let post_subdir = if tree_before || tree_after {
let before_tree = match Self::tree(&dir.tree1, &path, &before) { let before_tree = match Self::tree(&self.store, &path, &before) {
Ok(tree) => tree, Ok(tree) => tree,
Err(err) => return Some((path, Err(err))), Err(err) => return Some((path, Err(err))),
}; };
let after_tree = match Self::tree(&dir.tree2, &path, &after) { let after_tree = match Self::tree(&self.store, &path, &after) {
Ok(tree) => tree, Ok(tree) => tree,
Err(err) => return Some((path, Err(err))), Err(err) => return Some((path, Err(err))),
}; };
let subdir = let subdir =
TreeDiffDirItem::from_trees(&path, before_tree, after_tree, self.matcher); TreeDiffDirItem::from_trees(&path, &before_tree, &after_tree, self.matcher);
self.stack.push(TreeDiffItem::Dir(subdir)); self.stack.push(TreeDiffItem::Dir(subdir));
self.stack.len() - 1 self.stack.len() - 1
} else { } else {

View file

@ -35,7 +35,7 @@ fn file_value(file_id: &FileId) -> TreeValue {
} }
fn diff_stream_equals_iter(tree1: &MergedTree, tree2: &MergedTree, matcher: &dyn Matcher) { fn diff_stream_equals_iter(tree1: &MergedTree, tree2: &MergedTree, matcher: &dyn Matcher) {
let iter_diff: Vec<_> = TreeDiffIterator::new(tree1.clone(), tree2.clone(), matcher) let iter_diff: Vec<_> = TreeDiffIterator::new(tree1, tree2, matcher)
.map(|(path, diff)| (path, diff.unwrap())) .map(|(path, diff)| (path, diff.unwrap()))
.collect(); .collect();
let max_concurrent_reads = 10; let max_concurrent_reads = 10;