Skip to content

Commit 3b6f016

Browse files
sokrasokra
authored
Turbopack: Split AggregatedDirtyContainer (#86606)
Split AggregatedDirtyContainer rename and helper update from_task add dirty_container_count method Allow infinite test runs improve test case to test session dependent and restoring improve names <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # -->
1 parent a682261 commit 3b6f016

File tree

8 files changed

+332
-89
lines changed

8 files changed

+332
-89
lines changed

‎turbopack/crates/turbo-tasks-backend/src/backend/mod.rs‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -592,14 +592,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
592592
.set_active_until_clean();
593593
if ctx.should_track_activeness() {
594594
// A newly added Activeness need to make sure to schedule the tasks
595-
task_ids_to_schedule = get_many!(
596-
task,
597-
AggregatedDirtyContainer {
598-
task
599-
} count if count.get(self.session_id) > 0 => {
600-
task
601-
}
602-
);
595+
task_ids_to_schedule = task.dirty_containers(self.session_id).collect();
603596
task_ids_to_schedule.push(task_id);
604597
}
605598
get!(task, Activeness).unwrap()
@@ -660,16 +653,8 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
660653
"{task_id} {task_description}{count} (aggr={aggregation_number}, \
661654
{in_progress}, {activeness}{is_dirty})",
662655
);
663-
let children: Vec<_> = iter_many!(
664-
task,
665-
AggregatedDirtyContainer {
666-
task
667-
} count => {
668-
(task, count.get(ctx.session_id()))
669-
}
670-
)
671-
.filter(|(_, count)| *count > 0)
672-
.collect();
656+
let children: Vec<_> =
657+
task.dirty_containers_with_count(ctx.session_id()).collect();
673658
drop(task);
674659

675660
if missing_upper {
@@ -2998,9 +2983,9 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
29982983
}
29992984
}
30002985

3001-
let is_dirty = task.is_dirty(self.session_id);
3002-
let has_dirty_container = get!(task, AggregatedDirtyContainerCount)
3003-
.is_some_and(|count| count.get(self.session_id) > 0);
2986+
let is_dirty = get!(task, Dirty).is_some();
2987+
let has_dirty_container =
2988+
get!(task, AggregatedDirtyContainerCount).is_some_and(|count| count.count > 0);
30042989
let should_be_in_upper = is_dirty || has_dirty_container;
30052990

