Skip to content

Commit c4e6eb8

Browse files
sokrasokra
committed
Turbopack: fix cell not found bug
incorrect accessed category led to empty list of dependencies after restoring from persistent caching Some debug asserts were missing to validate the correct category on read access via `iter()` or `extract_if()`
1 parent 47b0387 commit c4e6eb8

File tree

7 files changed

+213
-64
lines changed

7 files changed

+213
-64
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
12121212
};
12131213
{
12141214
let mut ctx = self.execute_context(turbo_tasks);
1215-
let mut task = ctx.task(task_id, TaskDataCategory::Data);
1215+
let mut task = ctx.task(task_id, TaskDataCategory::All);
12161216
let in_progress = remove!(task, InProgress)?;
12171217
let InProgressState::Scheduled { done_event } = in_progress else {
12181218
task.add_new(CachedDataItem::InProgress { value: in_progress });

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

Lines changed: 141 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,7 +1003,12 @@ impl AggregationUpdateQueue {
10031003
#[cfg(feature = "trace_aggregation_update")]
10041004
let _span = trace_span!("process balance edge").entered();
10051005

1006-
let (mut upper, mut task) = ctx.task_pair(upper_id, task_id, TaskDataCategory::Meta);
1006+
let (mut upper, mut task) = ctx.task_pair(
1007+
upper_id,
1008+
task_id,
1009+
// For performance reasons this should stay `Meta` and not `All`
1010+
TaskDataCategory::Meta,
1011+
);
10071012
let upper_aggregation_number = get_aggregation_number(&upper);
10081013
let task_aggregation_number = get_aggregation_number(&task);
10091014

@@ -1187,7 +1192,11 @@ impl AggregationUpdateQueue {
11871192
name = ctx.get_task_description(task_id)
11881193
)
11891194
.entered();
1190-
let task = ctx.task(task_id, TaskDataCategory::Meta);
1195+
let task = ctx.task(
1196+
task_id,
1197+
// For performance reasons this should stay `Meta` and not `All`
1198+
TaskDataCategory::Meta,
1199+
);
11911200
self.find_and_schedule_dirty_internal(task_id, task, ctx);
11921201
}
11931202

@@ -1234,7 +1243,11 @@ impl AggregationUpdateQueue {
12341243
update: AggregatedDataUpdate,
12351244
) {
12361245
for upper_id in upper_ids {
1237-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1246+
let mut upper = ctx.task(
1247+
upper_id,
1248+
// For performance reasons this should stay `Meta` and not `All`
1249+
TaskDataCategory::Meta,
1250+
);
12381251
let diff = update.apply(
12391252
&mut upper,
12401253
ctx.session_id(),
@@ -1265,7 +1278,11 @@ impl AggregationUpdateQueue {
12651278
#[cfg(feature = "trace_aggregation_update")]
12661279
let _span = trace_span!("lost follower (n uppers)", uppers = upper_ids.len()).entered();
12671280

1268-
let mut follower = ctx.task(lost_follower_id, TaskDataCategory::Meta);
1281+
let mut follower = ctx.task(
1282+
lost_follower_id,
1283+
// For performance reasons this should stay `Meta` and not `All`
1284+
TaskDataCategory::Meta,
1285+
);
12691286
let mut follower_in_upper_ids = Vec::new();
12701287
let mut persistent_uppers = 0;
12711288
upper_ids.retain(|&mut upper_id| {
@@ -1298,7 +1315,11 @@ impl AggregationUpdateQueue {
12981315
if !data.is_empty() {
12991316
for upper_id in upper_ids.iter() {
13001317
// remove data from upper
1301-
let mut upper = ctx.task(*upper_id, TaskDataCategory::Meta);
1318+
let mut upper = ctx.task(
1319+
*upper_id,
1320+
// For performance reasons this should stay `Meta` and not `All`
1321+
TaskDataCategory::Meta,
1322+
);
13021323
let diff = data.apply(
13031324
&mut upper,
13041325
ctx.session_id(),
@@ -1331,7 +1352,11 @@ impl AggregationUpdateQueue {
13311352
}
13321353

13331354
for upper_id in follower_in_upper_ids {
1334-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1355+
let mut upper = ctx.task(
1356+
upper_id,
1357+
// For performance reasons this should stay `Meta` and not `All`
1358+
TaskDataCategory::Meta,
1359+
);
13351360
if update_count!(
13361361
upper,
13371362
Follower {
@@ -1379,7 +1404,11 @@ impl AggregationUpdateQueue {
13791404
.entered();
13801405

13811406
lost_follower_ids.retain(|lost_follower_id| {
1382-
let mut follower = ctx.task(*lost_follower_id, TaskDataCategory::Meta);
1407+
let mut follower = ctx.task(
1408+
*lost_follower_id,
1409+
// For performance reasons this should stay `Meta` and not `All`
1410+
TaskDataCategory::Meta,
1411+
);
13831412
let mut remove_upper = false;
13841413
let mut follower_in_upper = false;
13851414
update!(follower, Upper { task: upper_id }, |old| {
@@ -1404,7 +1433,11 @@ impl AggregationUpdateQueue {
14041433

14051434
if !data.is_empty() {
14061435
// remove data from upper
1407-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1436+
let mut upper = ctx.task(
1437+
upper_id,
1438+
// For performance reasons this should stay `Meta` and not `All`
1439+
TaskDataCategory::Meta,
1440+
);
14081441
let diff = data.apply(
14091442
&mut upper,
14101443
ctx.session_id(),
@@ -1434,7 +1467,11 @@ impl AggregationUpdateQueue {
14341467
follower_in_upper
14351468
});
14361469
for lost_follower_id in lost_follower_ids {
1437-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1470+
let mut upper = ctx.task(
1471+
upper_id,
1472+
// For performance reasons this should stay `Meta` and not `All`
1473+
TaskDataCategory::Meta,
1474+
);
14381475
if update_count!(
14391476
upper,
14401477
Follower {
@@ -1479,14 +1516,22 @@ impl AggregationUpdateQueue {
14791516
trace_span!("process new follower (n uppers)", uppers = upper_ids.len()).entered();
14801517

14811518
let follower_aggregation_number = {
1482-
let follower = ctx.task(new_follower_id, TaskDataCategory::Meta);
1519+
let follower = ctx.task(
1520+
new_follower_id,
1521+
// For performance reasons this should stay `Meta` and not `All`
1522+
TaskDataCategory::Meta,
1523+
);
14831524
get_aggregation_number(&follower)
14841525
};
14851526
let mut upper_upper_ids_with_new_follower = SmallVec::new();
14861527
let mut tasks_for_which_increment_active_count = SmallVec::new();
14871528
let mut is_active = false;
14881529
swap_retain(&mut upper_ids, |&mut upper_id| {
1489-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1530+
let mut upper = ctx.task(
1531+
upper_id,
1532+
// For performance reasons this should stay `Meta` and not `All`
1533+
TaskDataCategory::Meta,
1534+
);
14901535
// decide if it should be an inner or follower
14911536
let upper_aggregation_number = get_aggregation_number(&upper);
14921537

@@ -1539,7 +1584,11 @@ impl AggregationUpdateQueue {
15391584
});
15401585

15411586
if !upper_ids.is_empty() {
1542-
let mut follower = ctx.task(new_follower_id, TaskDataCategory::Meta);
1587+
let mut follower = ctx.task(
1588+
new_follower_id,
1589+
// For performance reasons this should stay `Meta` and not `All`
1590+
TaskDataCategory::Meta,
1591+
);
15431592
let mut uppers_count: Option<usize> = None;
15441593
let mut persistent_uppers = 0;
15451594
swap_retain(&mut upper_ids, |&mut upper_id| {
@@ -1579,7 +1628,11 @@ impl AggregationUpdateQueue {
15791628
if has_data || !is_active {
15801629
for upper_id in upper_ids.iter() {
15811630
// add data to upper
1582-
let mut upper = ctx.task(*upper_id, TaskDataCategory::Meta);
1631+
let mut upper = ctx.task(
1632+
*upper_id,
1633+
// For performance reasons this should stay `Meta` and not `All`
1634+
TaskDataCategory::Meta,
1635+
);
15831636
if has_data {
15841637
let diff = data.apply(
15851638
&mut upper,
@@ -1654,7 +1707,11 @@ impl AggregationUpdateQueue {
16541707
let mut followers_with_aggregation_number = new_follower_ids
16551708
.into_iter()
16561709
.map(|new_follower_id| {
1657-
let follower = ctx.task(new_follower_id, TaskDataCategory::Meta);
1710+
let follower = ctx.task(
1711+
new_follower_id,
1712+
// For performance reasons this should stay `Meta` and not `All`
1713+
TaskDataCategory::Meta,
1714+
);
16581715
(new_follower_id, get_aggregation_number(&follower))
16591716
})
16601717
.collect::<SmallVec<[_; 4]>>();
@@ -1665,7 +1722,11 @@ impl AggregationUpdateQueue {
16651722
let mut upper_upper_ids_for_new_followers = SmallVec::new();
16661723
let upper_aggregation_number;
16671724
{
1668-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1725+
let mut upper = ctx.task(
1726+
upper_id,
1727+
// For performance reasons this should stay `Meta` and not `All`
1728+
TaskDataCategory::Meta,
1729+
);
16691730
if ctx.should_track_activeness() {
16701731
let activeness_state = get!(upper, Activeness);
16711732
is_active = activeness_state.is_some();
@@ -1719,7 +1780,11 @@ impl AggregationUpdateQueue {
17191780
swap_retain(
17201781
&mut inner_tasks_with_aggregation_number,
17211782
|&mut (inner_id, _)| {
1722-
let mut inner = ctx.task(inner_id, TaskDataCategory::Meta);
1783+
let mut inner = ctx.task(
1784+
inner_id,
1785+
// For performance reasons this should stay `Meta` and not `All`
1786+
TaskDataCategory::Meta,
1787+
);
17231788
if update_count!(inner, Upper { task: upper_id }, 1) {
17241789
if count!(inner, Upper).is_power_of_two() {
17251790
self.push_optimize_task(inner_id);
@@ -1763,7 +1828,11 @@ impl AggregationUpdateQueue {
17631828
}
17641829
if !upper_data_updates.is_empty() {
17651830
// add data to upper
1766-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1831+
let mut upper = ctx.task(
1832+
upper_id,
1833+
// For performance reasons this should stay `Meta` and not `All`
1834+
TaskDataCategory::Meta,
1835+
);
17671836
let diffs = upper_data_updates
17681837
.into_iter()
17691838
.filter_map(|data| {
@@ -1804,7 +1873,11 @@ impl AggregationUpdateQueue {
18041873
if !is_active {
18051874
// We need to check this again, since this might have changed in the
18061875
// meantime due to race conditions
1807-
let upper = ctx.task(upper_id, TaskDataCategory::Meta);
1876+
let upper = ctx.task(
1877+
upper_id,
1878+
// For performance reasons this should stay `Meta` and not `All`
1879+
TaskDataCategory::Meta,
1880+
);
18081881
is_active = upper.has_key(&CachedDataItemKey::Activeness {});
18091882
}
18101883
if is_active {
@@ -1849,11 +1922,19 @@ impl AggregationUpdateQueue {
18491922
let _span = trace_span!("process new follower").entered();
18501923

18511924
let follower_aggregation_number = {
1852-
let follower = ctx.task(new_follower_id, TaskDataCategory::Meta);
1925+
let follower = ctx.task(
1926+
new_follower_id,
1927+
// For performance reasons this should stay `Meta` and not `All`
1928+
TaskDataCategory::Meta,
1929+
);
18531930
get_aggregation_number(&follower)
18541931
};
18551932

1856-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
1933+
let mut upper = ctx.task(
1934+
upper_id,
1935+
// For performance reasons this should stay `Meta` and not `All`
1936+
TaskDataCategory::Meta,
1937+
);
18571938
// decide if it should be an inner or follower
18581939
let upper_aggregation_number = get_aggregation_number(&upper);
18591940

@@ -1912,7 +1993,11 @@ impl AggregationUpdateQueue {
19121993
let mut is_active = upper.has_key(&CachedDataItemKey::Activeness {});
19131994
drop(upper);
19141995

1915-
let mut inner = ctx.task(new_follower_id, TaskDataCategory::Meta);
1996+
let mut inner = ctx.task(
1997+
new_follower_id,
1998+
// For performance reasons this should stay `Meta` and not `All`
1999+
TaskDataCategory::Meta,
2000+
);
19162001
if update_count!(inner, Upper { task: upper_id }, 1) {
19172002
if count!(inner, Upper).is_power_of_two() {
19182003
self.push_optimize_task(new_follower_id);
@@ -1924,7 +2009,11 @@ impl AggregationUpdateQueue {
19242009

19252010
if !data.is_empty() {
19262011
// add data to upper
1927-
let mut upper = ctx.task(upper_id, TaskDataCategory::Meta);
2012+
let mut upper = ctx.task(
2013+
upper_id,
2014+
// For performance reasons this should stay `Meta` and not `All`
2015+
TaskDataCategory::Meta,
2016+
);
19282017
let diff = data.apply(
19292018
&mut upper,
19302019
ctx.session_id(),
@@ -1949,7 +2038,11 @@ impl AggregationUpdateQueue {
19492038
});
19502039
}
19512040
if !is_active {
1952-
let upper = ctx.task(upper_id, TaskDataCategory::Meta);
2041+
let upper = ctx.task(
2042+
upper_id,
2043+
// For performance reasons this should stay `Meta` and not `All`
2044+
TaskDataCategory::Meta,
2045+
);
19532046
is_active = upper.has_key(&CachedDataItemKey::Activeness {});
19542047
}
19552048
if is_active {
@@ -1966,7 +2059,11 @@ impl AggregationUpdateQueue {
19662059
#[cfg(feature = "trace_aggregation_update")]
19672060
let _span = trace_span!("decrease active count").entered();
19682061

1969-
let mut task = ctx.task(task_id, TaskDataCategory::Meta);
2062+
let mut task = ctx.task(
2063+
task_id,
2064+
// For performance reasons this should stay `Meta` and not `All`
2065+
TaskDataCategory::Meta,
2066+
);
19702067
let state = get_mut_or_insert_with!(task, Activeness, || ActivenessState::new(task_id));
19712068
let is_zero = state.decrement_active_counter();
19722069
let is_empty = state.is_empty();
@@ -1991,7 +2088,11 @@ impl AggregationUpdateQueue {
19912088
#[cfg(feature = "trace_aggregation_update")]
19922089
let _span = trace_span!("increase active count").entered();
19932090

1994-
let mut task = ctx.task(task_id, TaskDataCategory::Meta);
2091+
let mut task = ctx.task(
2092+
task_id,
2093+
// For performance reasons this should stay `Meta` and not `All`
2094+
TaskDataCategory::Meta,
2095+
);
19952096
let state = get_mut_or_insert_with!(task, Activeness, || ActivenessState::new(task_id));
19962097
let is_positive_now = state.increment_active_counter();
19972098
let is_empty = state.is_empty();
@@ -2023,7 +2124,11 @@ impl AggregationUpdateQueue {
20232124
let _span =
20242125
trace_span!("check update aggregation number", base_aggregation_number).entered();
20252126

2026-
let mut task = ctx.task(task_id, TaskDataCategory::Meta);
2127+
let mut task = ctx.task(
2128+
task_id,
2129+
// For performance reasons this should stay `Meta` and not `All`
2130+
TaskDataCategory::Meta,
2131+
);
20272132
let current = get!(task, AggregationNumber).copied().unwrap_or_default();
20282133
let old = current.effective;
20292134
// The base aggregation number can only increase
@@ -2116,7 +2221,11 @@ impl AggregationUpdateQueue {
21162221
#[cfg(feature = "trace_aggregation_update")]
21172222
let _span = trace_span!("check optimize").entered();
21182223

2119-
let task = ctx.task(task_id, TaskDataCategory::All);
2224+
let task = ctx.task(
2225+
task_id,
2226+
// For performance reasons this should stay `Meta` and not `All`
2227+
TaskDataCategory::Meta,
2228+
);
21202229
let aggregation_number = get!(task, AggregationNumber).copied().unwrap_or_default();
21212230
if is_root_node(aggregation_number.effective) {
21222231
return;
@@ -2164,7 +2273,11 @@ impl AggregationUpdateQueue {
21642273
if task_id.is_transient() {
21652274
return None;
21662275
}
2167-
let task = ctx.task(task_id, TaskDataCategory::Meta);
2276+
let task = ctx.task(
2277+
task_id,
2278+
// For performance reasons this should stay `Meta` and not `All`
2279+
TaskDataCategory::Meta,
2280+
);
21682281
let n = get_aggregation_number(&task);
21692282
if is_root_node(n) {
21702283
root_uppers += 1;

0 commit comments

Comments
 (0)