diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs index de63acd9826bd..4611fc407e0da 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs @@ -869,16 +869,20 @@ impl TurboTasksBackendInner { return (task_id, None, None); } let len = inner.len(); - let mut meta = Vec::with_capacity(len); - let mut data = Vec::with_capacity(len); + let mut meta = inner.meta_modified.then(|| Vec::with_capacity(len)); + let mut data = inner.data_modified.then(|| Vec::with_capacity(len)); for (key, value) in inner.iter_all() { if key.is_persistent() && value.is_persistent() { match key.category() { TaskDataCategory::Meta => { - meta.push(CachedDataItem::from_key_and_value_ref(key, value)) + if let Some(meta) = &mut meta { + meta.push(CachedDataItem::from_key_and_value_ref(key, value)); + } } TaskDataCategory::Data => { - data.push(CachedDataItem::from_key_and_value_ref(key, value)) + if let Some(data) = &mut data { + data.push(CachedDataItem::from_key_and_value_ref(key, value)); + } } _ => {} } @@ -886,8 +890,8 @@ impl TurboTasksBackendInner { } ( task_id, - inner.meta_restored.then(|| B::serialize(task_id, &meta)), - inner.data_restored.then(|| B::serialize(task_id, &data)), + meta.map(|meta| B::serialize(task_id, &meta)), + data.map(|data| B::serialize(task_id, &data)), ) }; diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs index f76057e6ae7c4..e1a43103929ac 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs @@ -18,8 +18,8 @@ use turbo_tasks::{SessionId, TaskId, TurboTasksBackendApi}; use crate::{ backend::{ - storage::StorageWriteGuard, OperationGuard, TaskDataCategory, TransientTask, - TurboTasksBackend, TurboTasksBackendInner, + storage::{SpecificTaskDataCategory, StorageWriteGuard}, + OperationGuard, TaskDataCategory, TransientTask, TurboTasksBackend, TurboTasksBackendInner, }, backing_storage::BackingStorage, data::{ @@ -463,9 +463,10 @@ impl TaskGuard for TaskGuardImpl<'_, B> { } fn add(&mut self, item: CachedDataItem) -> bool { - self.check_access(item.category()); + let category = item.category(); + self.check_access(category); if !self.task_id.is_transient() && item.is_persistent() { - self.task.track_modification(); + self.task.track_modification(category.into_specific()); } self.task.add(item) } @@ -477,9 +478,10 @@ impl TaskGuard for TaskGuardImpl<'_, B> { } fn insert(&mut self, item: CachedDataItem) -> Option { - self.check_access(item.category()); + let category = item.category(); + self.check_access(category); if !self.task_id.is_transient() && item.is_persistent() { - self.task.track_modification(); + self.task.track_modification(category.into_specific()); } self.task.insert(item) } @@ -489,17 +491,19 @@ impl TaskGuard for TaskGuardImpl<'_, B> { key: CachedDataItemKey, update: impl FnOnce(Option) -> Option, ) { - self.check_access(key.category()); + let category = key.category(); + self.check_access(category); if !self.task_id.is_transient() && key.is_persistent() { - self.task.track_modification(); + self.task.track_modification(category.into_specific()); } self.task.update(key, update); } fn remove(&mut self, key: &CachedDataItemKey) -> Option { - self.check_access(key.category()); + let category = key.category(); + self.check_access(category); if !self.task_id.is_transient() && key.is_persistent() { - self.task.track_modification(); + self.task.track_modification(category.into_specific()); } self.task.remove(key) } @@ -510,9 +514,10 @@ impl TaskGuard for TaskGuardImpl<'_, B> { } fn get_mut(&mut self, key: &CachedDataItemKey) -> Option> { - self.check_access(key.category()); + let category = key.category(); + self.check_access(category); if !self.task_id.is_transient() && key.is_persistent() { - self.task.track_modification(); + self.task.track_modification(category.into_specific()); } self.task.get_mut(key) } @@ -522,9 +527,10 @@ impl TaskGuard for TaskGuardImpl<'_, B> { key: CachedDataItemKey, insert: impl FnOnce() -> CachedDataItemValue, ) -> CachedDataItemValueRefMut<'_> { - self.check_access(key.category()); + let category = key.category(); + self.check_access(category); if !self.task_id.is_transient() && key.is_persistent() { - self.task.track_modification(); + self.task.track_modification(category.into_specific()); } self.task.get_mut_or_insert_with(key, insert) } @@ -561,14 +567,17 @@ impl TaskGuard for TaskGuardImpl<'_, B> { { self.check_access(ty.category()); if !self.task_id.is_transient() && ty.is_persistent() { - self.task.track_modification(); + self.task.track_modification(ty.category().into_specific()); } self.task.extract_if(ty, f) } fn invalidate_serialization(&mut self) { + // TODO this causes race conditions, since we never know when a value is changed. We can't + // "snapshot" the value correctly. if !self.task_id.is_transient() { - self.task.track_modification(); + self.task.track_modification(SpecificTaskDataCategory::Data); + self.task.track_modification(SpecificTaskDataCategory::Meta); } } } diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs b/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs index 4719105ce152d..2b27335055d22 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs @@ -26,6 +26,22 @@ pub enum TaskDataCategory { All, } +impl TaskDataCategory { + pub fn into_specific(self) -> SpecificTaskDataCategory { + match self { + TaskDataCategory::Meta => SpecificTaskDataCategory::Meta, + TaskDataCategory::Data => SpecificTaskDataCategory::Data, + TaskDataCategory::All => unreachable!(), + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum SpecificTaskDataCategory { + Meta, + Data, +} + impl IntoIterator for TaskDataCategory { type Item = TaskDataCategory; @@ -77,9 +93,11 @@ bitfield! { pub meta_restored, set_meta_restored: 0; pub data_restored, set_data_restored: 1; /// Item was modified before snapshot mode was entered. - pub modified, set_modified: 2; + pub meta_modified, set_meta_modified: 2; + pub data_modified, set_data_modified: 3; /// Item was modified after snapshot mode was entered. A snapshot was taken. - pub snapshot, set_snapshot: 3; + pub meta_snapshot, set_meta_snapshot: 4; + pub data_snapshot, set_data_snapshot: 4; } impl InnerStorageState { @@ -105,6 +123,14 @@ impl InnerStorageState { TaskDataCategory::All => self.meta_restored() && self.data_restored(), } } + + pub fn any_snapshot(&self) -> bool { + self.meta_snapshot() || self.data_snapshot() + } + + pub fn any_modified(&self) -> bool { + self.meta_modified() || self.data_modified() + } } pub struct InnerStorageSnapshot { @@ -113,8 +139,8 @@ pub struct InnerStorageSnapshot { output: OptionStorage, upper: AutoMapStorage, dynamic: DynamicStorage, - pub meta_restored: bool, - pub data_restored: bool, + pub meta_modified: bool, + pub data_modified: bool, } impl From<&InnerStorage> for InnerStorageSnapshot { @@ -125,8 +151,8 @@ impl From<&InnerStorage> for InnerStorageSnapshot { output: inner.output.clone(), upper: inner.upper.clone(), dynamic: inner.dynamic.snapshot_for_persisting(), - meta_restored: inner.state.meta_restored(), - data_restored: inner.state.data_restored(), + meta_modified: inner.state.meta_modified(), + data_modified: inner.state.data_modified(), } } } @@ -701,7 +727,9 @@ impl Storage { // We also need to unset all the modified flags. for key in removed_modified { if let Some(mut inner) = self.map.get_mut(&key) { - inner.state_mut().set_modified(false); + let state = inner.state_mut(); + state.set_data_modified(false); + state.set_meta_modified(false); } } @@ -728,8 +756,15 @@ impl Storage { // And update the flags for key in removed_snapshots { if let Some(mut inner) = self.map.get_mut(&key) { - inner.state_mut().set_snapshot(false); - inner.state_mut().set_modified(true); + let state = inner.state_mut(); + if state.meta_snapshot() { + state.set_meta_snapshot(false); + state.set_meta_modified(true); + } + if state.data_snapshot() { + state.set_data_snapshot(false); + state.set_data_modified(true); + } } } @@ -779,15 +814,30 @@ pub struct StorageWriteGuard<'a> { impl StorageWriteGuard<'_> { /// Tracks mutation of this task - pub fn track_modification(&mut self) { - if !self.inner.state().snapshot() { - match (self.storage.snapshot_mode(), self.inner.state().modified()) { + pub fn track_modification(&mut self, category: SpecificTaskDataCategory) { + let state = self.inner.state(); + let snapshot = match category { + SpecificTaskDataCategory::Meta => state.meta_snapshot(), + SpecificTaskDataCategory::Data => state.data_snapshot(), + }; + if !snapshot { + let modified = match category { + SpecificTaskDataCategory::Meta => state.meta_modified(), + SpecificTaskDataCategory::Data => state.data_modified(), + }; + match (self.storage.snapshot_mode(), modified) { (false, false) => { // Not in snapshot mode and item is unmodified - self.storage - .modified - .insert(*self.inner.key(), ModifiedState::Modified); - self.inner.state_mut().set_modified(true); + if !state.any_snapshot() && !state.any_modified() { + self.storage + .modified + .insert(*self.inner.key(), ModifiedState::Modified); + } + let state = self.inner.state_mut(); + match category { + SpecificTaskDataCategory::Meta => state.set_meta_modified(true), + SpecificTaskDataCategory::Data => state.set_data_modified(true), + } } (false, true) => { // Not in snapshot mode and item is already modfied @@ -795,19 +845,31 @@ impl StorageWriteGuard<'_> { } (true, false) => { // In snapshot mode and item is unmodified (so it's not part of the snapshot) - self.storage - .modified - .insert(*self.inner.key(), ModifiedState::Snapshot(None)); - self.inner.state_mut().set_snapshot(true); + if !state.any_snapshot() { + self.storage + .modified + .insert(*self.inner.key(), ModifiedState::Snapshot(None)); + } + let state = self.inner.state_mut(); + match category { + SpecificTaskDataCategory::Meta => state.set_meta_snapshot(true), + SpecificTaskDataCategory::Data => state.set_data_snapshot(true), + } } (true, true) => { // In snapshot mode and item is modified (so it's part of the snapshot) // We need to store the original version that is part of the snapshot - self.storage.modified.insert( - *self.inner.key(), - ModifiedState::Snapshot(Some(Box::new((&**self.inner).into()))), - ); - self.inner.state_mut().set_snapshot(true); + if !state.any_snapshot() { + self.storage.modified.insert( + *self.inner.key(), + ModifiedState::Snapshot(Some(Box::new((&**self.inner).into()))), + ); + } + let state = self.inner.state_mut(); + match category { + SpecificTaskDataCategory::Meta => state.set_meta_snapshot(true), + SpecificTaskDataCategory::Data => state.set_data_snapshot(true), + } } } } @@ -1055,7 +1117,8 @@ where } while let Some(task_id) = self.modified.pop() { let inner = self.storage.map.get(&task_id).unwrap(); - if !inner.state().snapshot() { + let state = inner.state(); + if !state.any_snapshot() { let preprocessed = (self.preprocess)(task_id, &inner); drop(inner); return Some((self.process)(task_id, preprocessed));