Repository: vercel/next.js
Pull request: Turbopack: Use `Path`/`PathBuf` for all of the invalidation logic

Common convention in our codebase is that `Path`/`PathBuf` are used for raw OS-level paths, and `RcStr` is used for normalized paths exposed by `turbo_tasks_fs`.

These are raw OS paths, so we should use `Path`/`PathBuf`.

This has some benefits in that it makes the implementation of https://github.com/vercel/next.js/pull/82133 easier, because `Path`'s `Ord` implementation is better for our use-case.

Closes PACK-5151

Changed files:
filename: crates/next-api/src/project.rs
patch: @@ -323,14 +323,15 @@ impl ProjectContainer {
                 .await?;
         } else {
             project_fs.invalidate_with_reason(|path| invalidation::Initialize {
-                path: RcStr::from(path),
+                // this path is just used for display purposes
+                path: RcStr::from(path.to_string_lossy()),
             });
         }
         let output_fs = output_fs_operation(project)
             .read_strongly_consistent()
             .await?;
         output_fs.invalidate_with_reason(|path| invalidation::Initialize {
-            path: RcStr::from(path),
+            path: RcStr::from(path.to_string_lossy()),
         });
         Ok(())
     }
@@ -421,13 +422,14 @@ impl ProjectContainer {
                     .await?;
             } else {
                 project_fs.invalidate_with_reason(|path| invalidation::Initialize {
-                    path: RcStr::from(path),
+                    // this path is just used for display purposes
+                    path: RcStr::from(path.to_string_lossy()),
                 });
             }
         }
         if !ReadRef::ptr_eq(&prev_output_fs, &output_fs) {
             prev_output_fs.invalidate_with_reason(|path| invalidation::Initialize {
-                path: RcStr::from(path),
+                path: RcStr::from(path.to_string_lossy()),
             });
         }
 

filename: turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs
patch: @@ -1,4 +1,7 @@
-use std::sync::{LockResult, Mutex, MutexGuard};
+use std::{
+    path::PathBuf,
+    sync::{LockResult, Mutex, MutexGuard},
+};
 
 use concurrent_queue::ConcurrentQueue;
 use rustc_hash::FxHashMap;
@@ -13,10 +16,10 @@ pub enum WriteContent {
     Link(ReadRef<LinkContent>),
 }
 
-type InnerMap = FxHashMap<String, FxHashMap<Invalidator, Option<WriteContent>>>;
+type InnerMap = FxHashMap<PathBuf, FxHashMap<Invalidator, Option<WriteContent>>>;
 
 pub struct InvalidatorMap {
-    queue: ConcurrentQueue<(String, Invalidator, Option<WriteContent>)>,
+    queue: ConcurrentQueue<(PathBuf, Invalidator, Option<WriteContent>)>,
     map: Mutex<InnerMap>,
 }
 
