perf: reduce memory allocation while normalizing package path (#318)

Author: Boshen

src/cache.rs

@@ -14,8 +14,8 @@ use once_cell::sync::OnceCell as OnceLock;
 use rustc_hash::FxHasher;
 
 use crate::{
-    context::ResolveContext as Ctx, package_json::PackageJson, path::PathUtil, FileMetadata,
-    FileSystem, ResolveError, ResolveOptions, TsConfig,
+    context::ResolveContext as Ctx, package_json::PackageJson, FileMetadata, FileSystem,
+    ResolveError, ResolveOptions, TsConfig,
 };
 
 thread_local! {
@@ -136,39 +136,17 @@ impl CacheKey for CachedPath {
     }
 }
 
-pub struct CachedPathImpl {
-    hash: u64,
-    path: Box<Path>,
-    parent: Option<CachedPath>,
-    meta: OnceLock<Option<FileMetadata>>,
-    canonicalized: OnceLock<Option<PathBuf>>,
-    node_modules: OnceLock<Option<CachedPath>>,
-    package_json: OnceLock<Option<Arc<PackageJson>>>,
-}
-
-impl CachedPathImpl {
-    const fn new(hash: u64, path: Box<Path>, parent: Option<CachedPath>) -> Self {
-        Self {
-            hash,
-            path,
-            parent,
-            meta: OnceLock::new(),
-            canonicalized: OnceLock::new(),
-            node_modules: OnceLock::new(),
-            package_json: OnceLock::new(),
-        }
-    }
-
-    pub const fn path(&self) -> &Path {
-        &self.path
+impl CachedPath {
+    pub fn path(&self) -> &Path {
+        &self.0.path
     }
 
     pub fn to_path_buf(&self) -> PathBuf {
         self.path.to_path_buf()
     }
 
-    pub const fn parent(&self) -> Option<&CachedPath> {
-        self.parent.as_ref()
+    pub fn parent(&self) -> Option<&Self> {
+        self.0.parent.as_ref()
     }
 
     fn meta<Fs: FileSystem>(&self, fs: &Fs) -> Option<FileMetadata> {
@@ -195,30 +173,31 @@ impl CachedPathImpl {
         )
     }
 
-    pub fn realpath<Fs: FileSystem>(&self, fs: &Fs) -> io::Result<PathBuf> {
+    pub fn realpath<Fs: FileSystem>(&self, cache: &Cache<Fs>) -> io::Result<Self> {
         self.canonicalized
             .get_or_try_init(|| {
-                if fs.symlink_metadata(&self.path).is_ok_and(|m| m.is_symlink) {
-                    return fs.canonicalize(&self.path).map(Some);
+                if cache.fs.symlink_metadata(&self.path).is_ok_and(|m| m.is_symlink) {
+                    let canonicalized = cache.fs.canonicalize(&self.path)?;
+                    return Ok(Some(cache.value(&canonicalized)));
                 }
                 if let Some(parent) = self.parent() {
-                    let parent_path = parent.realpath(fs)?;
-                    return Ok(Some(
-                        parent_path.normalize_with(self.path.strip_prefix(&parent.path).unwrap()),
-                    ));
+                    let parent_path = parent.realpath(cache)?;
+                    let normalized = parent_path
+                        .normalize_with(self.path.strip_prefix(&parent.path).unwrap(), cache);
+                    return Ok(Some(normalized));
                 };
                 Ok(None)
             })
             .cloned()
-            .map(|r| r.unwrap_or_else(|| self.path.clone().to_path_buf()))
+            .map(|r| r.unwrap_or_else(|| self.clone()))
     }
 
     pub fn module_directory<Fs: FileSystem>(
         &self,
         module_name: &str,
         cache: &Cache<Fs>,
         ctx: &mut Ctx,
-    ) -> Option<CachedPath> {
+    ) -> Option<Self> {
         let cached_path = cache.value(&self.path.join(module_name));
         cached_path.is_dir(&cache.fs, ctx).then_some(cached_path)
     }
@@ -227,61 +206,31 @@ impl CachedPathImpl {
         &self,
         cache: &Cache<Fs>,
         ctx: &mut Ctx,
-    ) -> Option<CachedPath> {
+    ) -> Option<Self> {
         self.node_modules.get_or_init(|| self.module_directory("node_modules", cache, ctx)).clone()
     }
 
-    /// Find package.json of a path by traversing parent directories.
-    ///
-    /// # Errors
-    ///
-    /// * [ResolveError::JSON]
-    pub fn find_package_json<Fs: FileSystem>(
-        &self,
-        fs: &Fs,
-        options: &ResolveOptions,
-        ctx: &mut Ctx,
-    ) -> Result<Option<Arc<PackageJson>>, ResolveError> {
-        let mut cache_value = self;
-        // Go up directories when the querying path is not a directory
-        while !cache_value.is_dir(fs, ctx) {
-            if let Some(cv) = &cache_value.parent {
-                cache_value = cv.as_ref();
-            } else {
-                break;
-            }
-        }
-        let mut cache_value = Some(cache_value);
-        while let Some(cv) = cache_value {
-            if let Some(package_json) = cv.package_json(fs, options, ctx)? {
-                return Ok(Some(Arc::clone(&package_json)));
-            }
-            cache_value = cv.parent.as_deref();
-        }
-        Ok(None)
-    }
-
     /// Get package.json of the given path.
     ///
     /// # Errors
     ///
     /// * [ResolveError::JSON]
     pub fn package_json<Fs: FileSystem>(
         &self,
-        fs: &Fs,
         options: &ResolveOptions,
+        cache: &Cache<Fs>,
         ctx: &mut Ctx,
     ) -> Result<Option<Arc<PackageJson>>, ResolveError> {
         // Change to `std::sync::OnceLock::get_or_try_init` when it is stable.
         let result = self
             .package_json
             .get_or_try_init(|| {
                 let package_json_path = self.path.join("package.json");
-                let Ok(package_json_string) = fs.read_to_string(&package_json_path) else {
+                let Ok(package_json_string) = cache.fs.read_to_string(&package_json_path) else {
                     return Ok(None);
                 };
                 let real_path = if options.symlinks {
-                    self.realpath(fs)?.join("package.json")
+                    self.realpath(cache)?.path().join("package.json")
                 } else {
                     package_json_path.clone()
                 };
@@ -311,7 +260,37 @@ impl CachedPathImpl {
         result
     }
 
-    pub fn add_extension<Fs: FileSystem>(&self, ext: &str, cache: &Cache<Fs>) -> CachedPath {
+    /// Find package.json of a path by traversing parent directories.
+    ///
+    /// # Errors
+    ///
+    /// * [ResolveError::JSON]
+    pub fn find_package_json<Fs: FileSystem>(
+        &self,
+        options: &ResolveOptions,
+        cache: &Cache<Fs>,
+        ctx: &mut Ctx,
+    ) -> Result<Option<Arc<PackageJson>>, ResolveError> {
+        let mut cache_value = self;
+        // Go up directories when the querying path is not a directory
+        while !cache_value.is_dir(&cache.fs, ctx) {
+            if let Some(cv) = &cache_value.parent {
+                cache_value = cv;
+            } else {
+                break;
+            }
+        }
+        let mut cache_value = Some(cache_value);
+        while let Some(cv) = cache_value {
+            if let Some(package_json) = cv.package_json(options, cache, ctx)? {
+                return Ok(Some(Arc::clone(&package_json)));
+            }
+            cache_value = cv.parent.as_ref();
+        }
+        Ok(None)
+    }
+
+    pub fn add_extension<Fs: FileSystem>(&self, ext: &str, cache: &Cache<Fs>) -> Self {
         SCRATCH_PATH.with(|path| {
             // SAFETY: ???
             let path = unsafe { &mut *path.get() };
@@ -323,7 +302,7 @@ impl CachedPathImpl {
         })
     }
 
-    pub fn replace_extension<Fs: FileSystem>(&self, ext: &str, cache: &Cache<Fs>) -> CachedPath {
+    pub fn replace_extension<Fs: FileSystem>(&self, ext: &str, cache: &Cache<Fs>) -> Self {
         SCRATCH_PATH.with(|path| {
             // SAFETY: ???
             let path = unsafe { &mut *path.get() };
@@ -342,7 +321,7 @@ impl CachedPathImpl {
     }
 
     /// 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
+    pub fn normalize_with<P, Fs>(&self, subpath: P, cache: &Cache<Fs>) -> Self
     where
         P: AsRef<Path>,
         Fs: FileSystem,
@@ -378,6 +357,30 @@ impl CachedPathImpl {
     }
 }
 
+pub struct CachedPathImpl {
+    hash: u64,
+    path: Box<Path>,
+    parent: Option<CachedPath>,
+    meta: OnceLock<Option<FileMetadata>>,
+    canonicalized: OnceLock<Option<CachedPath>>,
+    node_modules: OnceLock<Option<CachedPath>>,
+    package_json: OnceLock<Option<Arc<PackageJson>>>,
+}
+
+impl CachedPathImpl {
+    const fn new(hash: u64, path: Box<Path>, parent: Option<CachedPath>) -> Self {
+        Self {
+            hash,
+            path,
+            parent,
+            meta: OnceLock::new(),
+            canonicalized: OnceLock::new(),
+            node_modules: OnceLock::new(),
+            package_json: OnceLock::new(),
+        }
+    }
+}
+
 /// Memoized cache key, code adapted from <https://stackoverflow.com/a/50478038>.
 trait CacheKey {
     fn tuple(&self) -> (u64, &Path);

src/lib.rs

@@ -249,7 +249,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         let path = self.load_realpath(&cached_path)?;
         // enhanced-resolve: restrictions
         self.check_restrictions(&path)?;
-        let package_json = cached_path.find_package_json(&self.cache.fs, &self.options, ctx)?;
+        let package_json = cached_path.find_package_json(&self.options, &self.cache, ctx)?;
         if let Some(package_json) = &package_json {
             // path must be inside the package.
             debug_assert!(path.starts_with(package_json.directory()));
@@ -499,8 +499,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
     ) -> ResolveResult {
         // 1. Find the closest package scope SCOPE to DIR.
         // 2. If no scope was found, return.
-        let Some(package_json) =
-            cached_path.find_package_json(&self.cache.fs, &self.options, ctx)?
+        let Some(package_json) = cached_path.find_package_json(&self.options, &self.cache, ctx)?
         else {
             return Ok(None);
         };
@@ -539,9 +538,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         // 1. If X/package.json is a file,
         if !self.options.description_files.is_empty() {
             // a. Parse X/package.json, and look for "main" field.
-            if let Some(package_json) =
-                cached_path.package_json(&self.cache.fs, &self.options, ctx)?
-            {
+            if let Some(package_json) = cached_path.package_json(&self.options, &self.cache, ctx)? {
                 // 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)
@@ -605,7 +602,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
 
     fn load_realpath(&self, cached_path: &CachedPath) -> Result<PathBuf, ResolveError> {
         if self.options.symlinks {
-            cached_path.realpath(&self.cache.fs).map_err(ResolveError::from)
+            cached_path.realpath(&self.cache).map(|c| c.to_path_buf()).map_err(ResolveError::from)
         } else {
             Ok(cached_path.to_path_buf())
         }
@@ -661,7 +658,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
     fn load_alias_or_file(&self, cached_path: &CachedPath, ctx: &mut Ctx) -> ResolveResult {
         if !self.options.alias_fields.is_empty() {
             if let Some(package_json) =
-                cached_path.find_package_json(&self.cache.fs, &self.options, ctx)?
+                cached_path.find_package_json(&self.options, &self.cache, ctx)?
             {
                 if let Some(path) =
                     self.load_browser_field(cached_path, None, &package_json, ctx)?
@@ -818,8 +815,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
     ) -> ResolveResult {
         // 2. If X does not match this pattern or DIR/NAME/package.json is not a file,
         //    return.
-        let Some(package_json) = cached_path.package_json(&self.cache.fs, &self.options, ctx)?
-        else {
+        let Some(package_json) = cached_path.package_json(&self.options, &self.cache, ctx)? else {
             return Ok(None);
         };
         // 3. Parse DIR/NAME/package.json, and look for "exports" field.
@@ -846,8 +842,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
     ) -> ResolveResult {
         // 1. Find the closest package scope SCOPE to DIR.
         // 2. If no scope was found, return.
-        let Some(package_json) =
-            cached_path.find_package_json(&self.cache.fs, &self.options, ctx)?
+        let Some(package_json) = cached_path.find_package_json(&self.options, &self.cache, ctx)?
         else {
             return Ok(None);
         };
@@ -1264,7 +1259,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                 if cached_path.is_dir(&self.cache.fs, ctx) {
                     // 4. Let pjson be the result of READ_PACKAGE_JSON(packageURL).
                     if let Some(package_json) =
-                        cached_path.package_json(&self.cache.fs, &self.options, ctx)?
+                        cached_path.package_json(&self.options, &self.cache, ctx)?
                     {
                         // 5. If pjson is not null and pjson.exports is not null or undefined, then
                         // 1. Return the result of PACKAGE_EXPORTS_RESOLVE(packageURL, packageSubpath, pjson.exports, defaultConditions).