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!(