# HG changeset patch # User Raphaël Gomès # Date 1583331011 -3600 # Node ID 07d9fd6097e6dc92978529bc257a9bbf885ad8d9 # Parent 8a237131ff0f5857d2b0d3791dd27d3547fed8b6 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 diff -r 8a237131ff0f -r 07d9fd6097e6 rust/hg-core/src/dirstate/status.rs --- 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 diff -r 8a237131ff0f -r 07d9fd6097e6 rust/hg-core/src/utils/files.rs --- 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)?)?; diff -r 8a237131ff0f -r 07d9fd6097e6 rust/hg-core/src/utils/path_auditor.rs --- 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, - audited_dirs: HashSet, + audited: Mutex>, + audited_dirs: RwLock>, root: PathBuf, } @@ -31,7 +32,7 @@ } } pub fn audit_path( - &mut self, + &self, path: impl AsRef, ) -> 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) -> bool { + pub fn check(&self, path: impl AsRef) -> 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!(