This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10907

dirstate-v2: Drop cached read_dir results after .hgignore changes
ClosedPublic

Authored by SimonSapin on Jun 24 2021, 4:52 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGc657beacdf2e: dirstate-v2: Drop cached read_dir results after .hgignore changes
Summary

Soon we’ll want the status algorithm to be able to skip std::fs::read_dir in
more cases, notabling when listing unknown files but not ignored files.

When ignore patterns change (which we detect by their hash, added to the
dirstate-v2 format in a previous changeset), a formerly-ignored file could
become unknown without changing its parent directory’s modification time.
Therefore we remove any directory mtime from the dirstate, effictively
invalidating the existing caches.

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.Jun 24 2021, 4:52 PM
Alphare added a subscriber: Alphare.Jun 25 2021, 10:24 AM

Won't this potentially miss some directories if we're in a partial match for example?

SimonSapin added a comment.Jun 25 2021, 10:35 AM

Good question. At the moment the matcher is only consulted for files and symlinks. Later if we add an optimization to skip "full" subtree traversal of unmatched directories, we’ll likely need to have a simpler traversal that only checks for outdated cache.

Alphare accepted this revision.Jul 2 2021, 11:59 AM
This revision is now accepted and ready to land.Jul 2 2021, 11:59 AM

Revision Contents
Changeset List

Diff 28737

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

