Skip to content

Commit

Permalink
perf: reduce memory allocation by using a thread_local path for path …
Browse files Browse the repository at this point in the history 
…methods
  • Loading branch information
@Boshen
Boshen committed Nov 22, 2024
1 parent 8ab444b commit 69512db
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 29 deletions.
75 changes: 74 additions & 1 deletion src/cache.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::{
borrow::{Borrow, Cow},
cell::UnsafeCell,
convert::AsRef,
hash::{BuildHasherDefault, Hash, Hasher},
io,
ops::Deref,
path::{Path, PathBuf},
path::{Component, Path, PathBuf},
sync::Arc,
};

Expand All @@ -17,6 +18,12 @@ use crate::{
FileSystem, ResolveError, ResolveOptions, TsConfig,
};

thread_local! {
/// Per-thread pre-allocated path that is used to perform operations on paths more quickly.
/// Learned from parcel <https://github.com/parcel-bundler/parcel/blob/a53f8f3ba1025c7ea8653e9719e0a61ef9717079/crates/parcel-resolver/src/cache.rs#L394>
pub static SCRATCH_PATH: UnsafeCell<PathBuf> = UnsafeCell::new(PathBuf::with_capacity(256));
}

#[derive(Default)]
pub struct Cache<Fs> {
pub(crate) fs: Fs,
Expand Down Expand Up @@ -303,6 +310,72 @@ impl CachedPathImpl {
}
result
}

pub fn add_extension<Fs: FileSystem>(&self, ext: &str, cache: &Cache<Fs>) -> CachedPath {
SCRATCH_PATH.with(|path| {
// SAFETY: ???
let path = unsafe { &mut *path.get() };
path.clear();
let s = path.as_mut_os_string();
s.push(self.path.as_os_str());
s.push(ext);
cache.value(path)
})
}

pub fn replace_extension<Fs: FileSystem>(&self, ext: &str, cache: &Cache<Fs>) -> CachedPath {
SCRATCH_PATH.with(|path| {
// SAFETY: ???
let path = unsafe { &mut *path.get() };
path.clear();
let s = path.as_mut_os_string();
let self_len = self.path.as_os_str().len();
let self_bytes = self.path.as_os_str().as_encoded_bytes();
let slice_to_copy = self.path.extension().map_or(self_bytes, |previous_extension| {
&self_bytes[..self_len - previous_extension.len() - 1]
});
// SAFETY: ???
s.push(unsafe { std::ffi::OsStr::from_encoded_bytes_unchecked(slice_to_copy) });
s.push(ext);
cache.value(path)
})
}

/// Returns a new path by resolving the given subpath (including "." and ".." components) with this path.
pub fn normalize_with<P, Fs>(&self, subpath: P, cache: &Cache<Fs>) -> CachedPath
where
P: AsRef<Path>,
Fs: FileSystem,
{
let subpath = subpath.as_ref();
let mut components = subpath.components();
let Some(head) = components.next() else { return cache.value(subpath) };
if matches!(head, Component::Prefix(..) | Component::RootDir) {
return cache.value(subpath);
}
SCRATCH_PATH.with(|path| {
// SAFETY: ???
let path = unsafe { &mut *path.get() };
path.clear();
path.push(&self.path);
for component in std::iter::once(head).chain(components) {
match component {
Component::CurDir => {}
Component::ParentDir => {
path.pop();
}
Component::Normal(c) => {
path.push(c);
}
Component::Prefix(..) | Component::RootDir => {
unreachable!("Path {:?} Subpath {:?}", self.path, subpath)

Check warning on line 371 in src/cache.rs

View check run for this annotation

Codecov / codecov/patch

src/cache.rs#L371 

Added line #L371 was not covered by tests
}
}
}

cache.value(path)
})
}
}