@@ -44,7 +47,7 @@ impl InvalidatorMap {
 
     pub fn insert(
         &self,
-        key: String,
+        key: PathBuf,
         invalidator: Invalidator,
         write_content: Option<WriteContent>,
     ) {
@@ -66,7 +69,15 @@ impl Serialize for InvalidatorMap {
     where
         S: serde::Serializer,
     {
-        serializer.serialize_newtype_struct("InvalidatorMap", &*self.lock().unwrap())
+        // TODO: This stores absolute `PathBuf`s, which are machine-specific. This should
+        // normalize/denormalize paths relative to the disk filesystem root.
+        //
+        // Potential optimization: We invalidate all fs reads immediately upon resuming from a
+        // persisted cache, but we don't invalidate the fs writes. Those read invalidations trigger
+        // re-inserts into the `InvalidatorMap`. If we knew that certain invalidators were only
+        // needed for reads, we could potentially avoid serializing those paths entirely.
+        let inner: &InnerMap = &self.lock().unwrap();
+        serializer.serialize_newtype_struct("InvalidatorMap", inner)
     }
 }
 

filename: turbopack/crates/turbo-tasks-fs/src/lib.rs
patch: @@ -254,7 +254,7 @@ impl DiskFileSystemInner {
     fn register_read_invalidator(&self, path: &Path) -> Result<()> {
         let invalidator = turbo_tasks::get_invalidator();
         self.invalidator_map
-            .insert(path_to_key(path), invalidator, None);
+            .insert(path.to_owned(), invalidator, None);
         if let Some(non_recursive) = &self.watcher.non_recursive_state
             && let Some(dir) = path.parent()
         {
@@ -273,7 +273,7 @@ impl DiskFileSystemInner {
         write_content: WriteContent,
     ) -> Result<Vec<(Invalidator, Option<WriteContent>)>> {
         let mut invalidator_map = self.invalidator_map.lock().unwrap();
-        let invalidators = invalidator_map.entry(path_to_key(path)).or_default();
+        let invalidators = invalidator_map.entry(path.to_owned()).or_default();
         let old_invalidators = invalidators
             .extract_if(|i, old_write_content| {
                 i == &invalidator
@@ -298,7 +298,7 @@ impl DiskFileSystemInner {
     fn register_dir_invalidator(&self, path: &Path) -> Result<()> {
         let invalidator = turbo_tasks::get_invalidator();
         self.dir_invalidator_map
-            .insert(path_to_key(path), invalidator, None);
+            .insert(path.to_owned(), invalidator, None);
         if let Some(non_recursive) = &self.watcher.non_recursive_state {
             non_recursive.ensure_watching(&self.watcher, path, self.root_path())?;
         }
@@ -333,7 +333,7 @@ impl DiskFileSystemInner {
     /// Calls the given
     fn invalidate_with_reason<R: InvalidationReason + Clone>(
         &self,
-        reason: impl Fn(String) -> R + Sync,
+        reason: impl Fn(&Path) -> R + Sync,
     ) {
         let _span = tracing::info_span!("invalidate filesystem", name = &*self.root).entered();
         let span = tracing::Span::current();
@@ -345,7 +345,7 @@ impl DiskFileSystemInner {
             .chain(dir_invalidator_map.into_par_iter())
             .flat_map(|(path, invalidators)| {
                 let _span = span.clone().entered();
-                let reason_for_path = reason(path);
+                let reason_for_path = reason(&path);
                 invalidators
                     .into_par_iter()
                     .map(move |i| (reason_for_path.clone(), i))
@@ -449,7 +449,7 @@ impl DiskFileSystem {
 
     pub fn invalidate_with_reason<R: InvalidationReason + Clone>(
         &self,
-        reason: impl Fn(String) -> R + Sync,
+        reason: impl Fn(&Path) -> R + Sync,
     ) {
         self.inner.invalidate_with_reason(reason);
     }
@@ -504,10 +504,6 @@ fn format_absolute_fs_path(path: &Path, name: &str, root_path: &Path) -> Option<
     }
 }
 
-pub fn path_to_key(path: impl AsRef<Path>) -> String {
-    path.as_ref().to_string_lossy().to_string()
-}
-
 #[turbo_tasks::value_impl]
 impl DiskFileSystem {
     /// Create a new instance of `DiskFileSystem`.
@@ -756,11 +752,12 @@ impl FileSystem for DiskFileSystem {
                 .await?;
             if compare == FileComparison::Equal {
                 if !old_invalidators.is_empty() {
-                    let key = path_to_key(&full_path);
                     for (invalidator, write_content) in old_invalidators {
-                        inner
-                            .invalidator_map
-                            .insert(key.clone(), invalidator, write_content);
+                        inner.invalidator_map.insert(
+                            full_path.clone().into_owned(),
+                            invalidator,
+                            write_content,
+                        );
                     }
                 }
                 return Ok(());
@@ -899,11 +896,12 @@ impl FileSystem for DiskFileSystem {
             };
             if is_equal {
                 if !old_invalidators.is_empty() {
-                    let key = path_to_key(&full_path);
                     for (invalidator, write_content) in old_invalidators {
-                        inner
-                            .invalidator_map
-                            .insert(key.clone(), invalidator, write_content);
+                        inner.invalidator_map.insert(
+                            full_path.clone().into_owned(),
+                            invalidator,
+                            write_content,
+                        );
                     }
                 }
                 return Ok(());

filename: turbopack/crates/turbo-tasks-fs/src/watcher.rs
patch: @@ -30,7 +30,6 @@ use crate::{
     DiskFileSystemInner, format_absolute_fs_path,
     invalidation::{WatchChange, WatchStart},
     invalidator_map::WriteContent,
-    path_to_key,
 };
 
 static WATCH_RECURSIVE_MODE: LazyLock<RecursiveMode> = LazyLock::new(|| {
@@ -279,7 +278,8 @@ impl DiskWatcher {
                     let _span = span.clone().entered();
                     let reason = WatchStart {
                         name: fs_inner.name.clone(),
-                        path: path.into(),
+                        // this path is just used for display purposes
+                        path: RcStr::from(path.to_string_lossy()),
                     };
                     invalidators
                         .into_par_iter()
@@ -374,7 +374,8 @@ impl DiskWatcher {
 
                             if report_invalidation_reason {
                                 inner.invalidate_with_reason(|path| InvalidateRescan {
-                                    path: RcStr::from(path),
+                                    // this path is just used for display purposes
+                                    path: RcStr::from(path.to_string_lossy()),
                                 });
                             } else {
                                 inner.invalidate();
@@ -601,12 +602,11 @@ fn invalidate(
 fn invalidate_path(
     inner: &DiskFileSystemInner,
     report_invalidation_reason: bool,
-    invalidator_map: &mut FxHashMap<String, FxHashMap<Invalidator, Option<WriteContent>>>,
+    invalidator_map: &mut FxHashMap<PathBuf, FxHashMap<Invalidator, Option<WriteContent>>>,
     paths: impl Iterator<Item = PathBuf>,
 ) {
     for path in paths {
-        let key = path_to_key(&path);
-        if let Some(invalidators) = invalidator_map.remove(&key) {
+        if let Some(invalidators) = invalidator_map.remove(&path) {
             invalidators
                 .into_iter()
                 .for_each(|(i, _)| invalidate(inner, report_invalidation_reason, &path, i));
@@ -617,12 +617,11 @@ fn invalidate_path(
 fn invalidate_path_and_children_execute(
     inner: &DiskFileSystemInner,
     report_invalidation_reason: bool,
-    invalidator_map: &mut FxHashMap<String, FxHashMap<Invalidator, Option<WriteContent>>>,
+    invalidator_map: &mut FxHashMap<PathBuf, FxHashMap<Invalidator, Option<WriteContent>>>,
     paths: impl Iterator<Item = PathBuf>,
 ) {
     for path in paths {
-        let path_key = path_to_key(&path);
-        for (_, invalidators) in invalidator_map.extract_if(|key, _| key.starts_with(&path_key)) {
+        for (_, invalidators) in invalidator_map.extract_if(|key, _| key.starts_with(&path)) {
             invalidators
                 .into_iter()
                 .for_each(|(i, _)| invalidate(inner, report_invalidation_reason, &path, i));