( )⚙ D12175 status: prefer relative paths in Rust code

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12175

status: prefer relative paths in Rust code
ClosedPublic

Authored by SimonSapin on Feb 14 2022, 1:00 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG94e36b230990: status: prefer relative paths in Rust code
Summary

… when the repository root is under the current directory,
so the kernel needs to traverse fewer directory in every call
to read_dir or symlink_metadata.

Better yet would be to use libc functions like openat and fstatat
to remove such repeated traversals entirely, but the standard library
does not provide APIs based on those.
Maybe with a crate like https://crates.io/crates/openat instead?

Benchmarks of rhg status show that this patch is neutral in some configurations,
and makes the command up to ~20% faster in others.
Below is semi-arbitrary subset of results. The four numeric columns are:
time (in seconds) with this changeset’s parent, time with this changeset,
time difference (negative is better), time ratio (less than 1 is better).

mercurial-dirstate-v1 | default-plain-clean.no-iu.pbr            | 0.0061 -> 0.0059: -0.0002 (0.97)
mercurial-dirstate-v2 | default-plain-clean.no-iu.pbr            | 0.0029 -> 0.0028: -0.0001 (0.97)
mozilla-dirstate-v1   | default-plain-clean.no-iu.pbr            | 0.2110 -> 0.2102: -0.0007 (1.00)
mozilla-dirstate-v2   | default-copies-clean.ignored.pbr         | 0.0489 -> 0.0401: -0.0088 (0.82)
mozilla-dirstate-v2   | default-copies-clean.no-iu.pbr           | 0.0479 -> 0.0393: -0.0085 (0.82)
mozilla-dirstate-v2   | default-copies-large.all.pbr             | 0.1262 -> 0.1210: -0.0051 (0.96)
mozilla-dirstate-v2   | default-copies-small.ignored-unknown.pbr | 0.1262 -> 0.1200: -0.0062 (0.95)
mozilla-dirstate-v2   | default-copies-small.ignored.pbr         | 0.0536 -> 0.0417: -0.0119 (0.78)
mozilla-dirstate-v2   | default-copies-small.no-iu.pbr           | 0.0482 -> 0.0393: -0.0089 (0.81)
mozilla-dirstate-v2   | default-plain-clean.ignored.pbr          | 0.0518 -> 0.0402: -0.0116 (0.78)
mozilla-dirstate-v2   | default-plain-clean.no-iu.pbr            | 0.0481 -> 0.0392: -0.0088 (0.82)
mozilla-dirstate-v2   | default-plain-large.all.pbr              | 0.1271 -> 0.1218: -0.0052 (0.96)
mozilla-dirstate-v2   | default-plain-small.ignored-unknown.pbr  | 0.1225 -> 0.1202: -0.0022 (0.98)
mozilla-dirstate-v2   | default-plain-small.ignored.pbr          | 0.0510 -> 0.0418: -0.0092 (0.82)
mozilla-dirstate-v2   | default-plain-small.no-iu.pbr            | 0.0480 -> 0.0394: -0.0086 (0.82)
netbeans-dirstate-v1  | default-plain-clean.no-iu.pbr            | 0.1442 -> 0.1422: -0.0020 (0.99)
netbeans-dirstate-v2  | default-plain-clean.no-iu.pbr            | 0.0325 -> 0.0282: -0.0043 (0.87)

Diff Detail

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

Event Timeline

SimonSapin created this revision.Feb 14 2022, 1:00 PM
Alphare accepted this revision.Feb 15 2022, 4:37 AM
This revision is now accepted and ready to land.Feb 15 2022, 4:37 AM
SimonSapin added a commit: rHG94e36b230990: status: prefer relative paths in Rust code.Feb 15 2022, 4:43 AM
Closed by commit rHG94e36b230990: status: prefer relative paths in Rust code (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 32174

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

dmap.ignore_patterns_hash = new_hash; dmap.ignore_patterns_hash = new_hash;
(ignore_fn, warnings, Some(changed)) (ignore_fn, warnings, Some(changed))
} else { } else {
(Box::new(|&_| true), vec![], None) (Box::new(|&_| true), vec![], None)
}; };
let filesystem_time_at_status_start = let filesystem_time_at_status_start =
filesystem_now(&root_dir).ok().map(TruncatedTimestamp::from); filesystem_now(&root_dir).ok().map(TruncatedTimestamp::from);
// If the repository is under the current directory, prefer using a
// relative path, so the kernel needs to traverse fewer directory in every
// call to `read_dir` or `symlink_metadata`.
// This is effective in the common case where the current directory is the
// repository root.
// TODO: Better yet would be to use libc functions like `openat` and
// `fstatat` to remove such repeated traversals entirely, but the standard
// library does not provide APIs based on those.
// Maybe with a crate like https://crates.io/crates/openat instead?
let root_dir = if let Some(relative) = std::env::current_dir()
.ok()
.and_then(|cwd| root_dir.strip_prefix(cwd).ok())
{
relative
} else {
&root_dir
};
let outcome = DirstateStatus { let outcome = DirstateStatus {
filesystem_time_at_status_start, filesystem_time_at_status_start,
..Default::default() ..Default::default()
}; };
let common = StatusCommon { let common = StatusCommon {
dmap, dmap,
options, options,
matcher, matcher,
/// metadata. /// metadata.
/// ///
/// If a `.hg` sub-directory is encountered: /// If a `.hg` sub-directory is encountered:
/// ///
/// * At the repository root, ignore that sub-directory /// * At the repository root, ignore that sub-directory
/// * Elsewhere, we’re listing the content of a sub-repo. Return an empty /// * Elsewhere, we’re listing the content of a sub-repo. Return an empty
/// list instead. /// list instead.
fn read_dir(path: &Path, is_at_repo_root: bool) -> io::Result<Vec<Self>> { fn read_dir(path: &Path, is_at_repo_root: bool) -> io::Result<Vec<Self>> {
// `read_dir` returns a "not found" error for the empty path
let at_cwd = path == Path::new("");
let read_dir_path = if at_cwd { Path::new(".") } else { path };
let mut results = Vec::new(); let mut results = Vec::new();
for entry in path.read_dir()? { for entry in read_dir_path.read_dir()? {
let entry = entry?; let entry = entry?;
let metadata = entry.metadata()?; let metadata = entry.metadata()?;
let name = get_bytes_from_os_string(entry.file_name()); let file_name = entry.file_name();
// FIXME don't do this when cached // FIXME don't do this when cached
if name == b".hg" { if file_name == ".hg" {
if is_at_repo_root { if is_at_repo_root {
// Skip the repo’s own .hg (might be a symlink) // Skip the repo’s own .hg (might be a symlink)
continue; continue;
} else if metadata.is_dir() { } else if metadata.is_dir() {
// A .hg sub-directory at another location means a subrepo, // A .hg sub-directory at another location means a subrepo,
// skip it entirely. // skip it entirely.
return Ok(Vec::new()); return Ok(Vec::new());
} }
} }
let full_path = if at_cwd {
file_name.clone().into()
} else {
entry.path()
};
let base_name = get_bytes_from_os_string(file_name).into();
results.push(DirEntry { results.push(DirEntry {
base_name: name.into(), base_name,
full_path: entry.path(), full_path,
metadata, metadata,
}) })
} }
Ok(results) Ok(results)
} }
} }
/// Return the `mtime` of a temporary file newly-created in the `.hg` directory /// Return the `mtime` of a temporary file newly-created in the `.hg` directory