[codex] Prehash resolver path dependencies

src/cache.rs

@@ -16,6 +16,7 @@
 
 use crate::{
   context::ResolveContext as Ctx,
+  hashing::{hash_path, IdentityHasher},
   package_json::{off_to_location, PackageJson},
   path::PathUtil,
   FileMetadata, FileSystem, JSONError, ResolveError, ResolveOptions, TsConfig,
@@ -43,11 +44,7 @@
   }
 
   pub fn value(&self, path: &Path) -> CachedPath {
-    let hash = {
-      let mut hasher = FxHasher::default();
-      path.hash(&mut hasher);
-      hasher.finish()
-    };
+    let hash = hash_path(path);
     if let Some(cache_entry) = self.paths.get((hash, path).borrow() as &dyn CacheKey) {
       return cache_entry.clone();
     }
@@ -116,8 +113,9 @@
 }
 
 impl PartialEq for CachedPath {
+  #[inline]
   fn eq(&self, other: &Self) -> bool {
-    self.0.path == other.0.path
+    self.0.hash == other.0.hash && (Arc::ptr_eq(&self.0, &other.0) || self.0.path == other.0.path)
   }
 }
 impl Eq for CachedPath {}
@@ -156,6 +154,7 @@
   canonicalized: OnceLock<Option<PathBuf>>,
   node_modules: OnceLock<Option<CachedPath>>,
   package_json: OnceLock<Option<Arc<PackageJson>>>,
+  package_json_path: OnceLock<PathBuf>,
 }
 
 impl CachedPathImpl {
@@ -168,6 +167,7 @@
       canonicalized: OnceLock::new(),
       node_modules: OnceLock::new(),
       package_json: OnceLock::new(),
+      package_json_path: OnceLock::new(),
     }
   }
 
