Skip to content

Commit 3dcbb84

Browse files
authored
Turbopack: skip invalidating a task on cell/output change when the dependency is outdated (#84551)
Relanding #84376 Reverts #84526
1 parent 3eb23fe commit 3dcbb84

File tree

10 files changed

+199
-17
lines changed

10 files changed

+199
-17
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
@import 'tailwindcss';
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import './globals.css'
2+
3+
import { ReactNode } from 'react'
4+
export default function Root({ children }: { children: ReactNode }) {
5+
return (
6+
<html>
7+
<body>{children}</body>
8+
</html>
9+
)
10+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function Page() {
2+
return <p>hello world</p>
3+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/**
2+
* @type {import('next').NextConfig}
3+
*/
4+
const nextConfig = {}
5+
6+
module.exports = nextConfig
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { nextTestSetup } from 'e2e-utils'
2+
import { retry } from 'next-test-utils'
3+
4+
describe('no-double-tailwind-execution', () => {
5+
const { next, isNextDev, skipped } = nextTestSetup({
6+
files: __dirname,
7+
skipDeployment: true,
8+
dependencies: {
9+
'@tailwindcss/postcss': '^4',
10+
tailwindcss: '^4',
11+
},
12+
env: {
13+
DEBUG: 'tailwindcss',
14+
...process.env,
15+
},
16+
})
17+
18+
if (skipped) {
19+
return
20+
}
21+
22+
it('should run tailwind only once initially and per change', async () => {
23+
const browser = await next.browser('/')
24+
expect(await browser.elementByCss('p').text()).toBe('hello world')
25+
26+
if (isNextDev) {
27+
const filePath = 'app/page.tsx'
28+
const origContent = await next.readFile(filePath)
29+
let getOutput = next.getCliOutputFromHere()
30+
await next.patchFile(
31+
filePath,
32+
origContent.replace('hello world', 'hello hmr'),
33+
async () => {
34+
await retry(async () => {
35+
expect(await browser.elementByCss('p').text()).toBe('hello hmr')
36+
let tailwindProcessingCount = [
37+
...getOutput().matchAll(
38+
/\[@tailwindcss\/postcss\] app\/globals.css/g
39+
),
40+
].length
41+
expect(tailwindProcessingCount).toBe(1)
42+
})
43+
}
44+
)
45+
}
46+
let tailwindProcessingCount = [
47+
...next.cliOutput.matchAll(/\[@tailwindcss\/postcss\] app\/globals.css/g),
48+
].length
49+
if (isNextDev) {
50+
expect(tailwindProcessingCount).toBe(3) // dev: initial + hmr + hmr (revert)
51+
} else {
52+
expect(tailwindProcessingCount).toBe(1) // build
53+
}
54+
})
55+
})
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const config = {
2+
plugins: ['@tailwindcss/postcss'],
3+
}
4+
5+
export default config

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,7 @@ pub enum AnyOperation {
601601
ConnectChild(connect_child::ConnectChildOperation),
602602
Invalidate(invalidate::InvalidateOperation),
603603
UpdateOutput(update_output::UpdateOutputOperation),
604+
UpdateCell(update_cell::UpdateCellOperation),
604605
CleanupOldEdges(cleanup_old_edges::CleanupOldEdgesOperation),
605606
AggregationUpdate(aggregation_update::AggregationUpdateQueue),
606607
Nested(Vec<AnyOperation>),
@@ -612,6 +613,7 @@ impl AnyOperation {
612613
AnyOperation::ConnectChild(op) => op.execute(ctx),
613614
AnyOperation::Invalidate(op) => op.execute(ctx),
614615
AnyOperation::UpdateOutput(op) => op.execute(ctx),
616+
AnyOperation::UpdateCell(op) => op.execute(ctx),
615617
AnyOperation::CleanupOldEdges(op) => op.execute(ctx),
616618
AnyOperation::AggregationUpdate(op) => op.execute(ctx),
617619
AnyOperation::Nested(ops) => {
@@ -626,6 +628,7 @@ impl AnyOperation {
626628
impl_operation!(ConnectChild connect_child::ConnectChildOperation);
627629
impl_operation!(Invalidate invalidate::InvalidateOperation);
628630
impl_operation!(UpdateOutput update_output::UpdateOutputOperation);
631+
impl_operation!(UpdateCell update_cell::UpdateCellOperation);
629632
impl_operation!(CleanupOldEdges cleanup_old_edges::CleanupOldEdgesOperation);
630633
impl_operation!(AggregationUpdate aggregation_update::AggregationUpdateQueue);
631634

@@ -639,6 +642,5 @@ pub use self::{
639642
cleanup_old_edges::OutdatedEdge,
640643
connect_children::connect_children,
641644
prepare_new_children::prepare_new_children,
642-
update_cell::UpdateCellOperation,
643645
update_collectible::UpdateCollectibleOperation,
644646
};

turbopack/crates/turbo-tasks-backend/src/backend/operation/update_cell.rs

Lines changed: 92 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,37 @@
1+
use std::mem::take;
2+
3+
use serde::{Deserialize, Serialize};
4+
use smallvec::SmallVec;
15
use turbo_tasks::{CellId, TaskId, backend::CellContent};
26

37
#[cfg(feature = "trace_task_dirty")]
48
use crate::backend::operation::invalidate::TaskDirtyCause;
59
use crate::{
610
backend::{
711
TaskDataCategory,
8-
operation::{ExecuteContext, InvalidateOperation, TaskGuard},
12+
operation::{
13+
AggregationUpdateQueue, ExecuteContext, Operation, TaskGuard,
14+
invalidate::make_task_dirty_internal,
15+
},
916
storage::{get_many, remove},
1017
},
11-
data::{CachedDataItem, CachedDataItemKey},
18+
data::{CachedDataItem, CachedDataItemKey, CellRef},
1219
};
1320

14-
pub struct UpdateCellOperation;
21+
#[derive(Serialize, Deserialize, Clone, Default)]
22+
#[allow(clippy::large_enum_variant)]
23+
pub enum UpdateCellOperation {
24+
InvalidateWhenCellDependency {
25+
cell_ref: CellRef,
26+
dependent_tasks: SmallVec<[TaskId; 4]>,
27+
queue: AggregationUpdateQueue,
28+
},
29+
AggregationUpdate {
30+
queue: AggregationUpdateQueue,
31+
},
32+
#[default]
33+
Done,
34+
}
1535

1636
impl UpdateCellOperation {
1737
pub fn run(task_id: TaskId, cell: CellId, content: CellContent, mut ctx: impl ExecuteContext) {
@@ -39,7 +59,7 @@ impl UpdateCellOperation {
3959
// This is a hack for the streaming hack. Stateful tasks are never recomputed, so this forces invalidation for them in case of this hack.
4060
task.has_key(&CachedDataItemKey::Stateful {}))
4161
{
42-
let dependent = get_many!(
62+
let dependent_tasks = get_many!(
4363
task,
4464
CellDependent { cell: dependent_cell, task }
4565
if dependent_cell == cell
@@ -49,17 +69,78 @@ impl UpdateCellOperation {
4969
drop(task);
5070
drop(old_content);
5171

52-
InvalidateOperation::run(
53-
dependent,
54-
#[cfg(feature = "trace_task_dirty")]
55-
TaskDirtyCause::CellChange {
56-
value_type: cell.type_id,
72+
UpdateCellOperation::InvalidateWhenCellDependency {
73+
cell_ref: CellRef {
74+
task: task_id,
75+
cell,
5776
},
58-
ctx,
59-
);
77+
dependent_tasks,
78+
queue: AggregationUpdateQueue::new(),
79+
}
80+
.execute(&mut ctx);
6081
} else {
6182
drop(task);
6283
drop(old_content);
6384
}
6485
}
6586
}
87+
88+
impl Operation for UpdateCellOperation {
89+
fn execute(mut self, ctx: &mut impl ExecuteContext) {
90+
loop {
91+
ctx.operation_suspend_point(&self);
92+
match self {
93+
UpdateCellOperation::InvalidateWhenCellDependency {
94+
cell_ref,
95+
ref mut dependent_tasks,
96+
ref mut queue,
97+
} => {
98+
if let Some(dependent_task_id) = dependent_tasks.pop() {
99+
if ctx.is_once_task(dependent_task_id) {
100+
// once tasks are never invalidated
101+
continue;
102+
}
103+
let dependent = ctx.task(dependent_task_id, TaskDataCategory::All);
104+
if dependent.has_key(&CachedDataItemKey::OutdatedCellDependency {
105+
target: cell_ref,
106+
}) {
107+
// cell dependency is outdated, so it hasn't read the cell yet
108+
// and doesn't need to be invalidated
109+
continue;
110+
}
111+
if !dependent
112+
.has_key(&CachedDataItemKey::CellDependency { target: cell_ref })
113+
{
114+
// cell dependency has been removed, so the task doesn't depend on the
115+
// cell anymore and doesn't need to be
116+
// invalidated
117+
continue;
118+
}
119+
make_task_dirty_internal(
120+
dependent,
121+
dependent_task_id,
122+
true,
123+
#[cfg(feature = "trace_task_dirty")]
124+
TaskDirtyCause::CellChange {
125+
value_type: cell_ref.cell.type_id,
126+
},
127+
queue,
128+
ctx,
129+
);
130+
}
131+
if dependent_tasks.is_empty() {
132+
self = UpdateCellOperation::AggregationUpdate { queue: take(queue) };
133+
}
134+
}
135+
UpdateCellOperation::AggregationUpdate { ref mut queue } => {
136+
if queue.process(ctx) {
137+
self = UpdateCellOperation::Done
138+
}
139+
}
140+
UpdateCellOperation::Done => {
141+
return;
142+
}
143+
}
144+
}
145+
}
146+
}

turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::{
1212
TaskDataCategory,
1313
operation::{
1414
AggregationUpdateQueue, ExecuteContext, Operation, TaskGuard,
15-
invalidate::{make_task_dirty, make_task_dirty_internal},
15+
invalidate::make_task_dirty_internal,
1616
},
1717
storage::{get, get_many},
1818
},
@@ -25,7 +25,6 @@ use crate::{
2525
#[derive(Serialize, Deserialize, Clone, Default)]
2626
pub enum UpdateOutputOperation {
2727
MakeDependentTasksDirty {
28-
#[cfg(feature = "trace_task_dirty")]
2928
task_id: TaskId,
3029
dependent_tasks: SmallVec<[TaskId; 4]>,
3130
children: SmallVec<[TaskId; 4]>,
@@ -132,7 +131,6 @@ impl UpdateOutputOperation {
132131
}
133132

134133
UpdateOutputOperation::MakeDependentTasksDirty {
135-
#[cfg(feature = "trace_task_dirty")]
136134
task_id,
137135
dependent_tasks,
138136
children,
@@ -148,15 +146,35 @@ impl Operation for UpdateOutputOperation {
148146
ctx.operation_suspend_point(&self);
149147
match self {
150148
UpdateOutputOperation::MakeDependentTasksDirty {
151-
#[cfg(feature = "trace_task_dirty")]
152149
task_id,
153150
ref mut dependent_tasks,
154151
ref mut children,
155152
ref mut queue,
156153
} => {
157154
if let Some(dependent_task_id) = dependent_tasks.pop() {
158-
make_task_dirty(
155+
if ctx.is_once_task(dependent_task_id) {
156+
// once tasks are never invalidated
157+
continue;
158+
}
159+
let dependent = ctx.task(dependent_task_id, TaskDataCategory::All);
160+
if dependent.has_key(&CachedDataItemKey::OutdatedOutputDependency {
161+
target: task_id,
162+
}) {
163+
// output dependency is outdated, so it hasn't read the output yet
164+
// and doesn't need to be invalidated
165+
continue;
166+
}
167+
if !dependent
168+
.has_key(&CachedDataItemKey::OutputDependency { target: task_id })
169+
{
170+
// output dependency has been removed, so the task doesn't depend on the
171+
// output anymore and doesn't need to be invalidated
172+
continue;
173+
}
174+
make_task_dirty_internal(
175+
dependent,
159176
dependent_task_id,
177+
true,
160178
#[cfg(feature = "trace_task_dirty")]
161179
TaskDirtyCause::OutputChange { task_id },
162180
queue,

0 commit comments

Comments
 (0)