return Ok(Some(child)); return Ok(Some(child));
} }
} else { } else {
return Ok(None); return Ok(None);
} }
} }
} }
pub(super) fn get_or_insert<'tree, 'path>(
&'tree mut self,
path: &HgPath,
) -> Result<&'tree mut Node<'on_disk>, DirstateV2ParseError> {
Self::get_or_insert_node(
self.on_disk,
&mut self.root,
path,
WithBasename::to_cow_owned,
|_| {},
)
}
pub(super) fn get_or_insert_node<'tree, 'path>( pub(super) fn get_or_insert_node<'tree, 'path>(
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
root: &'tree mut ChildNodes<'on_disk>, root: &'tree mut ChildNodes<'on_disk>,
path: &'path HgPath, path: &'path HgPath,
to_cow: impl Fn( to_cow: impl Fn(
WithBasename<&'path HgPath>, WithBasename<&'path HgPath>,
) -> WithBasename<Cow<'on_disk, HgPath>>, ) -> WithBasename<Cow<'on_disk, HgPath>>,
mut each_ancestor: impl FnMut(&mut Node), mut each_ancestor: impl FnMut(&mut Node),

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

use crate::dirstate::status::IgnoreFnType; use crate::dirstate::status::IgnoreFnType;
use crate::dirstate_tree::dirstate_map::BorrowedPath; use crate::dirstate_tree::dirstate_map::BorrowedPath;
use crate::dirstate_tree::dirstate_map::ChildNodesRef; use crate::dirstate_tree::dirstate_map::ChildNodesRef;
use crate::dirstate_tree::dirstate_map::DirstateMap; use crate::dirstate_tree::dirstate_map::DirstateMap;
use crate::dirstate_tree::dirstate_map::NodeData; use crate::dirstate_tree::dirstate_map::NodeData;
use crate::dirstate_tree::dirstate_map::NodeRef; use crate::dirstate_tree::dirstate_map::NodeRef;
use crate::dirstate_tree::on_disk::DirstateV2ParseError; use crate::dirstate_tree::on_disk::DirstateV2ParseError;
use crate::dirstate_tree::on_disk::Timestamp; use crate::dirstate_tree::on_disk::Timestamp;
use crate::dirstate_tree::path_with_basename::WithBasename;
use crate::matchers::get_ignore_function; use crate::matchers::get_ignore_function;
use crate::matchers::Matcher; use crate::matchers::Matcher;
use crate::utils::files::get_bytes_from_os_string; use crate::utils::files::get_bytes_from_os_string;
use crate::utils::files::get_path_from_bytes; use crate::utils::files::get_path_from_bytes;
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use crate::BadMatch; use crate::BadMatch;
use crate::DirstateStatus; use crate::DirstateStatus;
use crate::EntryState; use crate::EntryState;
let common = StatusCommon { let common = StatusCommon {
dmap, dmap,
options, options,
matcher, matcher,
ignore_fn, ignore_fn,
outcome: Default::default(), outcome: Default::default(),
ignore_patterns_have_changed: patterns_changed, ignore_patterns_have_changed: patterns_changed,
new_cachable_directories: Default::default(), new_cachable_directories: Default::default(),
outated_cached_directories: Default::default(),
filesystem_time_at_status_start: filesystem_now(&root_dir).ok(), filesystem_time_at_status_start: filesystem_now(&root_dir).ok(),
}; };
let is_at_repo_root = true; let is_at_repo_root = true;
let hg_path = &BorrowedPath::OnDisk(HgPath::new("")); let hg_path = &BorrowedPath::OnDisk(HgPath::new(""));
let has_ignored_ancestor = false; let has_ignored_ancestor = false;
let root_cached_mtime = None; let root_cached_mtime = None;
let root_dir_metadata = None; let root_dir_metadata = None;
// If the path we have for the repository root is a symlink, do follow it. // If the path we have for the repository root is a symlink, do follow it.
// (As opposed to symlinks within the working directory which are not // (As opposed to symlinks within the working directory which are not
// followed, using `std::fs::symlink_metadata`.) // followed, using `std::fs::symlink_metadata`.)
common.traverse_fs_directory_and_dirstate( common.traverse_fs_directory_and_dirstate(
has_ignored_ancestor, has_ignored_ancestor,
dmap.root.as_ref(), dmap.root.as_ref(),
hg_path, hg_path,
&root_dir, &root_dir,
root_dir_metadata, root_dir_metadata,
root_cached_mtime, root_cached_mtime,
is_at_repo_root, is_at_repo_root,
)?; )?;
let mut outcome = common.outcome.into_inner().unwrap(); let mut outcome = common.outcome.into_inner().unwrap();
let new_cachable = common.new_cachable_directories.into_inner().unwrap(); let new_cachable = common.new_cachable_directories.into_inner().unwrap();
let outdated = common.outated_cached_directories.into_inner().unwrap();
outcome.dirty = common.ignore_patterns_have_changed == Some(true) outcome.dirty = common.ignore_patterns_have_changed == Some(true)
|| !outdated.is_empty()
|| !new_cachable.is_empty(); || !new_cachable.is_empty();
// Remove outdated mtimes before adding new mtimes, in case a given
// directory is both
for path in &outdated {
let node = dmap.get_or_insert(path)?;
if let NodeData::CachedDirectory { .. } = &node.data {
node.data = NodeData::None
}
}
for (path, mtime) in &new_cachable { for (path, mtime) in &new_cachable {
let node = DirstateMap::get_or_insert_node( let node = dmap.get_or_insert(path)?;
dmap.on_disk,
&mut dmap.root,
path,
WithBasename::to_cow_owned,
|_| {},
)?;
match &node.data { match &node.data {
NodeData::Entry(_) => {} // Don’t overwrite an entry NodeData::Entry(_) => {} // Don’t overwrite an entry
NodeData::CachedDirectory { .. } | NodeData::None => { NodeData::CachedDirectory { .. } | NodeData::None => {
node.data = NodeData::CachedDirectory { mtime: *mtime } node.data = NodeData::CachedDirectory { mtime: *mtime }
} }
} }
} }
Ok((outcome, warnings)) Ok((outcome, 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
/// number of parameters passed to functions. /// number of parameters passed to functions.
struct StatusCommon<'a, 'tree, 'on_disk: 'tree> { struct StatusCommon<'a, 'tree, 'on_disk: 'tree> {
dmap: &'tree DirstateMap<'on_disk>, dmap: &'tree DirstateMap<'on_disk>,
options: StatusOptions, options: StatusOptions,
matcher: &'a (dyn Matcher + Sync), matcher: &'a (dyn Matcher + Sync),
ignore_fn: IgnoreFnType<'a>, ignore_fn: IgnoreFnType<'a>,
outcome: Mutex<DirstateStatus<'on_disk>>, outcome: Mutex<DirstateStatus<'on_disk>>,
new_cachable_directories: Mutex<Vec<(Cow<'on_disk, HgPath>, Timestamp)>>, new_cachable_directories: Mutex<Vec<(Cow<'on_disk, HgPath>, Timestamp)>>,
outated_cached_directories: Mutex<Vec<Cow<'on_disk, HgPath>>>,
/// Whether ignore files like `.hgignore` have changed since the previous /// Whether ignore files like `.hgignore` have changed since the previous
/// time a `status()` call wrote their hash to the dirstate. `None` means /// time a `status()` call wrote their hash to the dirstate. `None` means
/// we don’t know as this run doesn’t list either ignored or uknown files /// we don’t know as this run doesn’t list either ignored or uknown files
/// and therefore isn’t reading `.hgignore`. /// and therefore isn’t reading `.hgignore`.
ignore_patterns_have_changed: Option<bool>, ignore_patterns_have_changed: Option<bool>,
/// The current time at the start of the `status()` algorithm, as measured /// The current time at the start of the `status()` algorithm, as measured
let errno = error.raw_os_error().expect("expected real OS error"); let errno = error.raw_os_error().expect("expected real OS error");
self.outcome self.outcome
.lock() .lock()
.unwrap() .unwrap()
.bad .bad
.push((hg_path.to_owned().into(), BadMatch::OsError(errno))) .push((hg_path.to_owned().into(), BadMatch::OsError(errno)))
} }
fn check_for_outdated_directory_cache(
&self,
dirstate_node: &NodeRef<'tree, 'on_disk>,
) -> Result<(), DirstateV2ParseError> {
if self.ignore_patterns_have_changed == Some(true)
&& dirstate_node.cached_directory_mtime().is_some()
{
self.outated_cached_directories.lock().unwrap().push(
dirstate_node
.full_path_borrowed(self.dmap.on_disk)?
.detach_from_tree(),
)
}
Ok(())
}
/// If this returns true, we can get accurate results by only using /// If this returns true, we can get accurate results by only using
/// `symlink_metadata` for child nodes that exist in the dirstate and don’t /// `symlink_metadata` for child nodes that exist in the dirstate and don’t
/// need to call `read_dir`. /// need to call `read_dir`.
fn can_skip_fs_readdir( fn can_skip_fs_readdir(
&self, &self,
directory_metadata: Option<&std::fs::Metadata>, directory_metadata: Option<&std::fs::Metadata>,
cached_directory_mtime: Option<&Timestamp>, cached_directory_mtime: Option<&Timestamp>,
) -> bool { ) -> bool {
fn traverse_fs_and_dirstate( fn traverse_fs_and_dirstate(
&self, &self,
fs_path: &Path, fs_path: &Path,
fs_metadata: &std::fs::Metadata, fs_metadata: &std::fs::Metadata,
dirstate_node: NodeRef<'tree, 'on_disk>, dirstate_node: NodeRef<'tree, 'on_disk>,
has_ignored_ancestor: bool, has_ignored_ancestor: bool,
) -> Result<(), DirstateV2ParseError> { ) -> Result<(), DirstateV2ParseError> {
self.check_for_outdated_directory_cache(&dirstate_node)?;
let hg_path = &dirstate_node.full_path_borrowed(self.dmap.on_disk)?; let hg_path = &dirstate_node.full_path_borrowed(self.dmap.on_disk)?;
let file_type = fs_metadata.file_type(); let file_type = fs_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(
Ok(()) Ok(())
} }
/// 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,
dirstate_node: NodeRef<'tree, 'on_disk>, dirstate_node: NodeRef<'tree, 'on_disk>,
) -> Result<(), DirstateV2ParseError> { ) -> Result<(), DirstateV2ParseError> {
self.check_for_outdated_directory_cache(&dirstate_node)?;
self.mark_removed_or_deleted_if_file( self.mark_removed_or_deleted_if_file(
&dirstate_node.full_path_borrowed(self.dmap.on_disk)?, &dirstate_node.full_path_borrowed(self.dmap.on_disk)?,
dirstate_node.state()?, dirstate_node.state()?,
); );
dirstate_node dirstate_node
.children(self.dmap.on_disk)? .children(self.dmap.on_disk)?
.par_iter() .par_iter()
.map(|child_node| self.traverse_dirstate_only(child_node)) .map(|child_node| self.traverse_dirstate_only(child_node))