Patchwork D12175: status: prefer relative paths in Rust code

login
register
mail settings
Submitter phabricator
Date Feb. 14, 2022, 6:01 p.m.
Message ID <differential-rev-PHID-DREV-6dmk44yyowbonowpaamm-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50510/
State New
Headers show

Comments

phabricator - Feb. 14, 2022, 6:01 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION 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)

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D12175

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

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-core/src/dirstate_tree/status.rs b/rust/hg-core/src/dirstate_tree/status.rs
--- a/rust/hg-core/src/dirstate_tree/status.rs
+++ b/rust/hg-core/src/dirstate_tree/status.rs
@@ -65,6 +65,26 @@ 
 
     let filesystem_time_at_status_start =
         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 {
         filesystem_time_at_status_start,
         ..Default::default()
@@ -752,13 +772,16 @@ 
     /// * Elsewhere, we’re listing the content of a sub-repo. Return an empty
     ///   list instead.
     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();
-        for entry in path.read_dir()? {
+        for entry in read_dir_path.read_dir()? {
             let entry = entry?;
             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
-            if name == b".hg" {
+            if file_name == ".hg" {
                 if is_at_repo_root {
                     // Skip the repo’s own .hg (might be a symlink)
                     continue;
@@ -768,9 +791,15 @@ 
                     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 {
-                base_name: name.into(),
-                full_path: entry.path(),
+                base_name,
+                full_path,
                 metadata,
             })
         }