From d2ae8a8bf0ed4e6dd7dadbe8764b39a17e2187d9 Mon Sep 17 00:00:00 2001 From: Pedro Tacla Yamada Date: Tue, 20 Aug 2024 14:03:21 +1000 Subject: [PATCH] Remove dashmap (#10) --- .gitignore | 1 + CODE_OF_CONDUCT.md | 14 ++--- CONTRIBUTING.md | 10 ++-- Cargo.lock | 9 +-- crates/atlaspack_filesystem/Cargo.toml | 1 - crates/atlaspack_filesystem/src/lib.rs | 7 +-- .../src/os_file_system/canonicalize.rs | 16 +++-- crates/node-bindings/Cargo.toml | 1 - packages/utils/dev-dep-resolver/Cargo.toml | 2 +- packages/utils/dev-dep-resolver/src/lib.rs | 48 +++++++++------ packages/utils/node-resolver-rs/Cargo.toml | 1 - packages/utils/node-resolver-rs/src/cache.rs | 58 +++++++++++++------ 12 files changed, 99 insertions(+), 69 deletions(-) diff --git a/.gitignore b/.gitignore index aa6a405ca..4ce5bfb5c 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .idea/ .nyc_output/ .atlaspack-cache/ +.parcel-cache/ .vscode/* !.vscode/extensions.json coverage/ diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index c048a940b..139534425 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -6,13 +6,13 @@ We are committed to making participation in this project a harassment-free exper Examples of unacceptable behavior by participants include: -* The use of sexualized language or imagery -* Personal attacks -* Trolling or insulting/derogatory comments -* Public or private harassment -* Publishing other's private information, such as physical or electronic addresses, without explicit permission -* Submitting contributions or comments that you know to violate the intellectual property or privacy rights of others -* Other unethical or unprofessional conduct +- The use of sexualized language or imagery +- Personal attacks +- Trolling or insulting/derogatory comments +- Public or private harassment +- Publishing other's private information, such as physical or electronic addresses, without explicit permission +- Submitting contributions or comments that you know to violate the intellectual property or privacy rights of others +- Other unethical or unprofessional conduct Project maintainers have the right and responsibility to remove, edit, or reject comments, commits, code, wiki edits, issues, and other contributions that are not aligned to this Code of Conduct, or to ban temporarily or permanently any contributor for other behaviors that they deem inappropriate, threatening, offensive, or harmful. By adopting this Code of Conduct, project maintainers commit themselves to fairly and consistently applying these principles to every aspect of managing this project. Project maintainers who do not follow or enforce the Code of Conduct may be permanently removed from the project team. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 463176c87..3cb951a8a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,9 +6,9 @@ Thank you for considering a contribution to Atlaspack! Unfortunately pull reques For pull requests, please: -* Add tests for new features and bug fixes -* Follow the existing style -* Separate unrelated changes into multiple pull requests +- Add tests for new features and bug fixes +- Follow the existing style +- Separate unrelated changes into multiple pull requests See the existing issues for things to start contributing. @@ -18,5 +18,5 @@ Atlassian requires contributors to sign a Contributor License Agreement, known a Prior to accepting your contributions we ask that you please follow the appropriate link below to digitally sign the CLA. The Corporate CLA is for those who are contributing as a member of an organization and the individual CLA is for those contributing as an individual. -* [CLA for corporate contributors](https://opensource.atlassian.com/corporate) -* [CLA for individuals](https://opensource.atlassian.com/individual) +- [CLA for corporate contributors](https://opensource.atlassian.com/corporate) +- [CLA for individuals](https://opensource.atlassian.com/individual) diff --git a/Cargo.lock b/Cargo.lock index 252baf210..562b6ff27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -153,11 +153,11 @@ name = "atlaspack-dev-dep-resolver" version = "0.1.0" dependencies = [ "atlaspack-resolver", - "dashmap", "es-module-lexer", "glob", "rayon", "serde_json", + "xxhash-rust", ] [[package]] @@ -221,7 +221,6 @@ dependencies = [ "atlaspack_package_manager", "atlaspack_plugin_transformer_js", "crossbeam-channel", - "dashmap", "getrandom", "glob", "indexmap 1.9.3", @@ -258,7 +257,6 @@ dependencies = [ "atlaspack_filesystem", "bitflags 1.3.2", "criterion", - "dashmap", "glob-match", "indexmap 1.9.3", "is_elevated", @@ -347,7 +345,6 @@ version = "0.1.0" dependencies = [ "anyhow", "assert_fs", - "dashmap", "is_elevated", "mockall", "xxhash-rust", @@ -5034,9 +5031,9 @@ dependencies = [ [[package]] name = "xxhash-rust" -version = "0.8.10" +version = "0.8.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "927da81e25be1e1a2901d59b81b37dd2efd1fc9c9345a55007f09bf5a2d3ee03" +checksum = "6a5cbf750400958819fb6178eaa83bee5cd9c29a26a40cc241df8c70fdd46984" [[package]] name = "zerocopy" diff --git a/crates/atlaspack_filesystem/Cargo.toml b/crates/atlaspack_filesystem/Cargo.toml index 072323c4c..f5ef01362 100644 --- a/crates/atlaspack_filesystem/Cargo.toml +++ b/crates/atlaspack_filesystem/Cargo.toml @@ -5,7 +5,6 @@ edition = "2021" description = "FileSystem wrapper trait for use in Atlaspack codebase." [dependencies] -dashmap = "5.5.3" mockall = "0.12.1" xxhash-rust = { version = "0.8.2", features = ["xxh3"] } anyhow = "1.0.86" diff --git a/crates/atlaspack_filesystem/src/lib.rs b/crates/atlaspack_filesystem/src/lib.rs index 38f8370ee..72ccfaa3b 100644 --- a/crates/atlaspack_filesystem/src/lib.rs +++ b/crates/atlaspack_filesystem/src/lib.rs @@ -1,8 +1,7 @@ +use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; -use std::sync::Arc; - -use dashmap::DashMap; +use std::sync::{Arc, RwLock}; /// In-memory file-system for testing pub mod in_memory_file_system; @@ -18,7 +17,7 @@ pub mod os_file_system; pub type FileSystemRef = Arc; pub type FileSystemRealPathCache = - DashMap, xxhash_rust::xxh3::Xxh3Builder>; + RwLock, xxhash_rust::xxh3::Xxh3Builder>>; /// Trait abstracting file-system operations /// . diff --git a/crates/atlaspack_filesystem/src/os_file_system/canonicalize.rs b/crates/atlaspack_filesystem/src/os_file_system/canonicalize.rs index 110d4aec9..33a57358e 100644 --- a/crates/atlaspack_filesystem/src/os_file_system/canonicalize.rs +++ b/crates/atlaspack_filesystem/src/os_file_system/canonicalize.rs @@ -28,21 +28,27 @@ pub fn canonicalize(path: &Path, cache: &FileSystemRealPathCache) -> std::io::Re ret.push(c); // First, check the cache for the path up to this point. - let link = if let Some(cached) = cache.get(&ret) { - if let Some(link) = &*cached { - link.clone() + let read = cache.read().unwrap(); + let cached = read.get(&ret).cloned(); + drop(read); + let link = if let Some(cached) = cached { + if let Some(link) = cached { + link } else { continue; } } else { let stat = std::fs::symlink_metadata(&ret)?; if !stat.is_symlink() { - cache.insert(ret.clone(), None); + cache.write().unwrap().insert(ret.clone(), None); continue; } let link = std::fs::read_link(&ret)?; - cache.insert(ret.clone(), Some(link.clone())); + cache + .write() + .unwrap() + .insert(ret.clone(), Some(link.clone())); link }; diff --git a/crates/node-bindings/Cargo.toml b/crates/node-bindings/Cargo.toml index bc414c226..4d61aeaa9 100644 --- a/crates/node-bindings/Cargo.toml +++ b/crates/node-bindings/Cargo.toml @@ -22,7 +22,6 @@ atlaspack_plugin_transformer_js = { path = "../atlaspack_plugin_transformer_js" atlaspack_napi_helpers = { path = "../atlaspack_napi_helpers" } anyhow = "1.0.82" -dashmap = "5.4.0" glob = "0.3.1" log = "0.4.21" mockall = "0.12.1" diff --git a/packages/utils/dev-dep-resolver/Cargo.toml b/packages/utils/dev-dep-resolver/Cargo.toml index e96ef4d2e..2e23a7978 100644 --- a/packages/utils/dev-dep-resolver/Cargo.toml +++ b/packages/utils/dev-dep-resolver/Cargo.toml @@ -9,5 +9,5 @@ atlaspack-resolver = { path = "../node-resolver-rs" } es-module-lexer = { git = "https://github.com/devongovett/es-module-lexer" } serde_json = "1.0.91" rayon = "1.7.0" -dashmap = "5.4.0" glob = "0.3.1" +xxhash-rust = "0.8.12" diff --git a/packages/utils/dev-dep-resolver/src/lib.rs b/packages/utils/dev-dep-resolver/src/lib.rs index 0a31d70c0..35583dc04 100644 --- a/packages/utils/dev-dep-resolver/src/lib.rs +++ b/packages/utils/dev-dep-resolver/src/lib.rs @@ -1,7 +1,9 @@ use std::borrow::Cow; +use std::collections::{HashMap, HashSet}; use std::path::Component; use std::path::Path; use std::path::PathBuf; +use std::sync::{Arc, RwLock}; use atlaspack_resolver::CacheCow; use atlaspack_resolver::Invalidations; @@ -13,8 +15,6 @@ use atlaspack_resolver::ResolverError; use atlaspack_resolver::Specifier; use atlaspack_resolver::SpecifierError; use atlaspack_resolver::SpecifierType; -use dashmap::DashMap; -use dashmap::DashSet; use es_module_lexer::lex; use es_module_lexer::ImportKind; // use rayon::prelude::{ParallelBridge, ParallelIterator}; @@ -68,12 +68,12 @@ impl From for EsmGraphBuilderError { #[derive(Default)] pub struct Cache { - entries: DashMap, + entries: RwLock, xxhash_rust::xxh3::Xxh3Builder>>, } struct EsmGraphBuilder<'a> { - visited: DashSet, - visited_globs: DashSet, + visited: RwLock>, + visited_globs: RwLock>, invalidations: Invalidations, cjs_resolver: Resolver<'a>, esm_resolver: Resolver<'a>, @@ -82,11 +82,11 @@ struct EsmGraphBuilder<'a> { impl<'a> EsmGraphBuilder<'a> { pub fn build(&self, file: &Path) -> Result<(), EsmGraphBuilderError> { - if self.visited.contains(file) { + if self.visited.read().unwrap().contains(file) { return Ok(()); } - self.visited.insert(file.to_owned()); + self.visited.write().unwrap().insert(file.to_owned()); if let Some(ext) = file.extension() { if ext != "js" && ext != "cjs" && ext != "mjs" { @@ -95,14 +95,15 @@ impl<'a> EsmGraphBuilder<'a> { } } - if let Some(invalidations) = self.cache.entries.get(file) { + let read = self.cache.entries.read().unwrap(); + let value = read.get(file).cloned(); + drop(read); + if let Some(invalidations) = value { self.invalidations.extend(&invalidations); - for p in invalidations - .invalidate_on_file_change - .read() - .unwrap() - .iter() - { + let read = invalidations.invalidate_on_file_change.read().unwrap(); + let paths: Vec = read.iter().cloned().collect(); + drop(read); + for p in paths { self.build(&p)?; } return Ok(()); @@ -168,7 +169,12 @@ impl<'a> EsmGraphBuilder<'a> { .collect::>()?; self.invalidations.extend(&invalidations); - self.cache.entries.insert(file.to_owned(), invalidations); + self + .cache + .entries + .write() + .unwrap() + .insert(file.to_owned(), Arc::new(invalidations)); Ok(()) } @@ -207,11 +213,15 @@ impl<'a> EsmGraphBuilder<'a> { // Invalidate when new files match the glob. invalidations.invalidate_on_glob_create(pattern.to_string_lossy()); - if self.visited_globs.contains(&pattern) { + if self.visited_globs.read().unwrap().contains(&pattern) { return Ok(()); } - self.visited_globs.insert(pattern.to_path_buf()); + self + .visited_globs + .write() + .unwrap() + .insert(pattern.to_path_buf()); for path in glob::glob(pattern.to_string_lossy().as_ref())? { let path = path?; @@ -500,8 +510,8 @@ pub fn build_esm_graph( cache: &Cache, ) -> Result { let visitor = EsmGraphBuilder { - visited: DashSet::new(), - visited_globs: DashSet::new(), + visited: RwLock::new(HashSet::new()), + visited_globs: RwLock::new(HashSet::new()), invalidations: Invalidations::default(), cjs_resolver: Resolver::node( Cow::Borrowed(project_root), diff --git a/packages/utils/node-resolver-rs/Cargo.toml b/packages/utils/node-resolver-rs/Cargo.toml index d8a88a0a3..1cd8d2611 100644 --- a/packages/utils/node-resolver-rs/Cargo.toml +++ b/packages/utils/node-resolver-rs/Cargo.toml @@ -13,7 +13,6 @@ atlaspack_core = { path = "../../../crates/atlaspack_core" } atlaspack_filesystem = { path = "../../../crates/atlaspack_filesystem" } bitflags = "1.3.2" -dashmap = "5.4.0" glob-match = "0.2.1" indexmap = { version = "1.9.2", features = ["serde"] } itertools = "0.10.5" diff --git a/packages/utils/node-resolver-rs/src/cache.rs b/packages/utils/node-resolver-rs/src/cache.rs index e68ece170..62485f7cc 100644 --- a/packages/utils/node-resolver-rs/src/cache.rs +++ b/packages/utils/node-resolver-rs/src/cache.rs @@ -1,11 +1,10 @@ use std::borrow::Cow; +use std::collections::HashMap; use std::fmt; use std::ops::Deref; use std::path::Path; use std::path::PathBuf; -use std::sync::Arc; - -use dashmap::DashMap; +use std::sync::{Arc, RwLock}; use atlaspack_core::types::File; use atlaspack_filesystem::{FileSystemRealPathCache, FileSystemRef}; @@ -24,12 +23,13 @@ pub struct Cache { /// way to associate a lifetime with owned data stored in the same struct. We only vend temporary references /// from our public methods so this is ok for now. FrozenMap is an append only map, which doesn't require &mut /// to insert into. Since each value is in a Box, it won't move and therefore references are stable. - packages: DashMap, ResolverError>>, DefaultHasher>, - tsconfigs: DashMap, ResolverError>>, DefaultHasher>, + packages: RwLock, ResolverError>>, DefaultHasher>>, + tsconfigs: + RwLock, ResolverError>>, DefaultHasher>>, // In particular just the is_dir_cache spends around 8% of the time on a large project resolution // hashing paths. Instead of using a hashmap we should try a trie here. - is_dir_cache: DashMap, - is_file_cache: DashMap, + is_dir_cache: RwLock>, + is_file_cache: RwLock>, realpath_cache: FileSystemRealPathCache, } @@ -92,31 +92,39 @@ impl Cache { pub fn new(fs: FileSystemRef) -> Self { Self { fs, - packages: DashMap::with_hasher(DefaultHasher::default()), - tsconfigs: DashMap::with_hasher(DefaultHasher::default()), - is_file_cache: DashMap::with_hasher(DefaultHasher::default()), - is_dir_cache: DashMap::with_hasher(DefaultHasher::default()), + packages: RwLock::new(HashMap::with_hasher(DefaultHasher::default())), + tsconfigs: RwLock::new(HashMap::with_hasher(DefaultHasher::default())), + is_file_cache: RwLock::new(HashMap::with_hasher(DefaultHasher::default())), + is_dir_cache: RwLock::new(HashMap::with_hasher(DefaultHasher::default())), realpath_cache: FileSystemRealPathCache::default(), } } pub fn is_file(&self, path: &Path) -> bool { - if let Some(is_file) = self.is_file_cache.get(path) { + if let Some(is_file) = self.is_file_cache.read().unwrap().get(path) { return *is_file; } let is_file = self.fs.is_file(path); - self.is_file_cache.insert(path.to_path_buf(), is_file); + self + .is_file_cache + .write() + .unwrap() + .insert(path.to_path_buf(), is_file); is_file } pub fn is_dir(&self, path: &Path) -> bool { - if let Some(is_file) = self.is_dir_cache.get(path) { + if let Some(is_file) = self.is_dir_cache.read().unwrap().get(path) { return *is_file; } let is_file = self.fs.is_dir(path); - self.is_dir_cache.insert(path.to_path_buf(), is_file); + self + .is_dir_cache + .write() + .unwrap() + .insert(path.to_path_buf(), is_file); is_file } @@ -125,9 +133,11 @@ impl Cache { } pub fn read_package(&self, path: Cow) -> Arc, ResolverError>> { - if let Some(pkg) = self.packages.get(path.as_ref()) { + let read = self.packages.read().unwrap(); + if let Some(pkg) = read.get(path.as_ref()) { return pkg.clone(); } + drop(read); fn read_package<'a>( fs: &'a FileSystemRef, @@ -170,7 +180,11 @@ impl Cache { // Since we have exclusive access to packages, let entry = Arc::new(package.map(|pkg| Arc::new(pkg))); - let _ = self.packages.insert(path.clone(), entry.clone()); + let _ = self + .packages + .write() + .unwrap() + .insert(path.clone(), entry.clone()); entry.clone() } @@ -180,9 +194,11 @@ impl Cache { path: &Path, process: F, ) -> Arc, ResolverError>> { - if let Some(tsconfig) = self.tsconfigs.get(path) { + let read = self.tsconfigs.read().unwrap(); + if let Some(tsconfig) = read.get(path) { return tsconfig.clone(); } + drop(read); fn read_tsconfig<'a, F: FnOnce(&mut TsConfigWrapper) -> Result<(), ResolverError>>( fs: &FileSystemRef, @@ -207,7 +223,11 @@ impl Cache { // after insert let tsconfig = read_tsconfig(&self.fs, path, process).map(|t| Arc::new(t)); let tsconfig = Arc::new(tsconfig); - let _ = self.tsconfigs.insert(PathBuf::from(path), tsconfig.clone()); + let _ = self + .tsconfigs + .write() + .unwrap() + .insert(PathBuf::from(path), tsconfig.clone()); tsconfig }