Skip to content

Commit

Permalink
Revert threadlocal cache
Browse files Browse the repository at this point in the history
  • Loading branch information
mattcompiles committed Nov 14, 2024
1 parent 52a1243 commit 1267972
Showing 1 changed file with 43 additions and 22 deletions.
65 changes: 43 additions & 22 deletions packages/utils/node-resolver-rs/src/cache.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
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 std::sync::{Arc, RwLock};

use atlaspack_core::types::File;
use atlaspack_filesystem::{FileSystemRealPathCache, FileSystemRef};
use atlaspack_shared_map::ThreadLocalHashMap;

use crate::package_json::PackageJson;
use crate::package_json::SourceField;
use crate::tsconfig::TsConfig;
use crate::tsconfig::TsConfigWrapper;
use crate::ResolverError;

type DefaultHasher = xxhash_rust::xxh3::Xxh3Builder;
type LockedHashMap<Key, Value> = RwLock<HashMap<Key, Value, DefaultHasher>>;

pub struct Cache {
pub fs: FileSystemRef,
/// These map paths to parsed config files. They aren't really 'static, but Rust doens't have a good
/// 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.
#[allow(clippy::type_complexity)]
packages: ThreadLocalHashMap<PathBuf, Arc<Result<Arc<PackageJson>, ResolverError>>>,
#[allow(clippy::type_complexity)]
tsconfigs: ThreadLocalHashMap<PathBuf, Arc<Result<Arc<TsConfigWrapper>, ResolverError>>>,
packages: LockedHashMap<PathBuf, Arc<Result<Arc<PackageJson>, ResolverError>>>,
tsconfigs: LockedHashMap<PathBuf, Arc<Result<Arc<TsConfigWrapper>, ResolverError>>>,
// 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: ThreadLocalHashMap<PathBuf, bool>,
is_file_cache: ThreadLocalHashMap<PathBuf, bool>,
is_dir_cache: LockedHashMap<PathBuf, bool>,
is_file_cache: LockedHashMap<PathBuf, bool>,
realpath_cache: FileSystemRealPathCache,
}

Expand Down Expand Up @@ -91,31 +92,39 @@ impl Cache {
pub fn new(fs: FileSystemRef) -> Self {
Self {
fs,
packages: ThreadLocalHashMap::new(),
tsconfigs: ThreadLocalHashMap::new(),
is_file_cache: ThreadLocalHashMap::new(),
is_dir_cache: ThreadLocalHashMap::new(),
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) {
return is_file;
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) {
return is_file;
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
}

Expand All @@ -124,9 +133,11 @@ impl Cache {
}

pub fn read_package(&self, path: Cow<Path>) -> Arc<Result<Arc<PackageJson>, 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,
Expand Down Expand Up @@ -169,7 +180,11 @@ impl Cache {

// Since we have exclusive access to packages,
let entry = Arc::new(package.map(Arc::new));
self.packages.insert(path.clone(), entry.clone());
let _ = self
.packages
.write()
.unwrap()
.insert(path.clone(), entry.clone());

entry.clone()
}
Expand All @@ -179,9 +194,11 @@ impl Cache {
path: &Path,
process: F,
) -> Arc<Result<Arc<TsConfigWrapper>, 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<F: FnOnce(&mut TsConfigWrapper) -> Result<(), ResolverError>>(
fs: &FileSystemRef,
Expand All @@ -206,7 +223,11 @@ impl Cache {
// after insert
let tsconfig = read_tsconfig(&self.fs, path, process).map(Arc::new);
let tsconfig = Arc::new(tsconfig);
self.tsconfigs.insert(PathBuf::from(path), tsconfig.clone());
let _ = self
.tsconfigs
.write()
.unwrap()
.insert(PathBuf::from(path), tsconfig.clone());

tsconfig
}
Expand Down

0 comments on commit 1267972

Please sign in to comment.