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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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 28280

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,
) )
}, },
) )
} }