changeset 44535:07d9fd6097e6

rust-pathauditor: use interior mutability for use in multi-threaded contexts

The usual recommendation for using `RwLock` or `Mutex` is that if there are
about as many write as there are reads, use `Mutex`, and if there are more
reads than writes, use `RwLock`.

If after the main bottleneck (i.e. parallel traversal) is removed this shows
up on profiles, we should investigate using the `parking_lot` since we don't
need a poisoning API, or maybe move to different types of caches entirely.

Differential Revision: https://phab.mercurial-scm.org/D8213

--- a/rust/hg-core/src/dirstate/status.rs	Wed Mar 04 15:12:08 2020 +0100
+++ b/rust/hg-core/src/dirstate/status.rs	Wed Mar 04 15:10:11 2020 +0100
@@ -698,7 +698,7 @@
             // We walked all dirs under the roots that weren't ignored, and
             // everything that matched was stat'ed and is already in results.
             // The rest must thus be ignored or under a symlink.
-            let mut path_auditor = PathAuditor::new(root_dir);
+            let path_auditor = PathAuditor::new(root_dir);
 
             for (ref filename, entry) in to_visit {
                 // Report ignored items in the dmap as long as they are not

--- a/rust/hg-core/src/utils/files.rs	Wed Mar 04 15:12:08 2020 +0100
+++ b/rust/hg-core/src/utils/files.rs	Wed Mar 04 15:10:11 2020 +0100
@@ -210,7 +210,7 @@
     } else {
         name.to_owned()
     };
-    let mut auditor = PathAuditor::new(&root);
+    let auditor = PathAuditor::new(&root);
     if name != root && name.starts_with(&root) {
         let name = name.strip_prefix(&root).unwrap();
         auditor.audit_path(path_to_hg_path_buf(name)?)?;

--- a/rust/hg-core/src/utils/path_auditor.rs	Wed Mar 04 15:12:08 2020 +0100
+++ b/rust/hg-core/src/utils/path_auditor.rs	Wed Mar 04 15:10:11 2020 +0100
@@ -13,13 +13,14 @@
 };
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
+use std::sync::{Mutex, RwLock};
 
 /// Ensures that a path is valid for use in the repository i.e. does not use
 /// any banned components, does not traverse a symlink, etc.
 #[derive(Debug, Default)]
 pub struct PathAuditor {
-    audited: HashSet<HgPathBuf>,
-    audited_dirs: HashSet<HgPathBuf>,
+    audited: Mutex<HashSet<HgPathBuf>>,
+    audited_dirs: RwLock<HashSet<HgPathBuf>>,
     root: PathBuf,
 }
 
@@ -31,7 +32,7 @@
         }
     }
     pub fn audit_path(
-        &mut self,
+        &self,
         path: impl AsRef<HgPath>,
     ) -> Result<(), HgPathError> {
         // TODO windows "localpath" normalization
@@ -40,7 +41,7 @@
             return Ok(());
         }
         // TODO case normalization
-        if self.audited.contains(path) {
+        if self.audited.lock().unwrap().contains(path) {
             return Ok(());
         }
         // AIX ignores "/" at end of path, others raise EISDIR.
@@ -113,14 +114,14 @@
         for index in 0..parts.len() {
             let prefix = &parts[..index + 1].join(&b'/');
             let prefix = HgPath::new(prefix);
-            if self.audited_dirs.contains(prefix) {
+            if self.audited_dirs.read().unwrap().contains(prefix) {
                 continue;
             }
             self.check_filesystem(&prefix, &path)?;
-            self.audited_dirs.insert(prefix.to_owned());
+            self.audited_dirs.write().unwrap().insert(prefix.to_owned());
         }
 
-        self.audited.insert(path.to_owned());
+        self.audited.lock().unwrap().insert(path.to_owned());
 
         Ok(())
     }
@@ -171,7 +172,7 @@
         Ok(())
     }
 
-    pub fn check(&mut self, path: impl AsRef<HgPath>) -> bool {
+    pub fn check(&self, path: impl AsRef<HgPath>) -> bool {
         self.audit_path(path).is_ok()
     }
 }
@@ -184,7 +185,7 @@
 
     #[test]
     fn test_path_auditor() {
-        let mut auditor = PathAuditor::new(get_path_from_bytes(b"/tmp"));
+        let auditor = PathAuditor::new(get_path_from_bytes(b"/tmp"));
 
         let path = HgPath::new(b".hg/00changelog.i");
         assert_eq!(