/// Memoized cache key, code adapted from <https://stackoverflow.com/a/50478038>.
Expand Down
42 changes: 14 additions & 28 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
c,
Component::CurDir | Component::ParentDir | Component::Normal(_)
)));
let path = cached_path.path().normalize_with(specifier);
let cached_path = self.cache.value(&path);
let cached_path = cached_path.normalize_with(specifier, &self.cache);
// a. LOAD_AS_FILE(Y + X)
// b. LOAD_AS_DIRECTORY(Y + X)
if let Some(path) = self.load_as_file_or_directory(&cached_path, specifier, ctx)? {
Expand Down Expand Up @@ -546,9 +545,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
// b. If "main" is a falsy value, GOTO 2.
for main_field in package_json.main_fields(&self.options.main_fields) {
// c. let M = X + (json main field)
let main_field_path = cached_path.path().normalize_with(main_field);
let cached_path = cached_path.normalize_with(main_field, &self.cache);
// d. LOAD_AS_FILE(M)
let cached_path = self.cache.value(&main_field_path);
if let Some(path) = self.load_as_file(&cached_path, ctx)? {
return Ok(Some(path));
}
Expand Down Expand Up @@ -596,12 +594,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if ctx.fully_specified {
return Ok(None);
}
let path = path.path().as_os_str();
for extension in extensions {
let mut path_with_extension = path.to_os_string();
path_with_extension.reserve_exact(extension.len());
path_with_extension.push(extension);
let cached_path = self.cache.value(Path::new(&path_with_extension));
let cached_path = path.add_extension(extension, &self.cache);
if let Some(path) = self.load_alias_or_file(&cached_path, ctx)? {
return Ok(Some(path));
}
Expand Down Expand Up @@ -648,8 +642,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {

fn load_index(&self, cached_path: &CachedPath, ctx: &mut Ctx) -> ResolveResult {
for main_file in &self.options.main_files {
let main_path = cached_path.path().normalize_with(main_file);
let cached_path = self.cache.value(&main_path);
let cached_path = cached_path.normalize_with(main_file, &self.cache);
if self.options.enforce_extension.is_disabled() {
if let Some(path) = self.load_alias_or_file(&cached_path, ctx)? {
return Ok(Some(path));
Expand Down Expand Up @@ -722,8 +715,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
// 1. Try to interpret X as a combination of NAME and SUBPATH where the name
// may have a @scope/ prefix and the subpath begins with a slash (`/`).
if !package_name.is_empty() {
let package_path = cached_path.path().normalize_with(package_name);
let cached_path = self.cache.value(&package_path);
let cached_path = cached_path.normalize_with(package_name, &self.cache);
// Try foo/node_modules/package_name
if cached_path.is_dir(&self.cache.fs, ctx) {
// a. LOAD_PACKAGE_EXPORTS(X, DIR)
Expand Down Expand Up @@ -752,8 +744,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
// Try as file or directory for all other cases
// b. LOAD_AS_FILE(DIR/X)
// c. LOAD_AS_DIRECTORY(DIR/X)
let node_module_file = cached_path.path().normalize_with(specifier);
let cached_path = self.cache.value(&node_module_file);
let cached_path = cached_path.normalize_with(specifier, &self.cache);
if let Some(path) = self.load_as_file_or_directory(&cached_path, specifier, ctx)? {
return Ok(Some(path));
}
Expand Down Expand Up @@ -984,8 +975,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
}
}
AliasValue::Ignore => {
let path = cached_path.path().normalize_with(alias_key);
return Err(ResolveError::Ignored(path));
let cached_path = cached_path.normalize_with(alias_key, &self.cache);
return Err(ResolveError::Ignored(cached_path.to_path_buf()));
}
}
}
Expand Down Expand Up @@ -1025,8 +1016,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {

// Remove the leading slash so the final path is concatenated.
let tail = tail.trim_start_matches(SLASH_START);
let normalized = alias_value.normalize_with(tail);
Cow::Owned(normalized.to_string_lossy().to_string())
let normalized = alias_value_cached_path.normalize_with(tail, &self.cache);
Cow::Owned(normalized.path().to_string_lossy().to_string())
};

*should_stop = true;
Expand Down Expand Up @@ -1067,13 +1058,9 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
};
let path = cached_path.path();
let Some(filename) = path.file_name() else { return Ok(None) };
let path_without_extension = path.with_extension("");
ctx.with_fully_specified(true);
for extension in extensions {
let mut path_with_extension = path_without_extension.clone().into_os_string();
path_with_extension.reserve_exact(extension.len());
path_with_extension.push(extension);
let cached_path = self.cache.value(Path::new(&path_with_extension));
let cached_path = cached_path.replace_extension(extension, &self.cache);
if let Some(path) = self.load_alias_or_file(&cached_path, ctx)? {
ctx.with_fully_specified(false);
return Ok(Some(path));
Expand Down Expand Up @@ -1271,8 +1258,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
continue;
};
// 2. Set parentURL to the parent folder URL of parentURL.
let package_path = cached_path.path().normalize_with(package_name);
let cached_path = self.cache.value(&package_path);
let cached_path = cached_path.normalize_with(package_name, &self.cache);
// 3. If the folder at packageURL does not exist, then
// 1. Continue the next loop iteration.
if cached_path.is_dir(&self.cache.fs, ctx) {
Expand All @@ -1297,8 +1283,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
// 1. If pjson.main is a string, then
for main_field in package_json.main_fields(&self.options.main_fields) {
// 1. Return the URL resolution of main in packageURL.
let path = cached_path.path().normalize_with(main_field);
let cached_path = self.cache.value(&path);
let cached_path =
cached_path.normalize_with(main_field, &self.cache);

Check warning on line 1287 in src/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/lib.rs#L1286-L1287 

Added lines #L1286 - L1287 were not covered by tests
if cached_path.is_file(&self.cache.fs, ctx) {
return Ok(Some(cached_path));
}
Expand Down

0 comments on commit 69512db

Please # to comment.