This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10826

dirstate-v2: Skip readdir in status based on directory mtime
ClosedPublic

Authored by SimonSapin on Jun 1 2021, 12:50 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG7138c863d0a1: dirstate-v2: Skip readdir in status based on directory mtime
Summary

When calling read_dir during status and the directory is found to be
eligible for caching (see code comments), write the directory’s mtime to the
dirstate. The presence of a directory mtime in the dirstate is meaningful
and indicates eligibility.

When an eligible directory mtime is found in the dirstate and stat() shows
that the mtime has not changed, status can skip calling read_dir again
and instead rely on the names of child nodes in the dirstate tree.

The tempfile crate is used to create a temporary file in order to use its
modification time as "current time" with the same truncation as other files
and directories would have in their own modification time.

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 1 2021, 12:50 PM
SimonSapin added a commit: rHG7138c863d0a1: dirstate-v2: Skip readdir in status based on directory mtime.Jun 1 2021, 3:35 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG7138c863d0a1: dirstate-v2: Skip readdir in status based on directory mtime (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.
baymax updated this revision to Diff 28384.Jun 1 2021, 5:05 PM

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

Alphare added a subscriber: Alphare.Jun 2 2021, 4:08 AM

I have mostly feeback on form, here for posterity and for your follow-up patches.

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

I would point to the ambiguity resolution due to insufficient granularity in this comment so that we don't get the impression that this is naive too fast.

372

I find an early return (for the true case) would be easier to read since it would lead to less indentation.

680

This should probably log::debug that it failed to write a tempfile?

Revision Contents
Changeset List

Diff 28359

rust/hg-core/Cargo.toml

lazy_static = "1.4.0" lazy_static = "1.4.0"
rand = "0.7.3" rand = "0.7.3"
rand_pcg = "0.2.1" rand_pcg = "0.2.1"
rand_distr = "0.2.2" rand_distr = "0.2.2"
rayon = "1.3.0" rayon = "1.3.0"
regex = "1.3.9" regex = "1.3.9"
twox-hash = "1.5.0" twox-hash = "1.5.0"
same-file = "1.0.6" same-file = "1.0.6"
tempfile = "3.1.0"
crossbeam-channel = "0.4" crossbeam-channel = "0.4"
micro-timer = "0.3.0" micro-timer = "0.3.0"
log = "0.4.8" log = "0.4.8"
memmap = "0.7.0" memmap = "0.7.0"
zstd = "0.5.3" zstd = "0.5.3"
rust-crypto = "0.2.36" rust-crypto = "0.2.36"
format-bytes = "0.2.2" format-bytes = "0.2.2"
# We don't use the `miniz-oxide` backend to not change rhg benchmarks and until # We don't use the `miniz-oxide` backend to not change rhg benchmarks and until
# we have a clearer view of which backend is the fastest. # we have a clearer view of which backend is the fastest.
[dependencies.flate2] [dependencies.flate2]
version = "1.0.16" version = "1.0.16"
features = ["zlib"] features = ["zlib"]
default-features = false default-features = false
[dev-dependencies] [dev-dependencies]
clap = "*" clap = "*"
pretty_assertions = "0.6.1" pretty_assertions = "0.6.1"
tempfile = "3.1.0"

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

match self { match self {
NodeRef::InMemory(_path, node) => { NodeRef::InMemory(_path, node) => {
Ok(node.data.as_entry().map(|entry| entry.state)) Ok(node.data.as_entry().map(|entry| entry.state))
} }
NodeRef::OnDisk(node) => node.state(), NodeRef::OnDisk(node) => node.state(),
} }
} }
pub(super) fn cached_directory_mtime(
&self,
) -> Option<&on_disk::Timestamp> {
match self {
NodeRef::InMemory(_path, node) => match &node.data {
NodeData::CachedDirectory { mtime } => Some(mtime),
_ => None,
},
NodeRef::OnDisk(node) => node.cached_directory_mtime(),
}
}
pub(super) fn tracked_descendants_count(&self) -> u32 { pub(super) fn tracked_descendants_count(&self) -> u32 {
match self { match self {
NodeRef::InMemory(_path, node) => node.tracked_descendants_count, NodeRef::InMemory(_path, node) => node.tracked_descendants_count,
NodeRef::OnDisk(node) => node.tracked_descendants_count.get(), NodeRef::OnDisk(node) => node.tracked_descendants_count.get(),
} }
} }
} }
return Ok(Some(child)); return Ok(Some(child));
} }
} else { } else {
return Ok(None); return Ok(None);
} }
} }
} }
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),
) -> Result<&'tree mut Node<'on_disk>, DirstateV2ParseError> { ) -> Result<&'tree mut Node<'on_disk>, DirstateV2ParseError> {

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

base_name_start: Size, base_name_start: Size,
copy_source: OptPathSlice, copy_source: OptPathSlice,
children: ChildNodes, children: ChildNodes,
pub(super) tracked_descendants_count: Size, pub(super) tracked_descendants_count: Size,
/// Dependending on the value of `state`: /// Dependending on the value of `state`:
/// ///
/// * A null byte: `data` represents nothing /// * A null byte: `data` is not used.
///
/// * A `n`, `a`, `r`, or `m` ASCII byte: `state` and `data` together /// * A `n`, `a`, `r`, or `m` ASCII byte: `state` and `data` together
/// represents a dirstate entry like in the v1 format. /// represent a dirstate entry like in the v1 format.
///
/// * A `d` ASCII byte: the bytes of `data` should instead be interpreted /// * A `d` ASCII byte: the bytes of `data` should instead be interpreted
/// as the `Timestamp` for the mtime of a cached directory. /// as the `Timestamp` for the mtime of a cached directory.
/// ///
/// TODO: document directory caching /// The presence of this state means that at some point, this path in
/// the working directory was observed:
///
/// - To be a directory
/// - With the modification time as given by `Timestamp`
/// - That timestamp was already strictly in the past when observed,
/// meaning that later changes cannot happen in the same clock tick
/// and must cause a different modification time (unless the system
/// clock jumps back and we get unlucky, which is not impossible but
/// but deemed unlikely enough).
/// - The directory did not contain any child entry that did not have a
/// corresponding dirstate node.
///
/// This means that if `std::fs::symlink_metadata` later reports the
/// same modification time, we don’t need to call `std::fs::read_dir`
/// again for this directory and can iterate child dirstate nodes
/// instead.
state: u8, state: u8,
data: Entry, data: Entry,
} }
#[derive(BytesCast, Copy, Clone)] #[derive(BytesCast, Copy, Clone)]
#[repr(C)] #[repr(C)]
struct Entry { struct Entry {
mode: I32Be, mode: I32Be,
mtime: I32Be, mtime: I32Be,
size: I32Be, size: I32Be,
} }
/// Duration since the Unix epoch /// Duration since the Unix epoch
#[derive(BytesCast, Copy, Clone)] #[derive(BytesCast, Copy, Clone, PartialEq)]
#[repr(C)] #[repr(C)]
pub(super) struct Timestamp { pub(super) struct Timestamp {
seconds: I64Be, seconds: I64Be,
/// In `0 .. 1_000_000_000`. /// In `0 .. 1_000_000_000`.
/// ///
/// This timestamp is later or earlier than `(seconds, 0)` by this many /// This timestamp is later or earlier than `(seconds, 0)` by this many
/// nanoseconds, if `seconds` is non-negative or negative, respectively. /// nanoseconds, if `seconds` is non-negative or negative, respectively.
b'n' => Ok(entry(EntryState::Normal)), b'n' => Ok(entry(EntryState::Normal)),
b'a' => Ok(entry(EntryState::Added)), b'a' => Ok(entry(EntryState::Added)),
b'r' => Ok(entry(EntryState::Removed)), b'r' => Ok(entry(EntryState::Removed)),
b'm' => Ok(entry(EntryState::Merged)), b'm' => Ok(entry(EntryState::Merged)),
_ => Err(DirstateV2ParseError), _ => Err(DirstateV2ParseError),
} }
} }
pub(super) fn cached_directory_mtime(&self) -> Option<&Timestamp> {
if self.state == b'd' {
Some(self.data.as_timestamp())
} else {
None
}
}
pub(super) fn state( pub(super) fn state(
&self, &self,
) -> Result<Option<EntryState>, DirstateV2ParseError> { ) -> Result<Option<EntryState>, DirstateV2ParseError> {
match self.state { match self.state {
b'\0' | b'd' => Ok(None), b'\0' | b'd' => Ok(None),
b'n' => Ok(Some(EntryState::Normal)), b'n' => Ok(Some(EntryState::Normal)),
b'a' => Ok(Some(EntryState::Added)), b'a' => Ok(Some(EntryState::Added)),
b'r' => Ok(Some(EntryState::Removed)), b'r' => Ok(Some(EntryState::Removed)),
} }
fn as_timestamp(&self) -> &Timestamp { fn as_timestamp(&self) -> &Timestamp {
// Safety: same as above in `from_timestamp` // Safety: same as above in `from_timestamp`
unsafe { &*(self as *const Entry as *const Timestamp) } unsafe { &*(self as *const Entry as *const Timestamp) }
} }
} }
impl From<&'_ SystemTime> for Timestamp { impl From<SystemTime> for Timestamp {
fn from(system_time: &'_ SystemTime) -> Self { fn from(system_time: SystemTime) -> Self {
let (secs, nanos) = match system_time.duration_since(UNIX_EPOCH) { let (secs, nanos) = match system_time.duration_since(UNIX_EPOCH) {
Ok(duration) => { Ok(duration) => {
(duration.as_secs() as i64, duration.subsec_nanos()) (duration.as_secs() as i64, duration.subsec_nanos())
} }
Err(error) => { Err(error) => {
let negative = error.duration(); let negative = error.duration();
(-(negative.as_secs() as i64), negative.subsec_nanos()) (-(negative.as_secs() as i64), negative.subsec_nanos())
} }

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::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::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;
use crate::HgPathBuf; use crate::HgPathBuf;
use crate::PatternFileWarning; use crate::PatternFileWarning;
use crate::StatusError; use crate::StatusError;
use crate::StatusOptions; use crate::StatusOptions;
use micro_timer::timed; use micro_timer::timed;
use rayon::prelude::*; use rayon::prelude::*;
use std::borrow::Cow;
use std::io; use std::io;
use std::path::Path; use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::Mutex; use std::sync::Mutex;
use std::time::SystemTime;
/// Returns the status of the working directory compared to its parent /// Returns the status of the working directory compared to its parent
/// changeset. /// changeset.
/// ///
/// This algorithm is based on traversing the filesystem tree (`fs` in function /// This algorithm is based on traversing the filesystem tree (`fs` in function
/// and variable names) and dirstate tree at the same time. The core of this /// and variable names) and dirstate tree at the same time. The core of this
/// traversal is the recursive `traverse_fs_directory_and_dirstate` function /// traversal is the recursive `traverse_fs_directory_and_dirstate` function
/// and its use of `itertools::merge_join_by`. When reaching a path that only /// and its use of `itertools::merge_join_by`. When reaching a path that only
(Box::new(|&_| true), vec![]) (Box::new(|&_| true), vec![])
}; };
let common = StatusCommon { let common = StatusCommon {
dmap, dmap,
options, options,
matcher, matcher,
ignore_fn, ignore_fn,
outcome: Mutex::new(DirstateStatus::default()), outcome: Default::default(),
cached_directory_mtimes_to_add: Default::default(),
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_dir_metadata = None;
// 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
// 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_cached_mtime,
is_at_repo_root, is_at_repo_root,
)?; )?;
Ok((common.outcome.into_inner().unwrap(), warnings)) let outcome = common.outcome.into_inner().unwrap();
let to_add = common.cached_directory_mtimes_to_add.into_inner().unwrap();
for (path, mtime) in &to_add {
let node = DirstateMap::get_or_insert_node(
dmap.on_disk,
&mut dmap.root,
path,
WithBasename::to_cow_owned,
|_| {},
)?;
match &node.data {
NodeData::Entry(_) => {} // Don’t overwrite an entry
NodeData::CachedDirectory { .. } | NodeData::None => {
node.data = NodeData::CachedDirectory { mtime: *mtime }
}
}
}
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>>,
cached_directory_mtimes_to_add:
Mutex<Vec<(Cow<'on_disk, HgPath>, Timestamp)>>,
/// The current time at the start of the `status()` algorithm, as measured
/// and possibly truncated by the filesystem.
filesystem_time_at_status_start: Option<SystemTime>,
} }
impl<'a, 'tree, 'on_disk> StatusCommon<'a, 'tree, 'on_disk> { impl<'a, 'tree, 'on_disk> StatusCommon<'a, 'tree, 'on_disk> {
fn read_dir( fn read_dir(
&self, &self,
hg_path: &HgPath, hg_path: &HgPath,
fs_path: &Path, fs_path: &Path,
is_at_repo_root: bool, is_at_repo_root: bool,
) -> Result<Vec<DirEntry>, ()> { ) -> Result<Vec<DirEntry>, ()> {
DirEntry::read_dir(fs_path, is_at_repo_root) DirEntry::read_dir(fs_path, is_at_repo_root)
.map_err(|error| self.io_error(error, hg_path)) .map_err(|error| self.io_error(error, hg_path))
} }
fn io_error(&self, error: std::io::Error, hg_path: &HgPath) { fn io_error(&self, error: std::io::Error, hg_path: &HgPath) {
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)))
} }
/// 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
/// need to call `read_dir`.
fn can_skip_fs_readdir(
&self,
directory_metadata: Option<&std::fs::Metadata>,
cached_directory_mtime: Option<&Timestamp>,
) -> bool {
if !self.options.list_unknown && !self.options.list_ignored {
// All states that we care about listing have corresponding
// dirstate entries.
// This happens for example with `hg status -mard`.
return true;
}
if let Some(cached_mtime) = cached_directory_mtime {
// The dirstate contains a cached mtime for this directory, set by
// a previous run of the `status` algorithm which found this
// directory eligible for `read_dir` caching.
if let Some(meta) = directory_metadata {
if let Ok(current_mtime) = meta.modified() {
if current_mtime == cached_mtime.into() {
// The mtime of that directory has not changed since
AlphareUnsubmitted
Not Done

I would point to the ambiguity resolution due to insufficient granularity in this comment so that we don't get the impression that this is naive too fast.

Alphare: I would point to the ambiguity resolution due to insufficient granularity in this comment so…
// then, which means that the
// results of `read_dir` should also
// be unchanged.
return true;
}
}
}
}
false
}
/// Returns whether the filesystem directory was found to have any entry
/// that does not have a corresponding dirstate tree node.
fn traverse_fs_directory_and_dirstate( fn traverse_fs_directory_and_dirstate(
&self, &self,
has_ignored_ancestor: bool, has_ignored_ancestor: bool,
dirstate_nodes: ChildNodesRef<'tree, 'on_disk>, dirstate_nodes: ChildNodesRef<'tree, 'on_disk>,
directory_hg_path: &BorrowedPath<'tree, 'on_disk>, directory_hg_path: &BorrowedPath<'tree, 'on_disk>,
directory_fs_path: &Path, directory_fs_path: &Path,
directory_metadata: Option<&std::fs::Metadata>,
cached_directory_mtime: Option<&Timestamp>,
is_at_repo_root: bool, is_at_repo_root: bool,
) -> Result<(), DirstateV2ParseError> { ) -> Result<bool, DirstateV2ParseError> {
if !self.options.list_unknown && !self.options.list_ignored { if self.can_skip_fs_readdir(directory_metadata, cached_directory_mtime)
// We only care about files in the dirstate, so we can skip listing {
// filesystem directories entirely. dirstate_nodes
return dirstate_nodes
.par_iter() .par_iter()
.map(|dirstate_node| { .map(|dirstate_node| {
let fs_path = directory_fs_path.join(get_path_from_bytes( let fs_path = directory_fs_path.join(get_path_from_bytes(
dirstate_node.base_name(self.dmap.on_disk)?.as_bytes(), dirstate_node.base_name(self.dmap.on_disk)?.as_bytes(),
)); ));
match std::fs::symlink_metadata(&fs_path) { match std::fs::symlink_metadata(&fs_path) {
Ok(fs_metadata) => self.traverse_fs_and_dirstate( Ok(fs_metadata) => self.traverse_fs_and_dirstate(
&fs_path, &fs_path,
&fs_metadata, &fs_metadata,
dirstate_node, dirstate_node,
has_ignored_ancestor, has_ignored_ancestor,
), ),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => { Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
self.traverse_dirstate_only(dirstate_node) self.traverse_dirstate_only(dirstate_node)
} }
Err(error) => { Err(error) => {
let hg_path = let hg_path =
dirstate_node.full_path(self.dmap.on_disk)?; dirstate_node.full_path(self.dmap.on_disk)?;
Ok(self.io_error(error, hg_path)) Ok(self.io_error(error, hg_path))
} }
} }
}) })
.collect(); .collect::<Result<_, _>>()?;
// Conservatively don’t let the caller assume that there aren’t
// any, since we don’t know.
let directory_has_any_fs_only_entry = true;
return Ok(directory_has_any_fs_only_entry);
} }
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,
) { ) {
entries entries
.base_name(self.dmap.on_disk) .base_name(self.dmap.on_disk)
.unwrap() .unwrap()
.cmp(&fs_entry.base_name) .cmp(&fs_entry.base_name)
}, },
) )
.par_bridge() .par_bridge()
.map(|pair| { .map(|pair| {
use itertools::EitherOrBoth::*; use itertools::EitherOrBoth::*;
let is_fs_only = pair.is_right();
match pair { match pair {
Both(dirstate_node, fs_entry) => self Both(dirstate_node, fs_entry) => self
.traverse_fs_and_dirstate( .traverse_fs_and_dirstate(
&fs_entry.full_path, &fs_entry.full_path,
&fs_entry.metadata, &fs_entry.metadata,
dirstate_node, dirstate_node,
has_ignored_ancestor, has_ignored_ancestor,
), )?,
Left(dirstate_node) => { Left(dirstate_node) => {
self.traverse_dirstate_only(dirstate_node) self.traverse_dirstate_only(dirstate_node)?
} }
Right(fs_entry) => Ok(self.traverse_fs_only( Right(fs_entry) => self.traverse_fs_only(
has_ignored_ancestor, has_ignored_ancestor,
directory_hg_path, directory_hg_path,
fs_entry, fs_entry,
)), ),
} }
Ok(is_fs_only)
}) })
.collect() .try_reduce(|| false, |a, b| Ok(a || b))
} }
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,
self.outcome self.outcome
.lock() .lock()
.unwrap() .unwrap()
.traversed .traversed
.push(hg_path.detach_from_tree()) .push(hg_path.detach_from_tree())
} }
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( let directory_has_any_fs_only_entry = self
.traverse_fs_directory_and_dirstate(
is_ignored, is_ignored,
dirstate_node.children(self.dmap.on_disk)?, dirstate_node.children(self.dmap.on_disk)?,
hg_path, hg_path,
fs_path, fs_path,
Some(fs_metadata),
dirstate_node.cached_directory_mtime(),
is_at_repo_root, is_at_repo_root,
)?;
self.maybe_save_directory_mtime(
directory_has_any_fs_only_entry,
fs_metadata,
dirstate_node,
)? )?
} else { } else {
if file_or_symlink && self.matcher.matches(hg_path) { if file_or_symlink && self.matcher.matches(hg_path) {
if let Some(state) = dirstate_node.state()? { if let Some(state) = dirstate_node.state()? {
match state { match state {
EntryState::Added => self EntryState::Added => self
.outcome .outcome
.lock() .lock()
for child_node in dirstate_node.children(self.dmap.on_disk)?.iter() for child_node in dirstate_node.children(self.dmap.on_disk)?.iter()
{ {
self.traverse_dirstate_only(child_node)? self.traverse_dirstate_only(child_node)?
} }
} }
Ok(()) Ok(())
} }
fn maybe_save_directory_mtime(
&self,
directory_has_any_fs_only_entry: bool,
directory_metadata: &std::fs::Metadata,
dirstate_node: NodeRef<'tree, 'on_disk>,
) -> Result<(), DirstateV2ParseError> {
if !directory_has_any_fs_only_entry {
AlphareUnsubmitted
Not Done

I find an early return (for the true case) would be easier to read since it would lead to less indentation.

Alphare: I find an early return (for the true case) would be easier to read since it would lead to less…
// All filesystem directory entries from `read_dir` have a
// corresponding node in the dirstate, so we can reconstitute the
// names of those entries without calling `read_dir` again.
if let (Some(status_start), Ok(directory_mtime)) = (
&self.filesystem_time_at_status_start,
directory_metadata.modified(),
) {
// Although the Rust standard library’s `SystemTime` type
// has nanosecond precision, the times reported for a
// directory’s (or file’s) modified time may have lower
// resolution based on the filesystem (for example ext3
// only stores integer seconds), kernel (see
// https://stackoverflow.com/a/14393315/1162888), etc.
if &directory_mtime >= status_start {
// The directory was modified too recently, don’t cache its
// `read_dir` results.
//
// A timeline like this is possible:
//
// 1. A change to this directory (direct child was
// added or removed) cause its mtime to be set
// (possibly truncated) to `directory_mtime`
// 2. This `status` algorithm calls `read_dir`
// 3. An other change is made to the same directory is
// made so that calling `read_dir` agin would give
// different results, but soon enough after 1. that
// the mtime stays the same
//
// On a system where the time resolution poor, this
// scenario is not unlikely if all three steps are caused
// by the same script.
} else {
// We’ve observed (through `status_start`) that time has
// “progressed” since `directory_mtime`, so any further
// change to this directory is extremely likely to cause a
// different mtime.
//
// Having the same mtime again is not entirely impossible
// since the system clock is not monotonous. It could jump
// backward to some point before `directory_mtime`, then a
// directory change could potentially happen during exactly
// the wrong tick.
//
// We deem this scenario (unlike the previous one) to be
// unlikely enough in practice.
let timestamp = directory_mtime.into();
let cached = dirstate_node.cached_directory_mtime();
if cached != Some(&timestamp) {
let hg_path = dirstate_node
.full_path_borrowed(self.dmap.on_disk)?
.detach_from_tree();
self.cached_directory_mtimes_to_add
.lock()
.unwrap()
.push((hg_path, timestamp))
}
}
}
}
Ok(())
}
/// A file with `EntryState::Normal` in the dirstate was found in the /// A file with `EntryState::Normal` in the dirstate was found in the
/// filesystem /// filesystem
fn handle_normal_file( fn handle_normal_file(
&self, &self,
dirstate_node: &NodeRef<'tree, 'on_disk>, dirstate_node: &NodeRef<'tree, 'on_disk>,
fs_metadata: &std::fs::Metadata, fs_metadata: &std::fs::Metadata,
) -> Result<(), DirstateV2ParseError> { ) -> Result<(), DirstateV2ParseError> {
// Keep the low 31 bits // Keep the low 31 bits
base_name: name.into(), base_name: name.into(),
full_path: entry.path(), full_path: entry.path(),
metadata, metadata,
}) })
} }
Ok(results) Ok(results)
} }
} }
/// Return the `mtime` of a temporary file newly-created in the `.hg` directory
/// of the give repository.
///
/// This is similar to `SystemTime::now()`, with the result truncated to the
/// same time resolution as other files’ modification times. Using `.hg`
/// instead of the system’s default temporary directory (such as `/tmp`) makes
/// it more likely the temporary file is in the same disk partition as contents
/// of the working directory, which can matter since different filesystems may
/// store timestamps with different resolutions.
///
/// This may fail, typically if we lack write permissions. In that case we
/// should continue the `status()` algoritm anyway and consider the current
/// date/time to be unknown.
fn filesystem_now(repo_root: &Path) -> Result<SystemTime, io::Error> {
AlphareUnsubmitted
Not Done

This should probably log::debug that it failed to write a tempfile?

Alphare: This should probably `log::debug` that it failed to write a tempfile?
tempfile::tempfile_in(repo_root.join(".hg"))?
.metadata()?
.modified()
}