@@ -192,18 +192,18 @@
 
   pub async fn is_file<Fs: Send + Sync + FileSystem>(&self, fs: &Fs, ctx: &mut Ctx) -> bool {
     if let Some(meta) = self.meta(fs).await {
-      ctx.add_file_dependency(self.path());
+      ctx.add_file_dependency_with_hash(self.to_path_buf(), self.hash);
       meta.is_file
     } else {
-      ctx.add_missing_dependency(self.path());
+      ctx.add_missing_dependency_with_hash(self.to_path_buf(), self.hash);
       false
     }
   }
 
   pub async fn is_dir<Fs: Send + Sync + FileSystem>(&self, fs: &Fs, ctx: &mut Ctx) -> bool {
     self.meta(fs).await.map_or_else(
       || {
-        ctx.add_missing_dependency(self.path());
+        ctx.add_missing_dependency_with_hash(self.to_path_buf(), self.hash);
         false
       },
       |meta| meta.is_dir,
@@ -308,12 +308,15 @@
     options: &ResolveOptions,
     ctx: &mut Ctx,
   ) -> Result<Option<Arc<PackageJson>>, ResolveError> {
+    let package_json_path = self
+      .package_json_path
+      .get_or_init(|| async { self.path.join("package.json") })
+      .await;
     // Change to `std::sync::OnceLock::get_or_try_init` when it is stable.
     let result = self
       .package_json
       .get_or_try_init(|| async {
-        let package_json_path = self.path.join("package.json");
         let Ok(package_json_string) = fs.read(&package_json_path).await else {
           return Ok(None);
         };
         let real_path = if options.symlinks {
@@ -324,8 +327,7 @@
         match PackageJson::parse(package_json_path.clone(), real_path, package_json_string) {
           Ok(v) => Ok(Some(Arc::new(v))),
           Err(parse_err) => {
-            let package_json_path = self.path.join("package.json");
             let package_json_string = match fs.read_to_string(&package_json_path).await {
               Ok(c) => c,
               Err(io_err) => {
                 return Err(ResolveError::from(io_err));
@@ -335,15 +337,15 @@
 
             if let Some(err) = serde_err {
               Err(ResolveError::from_serde_json_error(
-                package_json_path,
+                package_json_path.clone(),
                 &err,
                 Some(package_json_string),
               ))
             } else {
               let (line, column) = off_to_location(&package_json_string, parse_err.index());
 
               Err(ResolveError::JSON(JSONError {
-                path: package_json_path,
+                path: package_json_path.clone(),
                 message: parse_err.error().to_string(),
                 line,
                 column,
@@ -363,14 +365,10 @@
       }
       Ok(None) => {
         // Avoid an allocation by making this lazy
-        if let Some(deps) = &mut ctx.missing_dependencies {
-          deps.push(self.path.join("package.json"));
-        }
+        ctx.add_missing_dependency(package_json_path);
       }
       Err(_) => {
-        if let Some(deps) = &mut ctx.file_dependencies {
-          deps.push(self.path.join("package.json"));
-        }
+        ctx.add_file_dependency(package_json_path);
       }
     }
     result
@@ -390,7 +388,9 @@
 
 impl PartialEq for dyn CacheKey + '_ {
   fn eq(&self, other: &Self) -> bool {
-    self.tuple().1 == other.tuple().1
+    let self_tuple = self.tuple();
+    let other_tuple = other.tuple();
+    self_tuple.0 == other_tuple.0 && self_tuple.1 == other_tuple.1
   }
 }
 
@@ -407,20 +407,3 @@
     self
   }
 }
-
-/// Since the cache key is memoized, use an identity hasher
-/// to avoid double cache.
-#[derive(Default)]
-struct IdentityHasher(u64);
-
-impl Hasher for IdentityHasher {
-  fn write(&mut self, _: &[u8]) {
-    unreachable!("Invalid use of IdentityHasher")
-  }
-  fn write_u64(&mut self, n: u64) {
-    self.0 = n;
-  }
-  fn finish(&self) -> u64 {
-    self.0
-  }
-}

src/context.rs

@@ -3,7 +3,7 @@ use std::{
   path::{Path, PathBuf},
 };
 
-use crate::error::ResolveError;
+use crate::{error::ResolveError, PathDependency};
 
 #[derive(Debug, Default, Clone)]
 pub struct ResolveContext(ResolveContextImpl);
@@ -22,6 +22,12 @@ pub struct ResolveContextImpl {
   /// Files that was found on file system
   pub missing_dependencies: Option<Vec<PathBuf>>,
 
+  /// Files that was found on file system, with precomputed hashes.
+  pub prehashed_file_dependencies: Option<Vec<PathDependency>>,
+
+  /// Files that were not found on file system, with precomputed hashes.
+  pub prehashed_missing_dependencies: Option<Vec<PathDependency>>,
+
   /// The current resolving alias for bailing recursion alias.
   pub resolving_alias: Option<String>,
 
@@ -62,18 +68,43 @@ impl ResolveContext {
     self.missing_dependencies.replace(vec![]);
   }
 
+  pub fn init_prehashed_dependencies(&mut self) {
+    self.prehashed_file_dependencies.replace(vec![]);
+    self.prehashed_missing_dependencies.replace(vec![]);
+  }
+
   pub fn add_file_dependency(&mut self, dep: &Path) {
-    if let Some(deps) = &mut self.file_dependencies {
+    if let Some(deps) = &mut self.prehashed_file_dependencies {
+      deps.push(PathDependency::new(dep.to_path_buf()));
+    } else if let Some(deps) = &mut self.file_dependencies {
       deps.push(dep.to_path_buf());
     }
   }
 
+  pub fn add_file_dependency_with_hash(&mut self, dep: PathBuf, hash: u64) {
+    if let Some(deps) = &mut self.prehashed_file_dependencies {
+      deps.push(PathDependency::with_hash(dep, hash));
+    } else if let Some(deps) = &mut self.file_dependencies {
+      deps.push(dep);
+    }
+  }
+
   pub fn add_missing_dependency(&mut self, dep: &Path) {
-    if let Some(deps) = &mut self.missing_dependencies {
+    if let Some(deps) = &mut self.prehashed_missing_dependencies {
+      deps.push(PathDependency::new(dep.to_path_buf()));
+    } else if let Some(deps) = &mut self.missing_dependencies {
       deps.push(dep.to_path_buf());
     }
   }
 
+  pub fn add_missing_dependency_with_hash(&mut self, dep: PathBuf, hash: u64) {
+    if let Some(deps) = &mut self.prehashed_missing_dependencies {
+      deps.push(PathDependency::with_hash(dep, hash));
+    } else if let Some(deps) = &mut self.missing_dependencies {
+      deps.push(dep);
+    }
+  }
+
   pub fn with_resolving_alias(&mut self, alias: String) {
     self.resolving_alias = Some(alias);
   }

src/hashing.rs

@@ -0,0 +1,30 @@
+use std::{
+  hash::{Hash, Hasher},
+  path::Path,
+};
+
+use rustc_hash::FxHasher;
+
+#[derive(Default)]
+pub struct IdentityHasher(u64);
+
+impl Hasher for IdentityHasher {
+  fn write(&mut self, _: &[u8]) {
+    unreachable!("Invalid use of IdentityHasher")
+  }
+
+  fn write_u64(&mut self, n: u64) {
+    self.0 = n;
+  }
+
+  fn finish(&self) -> u64 {
+    self.0
+  }
+}
+
+#[inline]
+pub fn hash_path(path: &Path) -> u64 {
+  let mut hasher = FxHasher::default();
+  path.hash(&mut hasher);
+  hasher.finish()
+}