( )⚙ D10745 dirstate-tree: Downgrade `&mut Node` to `&Node` in status and serialization

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10745

dirstate-tree: Downgrade `&mut Node` to `&Node` in status and serialization
ClosedPublic

Authored by SimonSapin on May 19 2021, 12:36 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG0252600fd1cf: dirstate-tree: Downgrade `&mut Node` to `&Node` in status and serialization
Summary

Mutable access is not used, and upcoming changes will make it more costly
(with copy-on-write nodes that can be read from disk representation)

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.May 19 2021, 12:36 PM
Alphare added a subscriber: Alphare.May 21 2021, 3:47 AM
Alphare added inline comments.
rust/hg-core/src/dirstate_tree/status.rs
284

Shouldn't this be par_iter()?

baymax updated this revision to Diff 28169.May 21 2021, 4:37 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 28268.May 30 2021, 3:27 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.May 31 2021, 3:39 AM
This revision is now accepted and ready to land.May 31 2021, 3:39 AM

Revision Contents
Changeset List

Diff 28169

rust/hg-core/src/dirstate_tree/dirstate_map.rs

} }
impl<'on_disk> Node<'on_disk> { impl<'on_disk> Node<'on_disk> {
pub(super) fn state(&self) -> Option<EntryState> { pub(super) fn state(&self) -> Option<EntryState> {
self.entry.as_ref().map(|entry| entry.state) self.entry.as_ref().map(|entry| entry.state)
} }
pub(super) fn sorted<'tree>( pub(super) fn sorted<'tree>(
nodes: &'tree mut ChildNodes<'on_disk>, nodes: &'tree ChildNodes<'on_disk>,
) -> Vec<(&'tree NodeKey<'on_disk>, &'tree mut Self)> { ) -> Vec<(&'tree NodeKey<'on_disk>, &'tree Self)> {
let mut vec: Vec<_> = nodes.iter_mut().collect(); let mut vec: Vec<_> = nodes.iter().collect();
// `sort_unstable_by_key` doesn’t allow keys borrowing from the value: // `sort_unstable_by_key` doesn’t allow keys borrowing from the value:
// https://github.com/rust-lang/rust/issues/34162 // https://github.com/rust-lang/rust/issues/34162
vec.sort_unstable_by(|(path1, _), (path2, _)| path1.cmp(path2)); vec.sort_unstable_by(|(path1, _), (path2, _)| path1.cmp(path2));
vec vec
} }
} }
impl<'on_disk> DirstateMap<'on_disk> { impl<'on_disk> DirstateMap<'on_disk> {

rust/hg-core/src/dirstate_tree/on_disk.rs

.nodes_with_copy_source_count .nodes_with_copy_source_count
.into(), .into(),
}; };
out[..header_len].copy_from_slice(header.as_bytes()); out[..header_len].copy_from_slice(header.as_bytes());
Ok(out) Ok(out)
} }
fn write_nodes( fn write_nodes(
nodes: &mut dirstate_map::ChildNodes, nodes: &dirstate_map::ChildNodes,
out: &mut Vec<u8>, out: &mut Vec<u8>,
) -> Result<ChildNodes, DirstateError> { ) -> Result<ChildNodes, DirstateError> {
// `dirstate_map::ChildNodes` is a `HashMap` with undefined iteration // `dirstate_map::ChildNodes` is a `HashMap` with undefined iteration
// order. Sort to enable binary search in the written file. // order. Sort to enable binary search in the written file.
let nodes = dirstate_map::Node::sorted(nodes); let nodes = dirstate_map::Node::sorted(nodes);
// First accumulate serialized nodes in a `Vec` // First accumulate serialized nodes in a `Vec`
let mut on_disk_nodes = Vec::with_capacity(nodes.len()); let mut on_disk_nodes = Vec::with_capacity(nodes.len());
for (full_path, node) in nodes { for (full_path, node) in nodes {
on_disk_nodes.push(Node { on_disk_nodes.push(Node {
children: write_nodes(&mut node.children, out)?, children: write_nodes(&node.children, out)?,
tracked_descendants_count: node.tracked_descendants_count.into(), tracked_descendants_count: node.tracked_descendants_count.into(),
full_path: write_slice::<u8>( full_path: write_slice::<u8>(
full_path.full_path().as_bytes(), full_path.full_path().as_bytes(),
out, out,
), ),
base_name_start: u32::try_from(full_path.base_name_start()) base_name_start: u32::try_from(full_path.base_name_start())
// Could only panic for paths over 4 GiB // Could only panic for paths over 4 GiB
.expect("dirstate-v2 offset overflow") .expect("dirstate-v2 offset overflow")
.into(), .into(),
copy_source: if let Some(source) = &node.copy_source { copy_source: if let Some(source) = &node.copy_source {
write_slice::<u8>(source.as_bytes(), out) write_slice::<u8>(source.as_bytes(), out)
} else { } else {
Slice { Slice {
start: 0.into(), start: 0.into(),
len: 0.into(), len: 0.into(),
} }
}, },
entry: if let Some(entry) = &mut node.entry { entry: if let Some(entry) = &node.entry {
OptEntry { OptEntry {
state: entry.state.into(), state: entry.state.into(),
mode: entry.mode.into(), mode: entry.mode.into(),
mtime: entry.mtime.into(), mtime: entry.mtime.into(),
size: entry.size.into(), size: entry.size.into(),
} }
} else { } else {
OptEntry { OptEntry {

rust/hg-core/src/dirstate_tree/status.rs

ignore_fn, ignore_fn,
outcome: Mutex::new(DirstateStatus::default()), outcome: Mutex::new(DirstateStatus::default()),
}; };
let is_at_repo_root = true; let is_at_repo_root = true;
let hg_path = HgPath::new(""); let hg_path = HgPath::new("");
let has_ignored_ancestor = false; let has_ignored_ancestor = false;
common.traverse_fs_directory_and_dirstate( common.traverse_fs_directory_and_dirstate(
has_ignored_ancestor, has_ignored_ancestor,
&mut dmap.root, &dmap.root,
hg_path, hg_path,
&root_dir, &root_dir,
is_at_repo_root, is_at_repo_root,
); );
Ok((common.outcome.into_inner().unwrap(), warnings)) Ok((common.outcome.into_inner().unwrap(), warnings))
} }
/// Bag of random things needed by various parts of the algorithm. Reduces the /// Bag of random things needed by various parts of the algorithm. Reduces the
.bad .bad
.push((hg_path.to_owned().into(), BadMatch::OsError(errno))) .push((hg_path.to_owned().into(), BadMatch::OsError(errno)))
}) })
} }
fn traverse_fs_directory_and_dirstate( fn traverse_fs_directory_and_dirstate(
&self, &self,
has_ignored_ancestor: bool, has_ignored_ancestor: bool,
dirstate_nodes: &'tree mut ChildNodes, dirstate_nodes: &'tree ChildNodes,
directory_hg_path: &'tree HgPath, directory_hg_path: &'tree HgPath,
directory_fs_path: &Path, directory_fs_path: &Path,
is_at_repo_root: bool, is_at_repo_root: bool,
) { ) {
let mut fs_entries = if let Ok(entries) = self.read_dir( let mut fs_entries = if let Ok(entries) = self.read_dir(
directory_hg_path, directory_hg_path,
directory_fs_path, directory_fs_path,
is_at_repo_root, is_at_repo_root,
} }
}) })
} }
fn traverse_fs_and_dirstate( fn traverse_fs_and_dirstate(
&self, &self,
fs_entry: &DirEntry, fs_entry: &DirEntry,
hg_path: &'tree HgPath, hg_path: &'tree HgPath,
dirstate_node: &'tree mut Node, dirstate_node: &'tree Node,
has_ignored_ancestor: bool, has_ignored_ancestor: bool,
) { ) {
let file_type = fs_entry.metadata.file_type(); let file_type = fs_entry.metadata.file_type();
let file_or_symlink = file_type.is_file() || file_type.is_symlink(); let file_or_symlink = file_type.is_file() || file_type.is_symlink();
if !file_or_symlink { if !file_or_symlink {
// If we previously had a file here, it was removed (with // If we previously had a file here, it was removed (with
// `hg rm` or similar) or deleted before it could be // `hg rm` or similar) or deleted before it could be
// replaced by a directory or something else. // replaced by a directory or something else.
self.mark_removed_or_deleted_if_file( self.mark_removed_or_deleted_if_file(
hg_path, hg_path,
dirstate_node.state(), dirstate_node.state(),
); );
} }
if file_type.is_dir() { if file_type.is_dir() {
if self.options.collect_traversed_dirs { if self.options.collect_traversed_dirs {
self.outcome.lock().unwrap().traversed.push(hg_path.into()) self.outcome.lock().unwrap().traversed.push(hg_path.into())
} }
let is_ignored = has_ignored_ancestor || (self.ignore_fn)(hg_path); let is_ignored = has_ignored_ancestor || (self.ignore_fn)(hg_path);
let is_at_repo_root = false; let is_at_repo_root = false;
self.traverse_fs_directory_and_dirstate( self.traverse_fs_directory_and_dirstate(
is_ignored, is_ignored,
&mut dirstate_node.children, &dirstate_node.children,
hg_path, hg_path,
&fs_entry.full_path, &fs_entry.full_path,
is_at_repo_root, is_at_repo_root,
); );
} else { } else {
if file_or_symlink && self.matcher.matches(hg_path) { if file_or_symlink && self.matcher.matches(hg_path) {
let full_path = Cow::from(hg_path); let full_path = Cow::from(hg_path);
if let Some(entry) = &dirstate_node.entry { if let Some(entry) = &dirstate_node.entry {
// node, but the filesystem has a file // node, but the filesystem has a file
self.mark_unknown_or_ignored( self.mark_unknown_or_ignored(
has_ignored_ancestor, has_ignored_ancestor,
full_path, full_path,
) )
} }
} }
for (child_hg_path, child_node) in &mut dirstate_node.children { for (child_hg_path, child_node) in &dirstate_node.children {
self.traverse_dirstate_only( self.traverse_dirstate_only(
child_hg_path.full_path(), child_hg_path.full_path(),
child_node, child_node,
) )
} }
} }
} }
} }
} }
} }
/// A node in the dirstate tree has no corresponding filesystem entry /// A node in the dirstate tree has no corresponding filesystem entry
fn traverse_dirstate_only( fn traverse_dirstate_only(
&self, &self,
hg_path: &'tree HgPath, hg_path: &'tree HgPath,
dirstate_node: &'tree mut Node, dirstate_node: &'tree Node,
) { ) {
self.mark_removed_or_deleted_if_file(hg_path, dirstate_node.state()); self.mark_removed_or_deleted_if_file(hg_path, dirstate_node.state());
dirstate_node.children.par_iter_mut().for_each( dirstate_node.children.par_iter().for_each(
AlphareUnsubmitted
Not Done

Shouldn't this be par_iter()?

Alphare: Shouldn't this be `par_iter()`?
|(child_hg_path, child_node)| { |(child_hg_path, child_node)| {
self.traverse_dirstate_only( self.traverse_dirstate_only(
child_hg_path.full_path(), child_hg_path.full_path(),
child_node, child_node,
) )
}, },
) )
} }