Skip to content

perf: eliminate symlink stat syscalls by reusing canonicalization #1184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
May 29, 2026
Merged

perf: eliminate symlink stat syscalls by reusing canonicalization #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/alias.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
let alias_path = Path::new(alias_value).normalize();
// Must not append anything to alias_value if it is a file.
let cached_alias_path = self.cache.value(&alias_path);
if self.cache.is_file(&cached_alias_path, ctx) {
if self.is_file(&cached_alias_path, ctx) {
return Ok(None);
}
// Remove the leading slash so the final path is concatenated.
Expand Down
45 changes: 37 additions & 8 deletions src/cache/cache_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use super::borrowed_path::BorrowedCachedPath;
use super::cached_path::{CachedPath, CachedPathImpl};
use super::hasher::IdentityHasher;
use crate::{
FileSystem, PackageJson, ResolveError, ResolveOptions, TsConfig,
FileMetadata, FileSystem, PackageJson, ResolveError, ResolveOptions, TsConfig,
context::ResolveContext as Ctx, path::PathUtil,
};

Expand Down Expand Up @@ -82,8 +82,8 @@ impl<Fs: FileSystem> Cache<Fs> {
}
}

pub(crate) fn is_file(&self, path: &CachedPath, ctx: &mut Ctx) -> bool {
if path.is_file(&self.fs).is_some_and(|b| b) {
pub(crate) fn is_file(&self, path: &CachedPath, symlinks: bool, ctx: &mut Ctx) -> bool {
if self.followed_metadata(path, symlinks).is_some_and(FileMetadata::is_file) {
ctx.add_file_dependency(path.path());
true
} else {
Expand All @@ -92,10 +92,39 @@ impl<Fs: FileSystem> Cache<Fs> {
}
}

pub(crate) fn is_dir(&self, path: &CachedPath, ctx: &mut Ctx) -> bool {
path.is_dir(&self.fs).unwrap_or_else(|| {
ctx.add_missing_dependency(path.path());
false
pub(crate) fn is_dir(&self, path: &CachedPath, symlinks: bool, ctx: &mut Ctx) -> bool {
self.followed_metadata(path, symlinks).map_or_else(
|| {
ctx.add_missing_dependency(path.path());
false
},
FileMetadata::is_dir,
)
}

/// `stat`-equivalent metadata (symlinks followed) for `path`, cached in the `followed` slot.
///
/// For a non-symlink the cached `lstat` already answers this, so no extra syscall is issued.
/// For a symlink with `symlinks` enabled, reuse canonicalization — which the resolver performs
/// anyway for the final resolved path — and read the canonical target's already-cached `lstat`,
/// avoiding a standalone `stat` of the symlink.
///
/// Falls back to a direct `stat` when symlinks are disabled, when canonicalization fails, or
/// when the canonical target has no metadata. The last case keeps the optimization purely
/// additive: a custom [`FileSystem`] whose `canonicalize` and `metadata` disagree still gets
/// the same answer `stat` gave before.
fn followed_metadata(&self, path: &CachedPath, symlinks: bool) -> Option<FileMetadata> {
path.meta.followed_or_init(|| match path.link_metadata(&self.fs) {
Some(meta) if meta.is_symlink() => {
let followed = if symlinks {
self.canonicalize_impl(path).ok().and_then(|c| c.link_metadata(&self.fs))
} else {
None
};
followed.or_else(|| self.fs.metadata(path.path()).ok())
}
// A non-symlink's `lstat` already is its `stat`; `None` stays `None`.
other => other,
})
}

Expand Down Expand Up @@ -133,7 +162,7 @@ impl<Fs: FileSystem> Cache<Fs> {
) -> Result<Option<Arc<PackageJson>>, ResolveError> {
let mut path = path.clone();
// Go up directories when the querying path is not a directory
while !self.is_dir(&path, ctx) {
while !self.is_dir(&path, options.symlinks, ctx) {
if let Some(cv) = path.parent(self) {
path = cv;
} else {
Expand Down
27 changes: 5 additions & 22 deletions src/cache/cached_path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,24 @@ impl CachedPath {
pub(crate) fn module_directory<Fs: FileSystem>(
&self,
module_name: &str,
symlinks: bool,
cache: &Cache<Fs>,
ctx: &mut Ctx,
) -> Option<Self> {
let cached_path = self.push(module_name, cache);
cache.is_dir(&cached_path, ctx).then_some(cached_path)
cache.is_dir(&cached_path, symlinks, ctx).then_some(cached_path)
}

pub(crate) fn cached_node_modules<Fs: FileSystem>(
&self,
symlinks: bool,
cache: &Cache<Fs>,
ctx: &mut Ctx,
) -> Option<Self> {
self.node_modules
.get_or_init(|| {
self.module_directory("node_modules", cache, ctx).map(|cp| Arc::downgrade(&cp.0))
self.module_directory("node_modules", symlinks, cache, ctx)
.map(|cp| Arc::downgrade(&cp.0))
})
.as_ref()
.and_then(|weak| {
Expand Down Expand Up @@ -246,26 +249,6 @@ impl CachedPath {
pub(crate) fn link_metadata<Fs: FileSystem>(&self, fs: &Fs) -> Option<FileMetadata> {
self.meta.link_or_init(|| fs.symlink_metadata(&self.path).ok())
}

/// `stat` view of this path (symlinks followed), cached.
///
/// For a non-symlink this reuses the cached `lstat` result and issues no extra syscall; only
/// an actual symlink needs a follow-up `stat` to learn what it points at.
fn followed_metadata<Fs: FileSystem>(&self, fs: &Fs) -> Option<FileMetadata> {
self.meta.followed_or_init(|| match self.link_metadata(fs) {
Some(meta) if meta.is_symlink() => fs.metadata(&self.path).ok(),
// A non-symlink's `lstat` already is its `stat`; `None` stays `None`.
other => other,
})
}

pub(crate) fn is_file<Fs: FileSystem>(&self, fs: &Fs) -> Option<bool> {
self.followed_metadata(fs).map(FileMetadata::is_file)
}

pub(crate) fn is_dir<Fs: FileSystem>(&self, fs: &Fs) -> Option<bool> {
self.followed_metadata(fs).map(FileMetadata::is_dir)
}
}

impl Hash for CachedPath {
Expand Down
20 changes: 10 additions & 10 deletions src/dts_resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {

fn dts_try_file(&self, base: &CachedPath, ext: &str, ctx: &mut Ctx) -> Option<CachedPath> {
let candidate = base.add_extension(ext, &self.cache);
if self.cache.is_file(&candidate, ctx) { Some(candidate) } else { None }
if self.is_file(&candidate, ctx) { Some(candidate) } else { None }
}

/// TS: `loadNodeModuleFromDirectoryWorker`
Expand All @@ -376,7 +376,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
candidate: &CachedPath,
ctx: &mut Ctx,
) -> ResolveResult {
if !self.cache.is_dir(candidate, ctx) {
if !self.is_dir(candidate, ctx) {
return Ok(None);
}

Expand Down Expand Up @@ -444,7 +444,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if let Some(path) = self.dts_resolve_as_file(expanded, &entry_path, ctx) {
return Ok(Some(path));
}
if self.cache.is_dir(&entry_path, ctx) {
if self.is_dir(&entry_path, ctx) {
let index = entry_path.push("index", &self.cache);
if let Some(path) = self.dts_resolve_as_file(expanded, &index, ctx) {
return Ok(Some(path));
Expand Down Expand Up @@ -479,7 +479,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
std::iter::successors(Some(directory.clone()), |cp| cp.parent(&self.cache))
{
let nm = ancestor.push("node_modules", &self.cache);
if !self.cache.is_dir(&nm, ctx) {
if !self.is_dir(&nm, ctx) {
continue;
}

Expand All @@ -494,7 +494,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if priority_exts.contains(Extensions::DECLARATION) {
let mangled = Self::dts_mangle_scoped_name(package_name);
let at_types_dir = nm.push("@types", &self.cache);
if self.cache.is_dir(&at_types_dir, ctx) {
if self.is_dir(&at_types_dir, ctx) {
let at_types_specifier = if rest.is_empty() {
mangled.clone()
} else {
Expand All @@ -519,7 +519,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
std::iter::successors(Some(directory.clone()), |cp| cp.parent(&self.cache))
{
let nm = ancestor.push("node_modules", &self.cache);
if !self.cache.is_dir(&nm, ctx) {
if !self.is_dir(&nm, ctx) {
continue;
}
if let Some(path) =
Expand All @@ -543,7 +543,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
let (package_name, rest) = Self::parse_package_specifier(specifier);
let pkg_dir = nm_dir.normalize_with(package_name, &self.cache);

if !self.cache.is_dir(&pkg_dir, ctx) {
if !self.is_dir(&pkg_dir, ctx) {
return Ok(None);
}

Expand Down Expand Up @@ -593,7 +593,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if let Some(path) = self.dts_resolve_as_file(extensions, &candidate, ctx) {
return Ok(Some(path));
}
if self.cache.is_dir(&candidate, ctx) {
if self.is_dir(&candidate, ctx) {
return self.dts_resolve_as_directory(extensions, &candidate, ctx);
}
}
Expand All @@ -608,7 +608,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
return Some(path);
}
// Fall back to original file if it exists
if self.cache.is_file(cached_path, ctx) {
if self.is_file(cached_path, ctx) {
return Some(cached_path.clone());
}
None
Expand Down Expand Up @@ -669,7 +669,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if let Some(path) = self.dts_resolve_as_file(extensions, &candidate, ctx) {
return Ok(Some(path));
}
if self.cache.is_dir(&candidate, ctx)
if self.is_dir(&candidate, ctx)
&& let Some(path) =
self.dts_resolve_as_directory(extensions, &candidate, ctx)?
{
Expand Down
40 changes: 25 additions & 15 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
let mut last = None;
// Go up directories when the querying path is not a directory
let mut cp = cached_path.clone();
if !self.cache.is_dir(&cp, ctx)
if !self.is_dir(&cp, ctx)
&& let Some(cv) = cp.parent(&self.cache)
{
cp = cv;
Expand Down Expand Up @@ -378,6 +378,16 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
}
}

/// [`Cache::is_file`] using this resolver's [`ResolveOptions::symlinks`] policy.
fn is_file(&self, path: &CachedPath, ctx: &mut Ctx) -> bool {
self.cache.is_file(path, self.options.symlinks, ctx)
}

/// [`Cache::is_dir`] using this resolver's [`ResolveOptions::symlinks`] policy.
fn is_dir(&self, path: &CachedPath, ctx: &mut Ctx) -> bool {
self.cache.is_dir(path, self.options.symlinks, ctx)
}

/// require(X) from module at path Y
///
/// X: specifier
Expand Down Expand Up @@ -789,14 +799,14 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
ctx: &mut Ctx,
) -> ResolveResult {
if self.options.resolve_to_context {
return Ok(self.cache.is_dir(cached_path, ctx).then(|| cached_path.clone()));
return Ok(self.is_dir(cached_path, ctx).then(|| cached_path.clone()));
}
if !specifier.ends_with('/')
&& let Some(path) = self.load_as_file(cached_path, tsconfig, ctx)?
{
return Ok(Some(path));
}
if self.cache.is_dir(cached_path, ctx)
if self.is_dir(cached_path, ctx)
&& let Some(path) = self.load_as_directory(cached_path, tsconfig, ctx)?
{
return Ok(Some(path));
Expand Down Expand Up @@ -903,7 +913,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if let Some(path) = self.load_browser_field_or_alias(cached_path, tsconfig, ctx)? {
return Ok(Some(path));
}
if self.cache.is_file(cached_path, ctx) && self.check_restrictions(cached_path.path()) {
if self.is_file(cached_path, ctx) && self.check_restrictions(cached_path.path()) {
return Ok(Some(cached_path.clone()));
}
Ok(None)
Expand Down Expand Up @@ -932,7 +942,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
std::iter::successors(Some(cached_path.clone()), |cp| cp.parent(&self.cache))
{
// Skip if /path/to/node_modules does not exist
if !self.cache.is_dir(&cached_path, ctx) {
if !self.is_dir(&cached_path, ctx) {
continue;
}

Expand All @@ -947,7 +957,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if !package_name.is_empty() {
let cached_path = cached_path.normalize_with(package_name, &self.cache);
// Try foo/node_modules/package_name
if self.cache.is_dir(&cached_path, ctx) {
if self.is_dir(&cached_path, ctx) {
// a. LOAD_PACKAGE_EXPORTS(X, DIR)
if let Some(path) = self.load_package_exports(
specifier,
Expand All @@ -967,7 +977,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
// i.e. `foo/node_modules/@scope` is not a directory for `foo/node_modules/@scope/package`
if package_name.starts_with('@')
&& let Some(path) = cached_path.parent(&self.cache).as_ref()
&& !self.cache.is_dir(path, ctx)
&& !self.is_dir(path, ctx)
{
continue;
}
Expand All @@ -981,7 +991,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
let cached_path = cached_path.normalize_with(specifier, &self.cache);

if self.options.resolve_to_context {
return Ok(self.cache.is_dir(&cached_path, ctx).then(|| cached_path.clone()));
return Ok(self.is_dir(&cached_path, ctx).then(|| cached_path.clone()));
}

// Only load the file if it is targeting a `X/sub/dir`.
Expand All @@ -993,7 +1003,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
}
// Otherwise just load the directory.
// No modern package manager creates `node_modules/X.js`.
if self.cache.is_dir(&cached_path, ctx) {
if self.is_dir(&cached_path, ctx) {
if let Some(path) =
self.load_browser_field_or_alias(&cached_path, tsconfig, ctx)?
{
Expand Down Expand Up @@ -1106,13 +1116,13 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
ctx: &mut Ctx,
) -> Option<CachedPath> {
if module_name == "node_modules" {
cached_path.cached_node_modules(&self.cache, ctx)
cached_path.cached_node_modules(self.options.symlinks, &self.cache, ctx)
} else if cached_path.path().components().next_back()
== Some(Component::Normal(OsStr::new(module_name)))
{
Some(cached_path.clone())
} else {
cached_path.module_directory(module_name, &self.cache, ctx)
cached_path.module_directory(module_name, self.options.symlinks, &self.cache, ctx)
}
}

Expand Down Expand Up @@ -1241,7 +1251,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
.as_ref()
.is_some_and(|s| path.ends_with(Path::new(s)))
{
return if self.cache.is_file(cached_path, ctx) {
return if self.is_file(cached_path, ctx) {
if self.check_restrictions(cached_path.path()) {
Ok(Some(cached_path.clone()))
} else {
Expand Down Expand Up @@ -1298,7 +1308,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
}
}
// Bail if path is module directory such as `ipaddr.js`
if !self.cache.is_file(cached_path, ctx) {
if !self.is_file(cached_path, ctx) {
ctx.with_fully_specified(false);
return Ok(None);
} else if !self.check_restrictions(cached_path.path()) {
Expand Down Expand Up @@ -1381,7 +1391,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
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 self.cache.is_dir(&cached_path, ctx) {
if self.is_dir(&cached_path, ctx) {
// 4. Let pjson be the result of READ_PACKAGE_JSON(packageURL).
if let Some(package_json) =
self.cache.get_package_json(&cached_path, &self.options, ctx)?
Expand All @@ -1406,7 +1416,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
// 1. Return the URL resolution of main in packageURL.
let cached_path =
cached_path.normalize_with(main_field, &self.cache);
if self.cache.is_file(&cached_path, ctx)
if self.is_file(&cached_path, ctx)
&& self.check_restrictions(cached_path.path())
{
return Ok(Some(cached_path));
Expand Down
4 changes: 2 additions & 2 deletions src/tsconfig_resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
if let Some(tsconfig) = cv.tsconfig.get_or_try_init(|| {
let tsconfig_path = cv.path.join("tsconfig.json");
let tsconfig_path = self.cache.value(&tsconfig_path);
if self.cache.is_file(&tsconfig_path, &mut ctx) {
if self.is_file(&tsconfig_path, &mut ctx) {
match self.resolve_tsconfig(tsconfig_path.path()) {
Ok(tsconfig) => Ok(Some(tsconfig)),
// Skip unreadable tsconfig files (e.g. permission denied)
Expand Down Expand Up @@ -354,7 +354,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
let Some(root_dirs) = &tsconfig.compiler_options.root_dirs else { return Ok(None) };

// Use the containing directory, not the file itself
let containing_directory = if self.cache.is_dir(cached_path, ctx) {
let containing_directory = if self.is_dir(cached_path, ctx) {
cached_path.clone()
} else {
cached_path.parent(&self.cache).unwrap_or_else(|| cached_path.clone())
Expand Down
Loading