From 18b1f90aefb5a4f9a7024a456f7dc44dd727fc9e Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Fri, 25 Jul 2025 16:13:12 -0700 Subject: [PATCH 1/2] Turbopack: Use `Path`/`PathBuf` for all of the invalidation logic --- crates/next-api/src/project.rs | 10 +++--- .../turbo-tasks-fs/src/invalidator_map.rs | 17 +++++++--- turbopack/crates/turbo-tasks-fs/src/lib.rs | 34 +++++++++---------- .../crates/turbo-tasks-fs/src/watcher.rs | 17 +++++----- 4 files changed, 42 insertions(+), 36 deletions(-) diff --git a/crates/next-api/src/project.rs b/crates/next-api/src/project.rs index 87204aca4256..ec4964a5a359 100644 --- a/crates/next-api/src/project.rs +++ b/crates/next-api/src/project.rs @@ -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()), }); } diff --git a/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs b/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs index b384027f04eb..a99b7f853251 100644 --- a/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs +++ b/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs @@ -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), } -type InnerMap = FxHashMap>>; +type InnerMap = FxHashMap>>; pub struct InvalidatorMap { - queue: ConcurrentQueue<(String, Invalidator, Option)>, + queue: ConcurrentQueue<(PathBuf, Invalidator, Option)>, map: Mutex, } @@ -44,7 +47,7 @@ impl InvalidatorMap { pub fn insert( &self, - key: String, + key: PathBuf, invalidator: Invalidator, write_content: Option, ) { @@ -66,7 +69,11 @@ impl Serialize for InvalidatorMap { where S: serde::Serializer, { - serializer.serialize_newtype_struct("InvalidatorMap", &*self.lock().unwrap()) + // TODO: This stores `PathBuf`s, which are machine-specific, but turbo_tasks doesn't + // actually care about these, so we should skip serializing them entirely. Really we just + // need a way to store list of invalidator task ids across restarts. + let inner: &LockedInvalidatorMap = &self.lock().unwrap(); + serializer.serialize_newtype_struct("InvalidatorMap", inner) } } diff --git a/turbopack/crates/turbo-tasks-fs/src/lib.rs b/turbopack/crates/turbo-tasks-fs/src/lib.rs index de1aaf4898ba..2893e0b60924 100644 --- a/turbopack/crates/turbo-tasks-fs/src/lib.rs +++ b/turbopack/crates/turbo-tasks-fs/src/lib.rs @@ -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)>> { 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( &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( &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) -> 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(()); diff --git a/turbopack/crates/turbo-tasks-fs/src/watcher.rs b/turbopack/crates/turbo-tasks-fs/src/watcher.rs index 093b6c5b63b0..646703b3a05a 100644 --- a/turbopack/crates/turbo-tasks-fs/src/watcher.rs +++ b/turbopack/crates/turbo-tasks-fs/src/watcher.rs @@ -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 = 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>>, + invalidator_map: &mut FxHashMap>>, paths: impl Iterator, ) { 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>>, + invalidator_map: &mut FxHashMap>>, paths: impl Iterator, ) { 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)); From 5c1aea2821d54b8e09f3956b95510b5be643f38c Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Tue, 29 Jul 2025 15:16:25 -0700 Subject: [PATCH 2/2] update comment --- .../crates/turbo-tasks-fs/src/invalidator_map.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs b/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs index a99b7f853251..c5c3bf8630c7 100644 --- a/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs +++ b/turbopack/crates/turbo-tasks-fs/src/invalidator_map.rs @@ -69,10 +69,14 @@ impl Serialize for InvalidatorMap { where S: serde::Serializer, { - // TODO: This stores `PathBuf`s, which are machine-specific, but turbo_tasks doesn't - // actually care about these, so we should skip serializing them entirely. Really we just - // need a way to store list of invalidator task ids across restarts. - let inner: &LockedInvalidatorMap = &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) } }