From 4dda5fac098d18d95768c130fbb87c4b4c0dc609 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 11 Oct 2024 07:53:58 +0200 Subject: [PATCH] [Turbopack] new backend cleanup --- .../turbo-tasks-backend/src/backend/mod.rs | 21 ++++++---- .../backend/operation/aggregation_update.rs | 40 +++++++++---------- .../src/backend/operation/invalidate.rs | 6 +-- .../src/backend/operation/update_cell.rs | 6 +-- .../src/backend/operation/update_output.rs | 4 +- .../src/backend/storage.rs | 6 +-- .../crates/turbo-tasks-backend/src/data.rs | 25 ++++++++++-- 7 files changed, 67 insertions(+), 41 deletions(-) diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs index 10178751ff166..cb0cd2911de89 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs @@ -358,7 +358,7 @@ impl TurboTasksBackendInner { // Check the dirty count of the root node let dirty_tasks = get!(task, AggregatedDirtyContainerCount) - .copied() + .cloned() .unwrap_or_default() .get(self.session_id); if dirty_tasks > 0 || is_dirty { @@ -375,7 +375,14 @@ impl TurboTasksBackendInner { value: RootState::new(ActiveType::CachedActiveUntilClean, task_id), }); // A newly added AggregateRoot need to make sure to schedule the tasks - task_ids_to_schedule = get_many!(task, AggregatedDirtyContainer { task } count if count.get(self.session_id) > 0 => task); + task_ids_to_schedule = get_many!( + task, + AggregatedDirtyContainer { + task + } count if count.get(self.session_id) > 0 => { + *task + } + ); if is_dirty { task_ids_to_schedule.push(task_id); } @@ -1076,7 +1083,7 @@ impl TurboTasksBackendInner { // handle cell counters: update max index and remove cells that are no longer used let mut removed_cells = HashMap::new(); let mut old_counters: HashMap<_, _> = - get_many!(task, CellTypeMaxIndex { cell_type } max_index => (cell_type, max_index)); + get_many!(task, CellTypeMaxIndex { cell_type } max_index => (*cell_type, *max_index)); for (&cell_type, &max_index) in cell_counters.iter() { if let Some(old_max_index) = old_counters.remove(&cell_type) { if old_max_index != max_index { @@ -1234,7 +1241,7 @@ impl TurboTasksBackendInner { let data_update = if old_dirty_state.is_some() || new_dirty_state.is_some() { let mut dirty_containers = get!(task, AggregatedDirtyContainerCount) - .copied() + .cloned() .unwrap_or_default(); if let Some(old_dirty_state) = old_dirty_state { dirty_containers.update_with_dirty_state(&old_dirty_state); @@ -1245,7 +1252,7 @@ impl TurboTasksBackendInner { (None, Some(new)) => dirty_containers.update_with_dirty_state(&new), (Some(old), Some(new)) => dirty_containers.replace_dirty_state(&old, &new), }; - if !aggregated_update.is_default() { + if !aggregated_update.is_zero() { if aggregated_update.get(self.session_id) < 0 { if let Some(root_state) = get!(task, AggregateRoot) { root_state.all_clean_event.notify(usize::MAX); @@ -1403,7 +1410,7 @@ impl TurboTasksBackendInner { task, AggregatedCollectible { collectible - } count if collectible.collectible_type == collectible_type && count > 0 => { + } count if collectible.collectible_type == collectible_type && *count > 0 => { collectible.cell } ) { @@ -1416,7 +1423,7 @@ impl TurboTasksBackendInner { Collectible { collectible } count if collectible.collectible_type == collectible_type => { - (collectible.cell, count) + (collectible.cell, *count) } ) { *collectibles diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs index bcfb7219e1cda..5e17e3adc5ae9 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs @@ -31,9 +31,9 @@ fn get_followers_with_aggregation_number( aggregation_number: u32, ) -> Vec { if is_aggregating_node(aggregation_number) { - get_many!(task, Follower { task } count if count > 0 => task) + get_many!(task, Follower { task } count if *count > 0 => *task) } else { - get_many!(task, Child { task } => task) + get_many!(task, Child { task } => *task) } } @@ -42,11 +42,11 @@ fn get_followers(task: &TaskGuard<'_>) -> Vec { } pub fn get_uppers(task: &TaskGuard<'_>) -> Vec { - get_many!(task, Upper { task } count if count > 0 => task) + get_many!(task, Upper { task } count if *count > 0 => *task) } fn iter_uppers<'a>(task: &'a TaskGuard<'a>) -> impl Iterator + 'a { - iter_many!(task, Upper { task } count if count > 0 => task) + iter_many!(task, Upper { task } count if *count > 0 => *task) } pub fn get_aggregation_number(task: &TaskGuard<'_>) -> u32 { @@ -126,17 +126,17 @@ impl AggregatedDataUpdate { let aggregation = get_aggregation_number(task); let mut dirty_container_count = Default::default(); let mut collectibles_update: Vec<_> = - get_many!(task, Collectible { collectible } => (collectible, 1)); + get_many!(task, Collectible { collectible } => (*collectible, 1)); if is_aggregating_node(aggregation) { dirty_container_count = get!(task, AggregatedDirtyContainerCount) - .copied() + .cloned() .unwrap_or_default(); let collectibles = iter_many!( task, AggregatedCollectible { collectible - } count if count > 0 => { - collectible + } count if *count > 0 => { + *collectible } ); for collectible in collectibles { @@ -148,7 +148,7 @@ impl AggregatedDataUpdate { } let mut result = Self::new().collectibles_update(collectibles_update); - if !dirty_container_count.is_default() { + if !dirty_container_count.is_zero() { let DirtyContainerCount { count, count_in_session, @@ -170,7 +170,7 @@ impl AggregatedDataUpdate { collectibles_update, } = &mut self; if let Some((_, value)) = dirty_container_update.as_mut() { - *value = value.invert() + *value = value.negate() } for (_, value) in collectibles_update.iter_mut() { *value = -*value; @@ -199,7 +199,7 @@ impl AggregatedDataUpdate { }) } - let mut aggregated_update = Default::default(); + let mut aggregated_update = DirtyContainerCount::default(); update!( task, AggregatedDirtyContainer { @@ -208,7 +208,7 @@ impl AggregatedDataUpdate { |old: Option| { let mut new = old.unwrap_or_default(); aggregated_update = new.update_count(count); - (!new.is_default()).then_some(new) + (!new.is_zero()).then_some(new) } ); @@ -225,10 +225,10 @@ impl AggregatedDataUpdate { if let Some(dirty_state) = dirty_state { new.undo_update_with_dirty_state(&dirty_state); } - if !aggregated_update.is_default() { + if !aggregated_update.is_zero() { result.dirty_container_update = Some((task_id, aggregated_update)); } - (!new.is_default()).then_some(new) + (!new.is_zero()).then_some(new) }); if let Some((_, count)) = result.dirty_container_update.as_ref() { if count.get(session_id) < 0 { @@ -269,8 +269,8 @@ impl AggregatedDataUpdate { CollectiblesDependent { collectible_type, task, - } if collectible_type == ty => { - task + } if *collectible_type == ty => { + *task } ); if !dependent.is_empty() { @@ -608,7 +608,7 @@ impl AggregationUpdateQueue { value: RootState::new(ActiveType::CachedActiveUntilClean, task_id), }); } - let dirty_containers: Vec<_> = get_many!(task, AggregatedDirtyContainer { task } count if count.get(session_id) > 0 => task); + let dirty_containers: Vec<_> = get_many!(task, AggregatedDirtyContainer { task } count if count.get(session_id) > 0 => *task); if !dirty_containers.is_empty() { self.push(AggregationUpdateJob::FindAndScheduleDirty { task_ids: dirty_containers, @@ -954,7 +954,7 @@ impl AggregationUpdateQueue { if !is_aggregating_node(old) && is_aggregating_node(aggregation_number) { // When converted from leaf to aggregating node, all children become // followers - let children: Vec<_> = get_many!(task, Child { task } => task); + let children: Vec<_> = get_many!(task, Child { task } => *task); for child_id in children { task.add_new(CachedDataItem::Follower { task: child_id, @@ -966,7 +966,7 @@ impl AggregationUpdateQueue { if is_aggregating_node(aggregation_number) { // followers might become inner nodes when the aggregation number is // increased - let followers = iter_many!(task, Follower { task } count if count > 0 => task); + let followers = iter_many!(task, Follower { task } count if *count > 0 => *task); for follower_id in followers { self.push(AggregationUpdateJob::BalanceEdge { upper_id: task_id, @@ -978,7 +978,7 @@ impl AggregationUpdateQueue { self.push(AggregationUpdateJob::BalanceEdge { upper_id, task_id }); } } else { - let children = iter_many!(task, Child { task } => task); + let children = iter_many!(task, Child { task } => *task); for child_id in children { self.push(AggregationUpdateJob::UpdateAggregationNumber { task_id: child_id, diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs index c03cf974d3340..c0ad76c1b7b9c 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs @@ -112,7 +112,7 @@ pub fn make_task_dirty_internal( }) => { // Got dirty in that one session only let mut dirty_container = get!(task, AggregatedDirtyContainerCount) - .copied() + .cloned() .unwrap_or_default(); dirty_container.update_session_dependent(session_id, 1); dirty_container @@ -120,7 +120,7 @@ pub fn make_task_dirty_internal( None => { // Get dirty for all sessions get!(task, AggregatedDirtyContainerCount) - .copied() + .cloned() .unwrap_or_default() } _ => unreachable!(), @@ -128,7 +128,7 @@ pub fn make_task_dirty_internal( let aggregated_update = dirty_container.update_with_dirty_state(&DirtyState { clean_in_session: None, }); - if !aggregated_update.is_default() { + if !aggregated_update.is_zero() { queue.extend(AggregationUpdateJob::data_update( task, AggregatedDataUpdate::new().dirty_container_update(task_id, aggregated_update), diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_cell.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_cell.rs index 330849010fdc4..30dbad3d36309 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_cell.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_cell.rs @@ -40,9 +40,9 @@ impl UpdateCellOperation { let dependent = get_many!( task, - CellDependent { cell: dependent_cell, task } _value - if dependent_cell == cell - => task + CellDependent { cell: dependent_cell, task } + if *dependent_cell == cell + => *task ); drop(task); diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs index f791a11930b87..3874fbd0da535 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs @@ -103,8 +103,8 @@ impl UpdateOutputOperation { value: output_value, }); - let dependent_tasks = get_many!(task, OutputDependent { task } => task); - let children = get_many!(task, Child { task } => task); + let dependent_tasks = get_many!(task, OutputDependent { task } => *task); + let children = get_many!(task, Child { task } => *task); let mut queue = AggregationUpdateQueue::new(); diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs b/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs index 64025dad05524..2ca24bbdf82ab 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs @@ -435,7 +435,7 @@ macro_rules! iter_many { $task .iter($crate::data::indicies::$key) .filter_map(|(key, _)| match key { - &$crate::data::CachedDataItemKey::$key $key_pattern $(if $cond)? => Some( + $crate::data::CachedDataItemKey::$key $key_pattern $(if $cond)? => Some( $iter_item ), _ => None, @@ -446,8 +446,8 @@ macro_rules! iter_many { .iter($crate::data::indicies::$key) .filter_map(|(key, value)| match (key, value) { ( - &$crate::data::CachedDataItemKey::$key $input, - &$crate::data::CachedDataItemValue::$key { value: $value_pattern } + $crate::data::CachedDataItemKey::$key $input, + $crate::data::CachedDataItemValue::$key { value: $value_pattern } ) $(if $cond)? => Some($iter_item), _ => None, }) diff --git a/turbopack/crates/turbo-tasks-backend/src/data.rs b/turbopack/crates/turbo-tasks-backend/src/data.rs index c45f947fe40dd..04fdd851affe5 100644 --- a/turbopack/crates/turbo-tasks-backend/src/data.rs +++ b/turbopack/crates/turbo-tasks-backend/src/data.rs @@ -108,13 +108,18 @@ fn add_with_diff(v: &mut i32, u: i32) -> i32 { } } -#[derive(Debug, Default, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +/// Represents a count of dirty containers. Since dirtyness can be session dependent, there might be +/// a different count for a specific session. It only need to store the highest session count, since +/// old sessions can't be visited again, so we can ignore their counts. +#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct DirtyContainerCount { pub count: i32, pub count_in_session: Option<(SessionId, i32)>, } impl DirtyContainerCount { + /// Get the count for a specific session. It's only expected to be asked for the current + /// session, since old session counts might be dropped. pub fn get(&self, session: SessionId) -> i32 { if let Some((s, count)) = self.count_in_session { if s == session { @@ -124,6 +129,7 @@ impl DirtyContainerCount { self.count } + /// Increase/decrease the count by the given value. pub fn update(&mut self, count: i32) -> DirtyContainerCount { self.update_count(&DirtyContainerCount { count, @@ -131,6 +137,8 @@ impl DirtyContainerCount { }) } + /// Increase/decrease the count by the given value, but does not update the count for a specific + /// session. This matches the "dirty, but clean in one session" behavior. pub fn update_session_dependent( &mut self, ignore_session: SessionId, @@ -142,6 +150,10 @@ impl DirtyContainerCount { }) } + /// Adds the `count` to the current count. This correctly handles session dependent counts. + /// Returns a new count object that represents the aggregated count. The aggregated count will + /// be +1 when the self count changes from <= 0 to > 0 and -1 when the self count changes from > + /// 0 to <= 0. The same for the session dependent count. pub fn update_count(&mut self, count: &DirtyContainerCount) -> DirtyContainerCount { let mut diff = DirtyContainerCount::default(); match ( @@ -181,6 +193,7 @@ impl DirtyContainerCount { diff } + /// Applies a dirty state to the count. Returns an aggregated count that represents the change. pub fn update_with_dirty_state(&mut self, dirty: &DirtyState) -> DirtyContainerCount { if let Some(clean_in_session) = dirty.clean_in_session { self.update_session_dependent(clean_in_session, 1) @@ -189,6 +202,8 @@ impl DirtyContainerCount { } } + /// Undoes the effect of a dirty state on the count. Returns an aggregated count that represents + /// the change. pub fn undo_update_with_dirty_state(&mut self, dirty: &DirtyState) -> DirtyContainerCount { if let Some(clean_in_session) = dirty.clean_in_session { self.update_session_dependent(clean_in_session, -1) @@ -197,6 +212,8 @@ impl DirtyContainerCount { } } + /// Replaces the old dirty state with the new one. Returns an aggregated count that represents + /// the change. pub fn replace_dirty_state( &mut self, old: &DirtyState, @@ -207,11 +224,13 @@ impl DirtyContainerCount { diff } - pub fn is_default(&self) -> bool { + /// Returns true if the count is zero and appling it would have no effect + pub fn is_zero(&self) -> bool { self.count == 0 && self.count_in_session.map(|(_, c)| c == 0).unwrap_or(true) } - pub fn invert(&self) -> Self { + /// Negates the counts. + pub fn negate(&self) -> Self { Self { count: -self.count, count_in_session: self.count_in_session.map(|(s, c)| (s, -c)),