30062991
let aggregation_number = get_aggregation_number(&task);
@@ -3025,7 +3010,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
30253010
for upper_id in uppers {
30263011
let task = ctx.task(task_id, TaskDataCategory::All);
30273012
let in_upper = get!(task, AggregatedDirtyContainer { task: task_id })
3028-
.is_some_and(|dirty| dirty.get(self.session_id) > 0);
3013+
.is_some_and(|&dirty| dirty > 0);
30293014
if !in_upper {
30303015
panic!(
30313016
"Task {} ({}) is dirty, but is not listed in the upper task {} \

‎turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs‎

Lines changed: 100 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ use crate::{
2828
backend::{
2929
TaskDataCategory, get_mut, get_mut_or_insert_with,
3030
operation::{ExecuteContext, Operation, TaskGuard, invalidate::make_task_dirty},
31-
storage::{count, get, get_many, iter_many, remove, update, update_count},
31+
storage::{
32+
count, get, get_many, iter_many, remove, update, update_count, update_count_and_get,
33+
},
3234
},
3335
data::{
3436
ActivenessState, AggregationNumber, CachedDataItem, CachedDataItemKey, CachedDataItemType,
@@ -271,23 +273,23 @@ impl AggregatedDataUpdate {
271273
collectibles_update.push((collectible, 1));
272274
}
273275
}
274-
if let Some((dirtyness, clean_in_session)) = task.dirtyness_and_session() {
275-
dirty_container_count.update_with_dirtyness_and_session(dirtyness, clean_in_session);
276+
let mut dirty_count = dirty_container_count.count;
277+
let mut current_session_clean_count =
278+
dirty_container_count.current_session_clean(current_session_id);
279+
let (dirty, current_session_clean) = task.dirty(current_session_id);
280+
if dirty {
281+
dirty_count += 1;
282+
}
283+
if current_session_clean {
284+
current_session_clean_count += 1;
276285
}
277286

278287
let mut result = Self::new().collectibles_update(collectibles_update);
279-
if !dirty_container_count.is_zero() {
288+
if dirty_count > 0 {
280289
result = result.dirty_container_update(
281290
task.id(),
282-
if dirty_container_count.count > 0 {
283-
1
284-
} else {
285-
0
286-
},
287-
if dirty_container_count.count > 0
288-
&& dirty_container_count.current_session_clean(current_session_id)
289-
>= dirty_container_count.count
290-
{
291+
if dirty_count > 0 { 1 } else { 0 },
292+
if dirty_count > 0 && dirty_count - current_session_clean_count <= 0 {
291293
1
292294
} else {
293295
0
@@ -322,39 +324,105 @@ impl AggregatedDataUpdate {
322324
should_track_activeness: bool,
323325
queue: &mut AggregationUpdateQueue,
324326
) -> AggregatedDataUpdate {
327+
fn before_after_to_diff_value(before: bool, after: bool) -> i32 {
328+
match (before, after) {
329+
(true, false) => -1,
330+
(false, true) => 1,
331+
_ => 0,
332+
}
333+
}
334+
325335
let Self {
326336
dirty_container_update,
327337
collectibles_update,
328338
} = self;
329339
let mut result = Self::default();
330-
if let &Some((dirty_container_id, count, session_dependent_clean_update)) =
340+
if let &Some((dirty_container_id, count, current_session_clean_update)) =
331341
dirty_container_update
332342
{
333343
if should_track_activeness {
334344
// When a dirty container count is increased and the task is considered as active
335345
// we need to schedule the dirty tasks in the new dirty container
336-
let current_session_update = count - *session_dependent_clean_update;
346+
let current_session_update = count - *current_session_clean_update;
337347
if current_session_update > 0 && task.has_key(&CachedDataItemKey::Activeness {}) {
338348
queue.push_find_and_schedule_dirty(dirty_container_id)
339349
}
340350
}
341351

342-
let mut aggregated_update = DirtyContainerCount::default();
343-
update!(
344-
task,
345-
AggregatedDirtyContainer {
346-
task: dirty_container_id
347-
},
348-
|old: Option<DirtyContainerCount>| {
349-
let mut new = old.unwrap_or_default();
350-
aggregated_update =
351-
new.update_count(&DirtyContainerCount::from_current_session_clean(
352-
count,
353-
current_session_id,
354-
*session_dependent_clean_update,
355-
));
356-
(!new.is_zero()).then_some(new)
357-
}
352+
// Update AggregatedDirtyContainer and compute aggregated update
353+
let mut dirty_container_count_update = 0;
354+
let old_dirty_single_container_count;
355+
let new_dirty_single_container_count;
356+
if count != 0 {
357+
new_dirty_single_container_count = update_count_and_get!(
358+
task,
359+
AggregatedDirtyContainer {
360+
task: dirty_container_id
361+
},
362+
count
363+
);
364+
old_dirty_single_container_count = new_dirty_single_container_count - count;
365+
dirty_container_count_update = before_after_to_diff_value(
366+
old_dirty_single_container_count > 0,
367+
new_dirty_single_container_count > 0,
368+
);
369+
} else {
370+
new_dirty_single_container_count = get!(
371+
task,
372+
AggregatedDirtyContainer {
373+
task: dirty_container_id
374+
}
375+
)
376+
.copied()
377+
.unwrap_or_default();
378+
old_dirty_single_container_count = new_dirty_single_container_count;
379+
}
380+
381+
// Update AggregatedSessionDependentCleanContainer
382+
let old_single_container_current_session_clean_count;
383+
let new_single_container_current_session_clean_count;
384+
if *current_session_clean_update != 0 {
385+
new_single_container_current_session_clean_count = update_count_and_get!(
386+
task,
387+
AggregatedSessionDependentCleanContainer {
388+
task: dirty_container_id,
389+
session_id: current_session_id
390+
},
391+
*current_session_clean_update
392+
);
393+
old_single_container_current_session_clean_count =
394+
new_single_container_current_session_clean_count
395+
- *current_session_clean_update;
396+
} else {
397+
new_single_container_current_session_clean_count = get!(
398+
task,
399+
AggregatedSessionDependentCleanContainer {
400+
task: dirty_container_id,
401+
session_id: current_session_id
402+
}
403+
)
404+
.copied()
405+
.unwrap_or_default();
406+
old_single_container_current_session_clean_count =
407+
new_single_container_current_session_clean_count;
408+
}
409+
410+
// compute aggregated update
411+
let was_single_container_clean = old_dirty_single_container_count > 0
412+
&& old_dirty_single_container_count
413+
- old_single_container_current_session_clean_count
414+
<= 0;
415+
let is_single_container_clean = new_dirty_single_container_count > 0
416+
&& new_dirty_single_container_count
417+
- new_single_container_current_session_clean_count
418+
<= 0;
419+
let current_session_clean_update =
420+
before_after_to_diff_value(was_single_container_clean, is_single_container_clean);
421+
422+
let aggregated_update = DirtyContainerCount::from_current_session_clean(
423+
dirty_container_count_update,
424+
current_session_id,
425+
current_session_clean_update,
358426
);
359427

360428
if !aggregated_update.is_zero() {
@@ -1273,7 +1341,7 @@ impl AggregationUpdateQueue {
12731341
// this would already be scheduled by the `Activeness`
12741342
let is_active_until_clean = get!(task, Activeness).is_some_and(|a| a.active_until_clean);
12751343
if !is_active_until_clean {
1276-
let mut dirty_containers = iter_many!(task, AggregatedDirtyContainer { task } count if count.get(session_id) > 0 => task).peekable();
1344+
let mut dirty_containers = task.dirty_containers(session_id).peekable();
12771345
let is_empty = dirty_containers.peek().is_none();
12781346
if !is_empty || dirty {
12791347
self.extend_find_and_schedule_dirty(dirty_containers);

‎turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use crate::{
2525
backing_storage::BackingStorage,
2626
data::{
2727
CachedDataItem, CachedDataItemKey, CachedDataItemType, CachedDataItemValue,
28-
CachedDataItemValueRef, CachedDataItemValueRefMut, Dirtyness,
28+
CachedDataItemValueRef, CachedDataItemValueRefMut, DirtyContainerCount, Dirtyness,
2929
},
3030
};
3131

@@ -430,6 +430,50 @@ pub trait TaskGuard: Debug {
430430
)),
431431
}
432432
}
433+
/// Returns (is_dirty, is_clean_in_current_session)
434+
fn dirty(&self, session_id: SessionId) -> (bool, bool) {
435+
match get!(self, Dirty) {
436+
None => (false, false),
437+
Some(Dirtyness::Dirty) => (true, false),
438+
Some(Dirtyness::SessionDependent) => (
439+
true,
440+
get!(self, CleanInSession).copied() == Some(session_id),
441+
),
442+
}
443+
}
444+
fn dirty_containers(&self, session_id: SessionId) -> impl Iterator<Item = TaskId> {
445+
self.dirty_containers_with_count(session_id)
446+
.map(|(task_id, _)| task_id)
447+
}
448+
fn dirty_containers_with_count(
449+
&self,
450+
session_id: SessionId,
451+
) -> impl Iterator<Item = (TaskId, i32)> {
452+
iter_many!(self, AggregatedDirtyContainer { task } count => (task, *count)).filter(
453+
move |&(task_id, count)| {
454+
if count > 0 {
455+
let clean_count = get!(
456+
self,
457+
AggregatedSessionDependentCleanContainer {
458+
task: task_id,
459+
session_id
460+
}
461+
)
462+
.copied()
463+
.unwrap_or_default();
464+
count > clean_count
465+
} else {
466+
false
467+
}
468+
},
469+
)
470+
}
471+
472+
fn dirty_container_count(&self) -> DirtyContainerCount {
473+
get!(self, AggregatedDirtyContainerCount)
474+
.cloned()
475+
.unwrap_or_default()
476+
}
433477
}
434478

435479
pub struct TaskGuardImpl<'a, B: BackingStorage> {

‎turbopack/crates/turbo-tasks-backend/src/backend/storage.rs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,38 @@ macro_rules! update_count {
10961096
};
10971097
}
10981098

1099+
macro_rules! update_count_and_get {
1100+
($task:ident, $key:ident $input:tt, -$update:expr) => {{
1101+
let update = $update;
1102+
let mut new = 0;
1103+
$crate::backend::storage::update!($task, $key $input, |old: Option<_>| {
1104+
let old = old.unwrap_or(0);
1105+
new = old - update;
1106+
(new != 0).then_some(new)
1107+
});
1108+
new
1109+
}};
1110+
($task:ident, $key:ident $input:tt, $update:expr) => {
1111+
match $update {
1112+
update => {
1113+
let mut new = 0;
1114+
$crate::backend::storage::update!($task, $key $input, |old: Option<_>| {
1115+
let old = old.unwrap_or(0);
1116+
new = old + update;
1117+
(new != 0).then_some(new)
1118+
});
1119+
new
1120+
}
1121+
}
1122+
};
1123+
($task:ident, $key:ident, -$update:expr) => {
1124+
$crate::backend::storage::update_count_and_get!($task, $key {}, -$update)
1125+
};
1126+
($task:ident, $key:ident, $update:expr) => {
1127+
$crate::backend::storage::update_count_and_get!($task, $key {}, $update)
1128+
};
1129+
}
1130+
10991131
macro_rules! remove {
11001132
($task:ident, $key:ident $input:tt) => {{
11011133
#[allow(unused_imports)]
@@ -1122,6 +1154,7 @@ pub(crate) use iter_many;
11221154
pub(crate) use remove;
11231155
pub(crate) use update;
11241156
pub(crate) use update_count;
1157+
pub(crate) use update_count_and_get;
11251158

11261159
pub struct SnapshotGuard<'l> {
11271160
storage: &'l Storage,

0 commit comments

Comments
 (0)