Skip to content

Commit 3af0bd6

Browse files
committed
Backport: fix(turbo-tasks): Store persistence of wrapped task on RawVc::LocalOutput (#78488)
Using the parent task id for determining the persistence of a `RawVc::LocalOutput` is (sometimes) wrong. If we have a transient task calling a persistent task, the wrapped `LocalOutput` should be persistent. We just store the persistence here, and not the task id of the wrapped `LocalOutput`, because when this is constructed, no non-local task id has been reserved yet. We don't really need the full task id though: just the persistence/transient bit is sufficient. After this PR stack, the parent task id isn't really used for anything anymore, so we can get rid of it (TODO). Tested using the repro here: https://vercel.slack.com/archives/C046HAU4H7F/p1744792143917369?thread_ts=1744792143.917369
1 parent b40778b commit 3af0bd6

File tree

7 files changed

+82
-22
lines changed

7 files changed

+82
-22
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ impl UpdateOutputOperation {
9191
cell,
9292
})
9393
}
94-
Ok(Ok(RawVc::LocalOutput(_, _))) => {
95-
panic!("LocalOutput must not be output of a task");
94+
Ok(Ok(RawVc::LocalOutput(..))) => {
95+
panic!("Non-local tasks must not return a local Vc");
9696
}
9797
Ok(Err(err)) => {
9898
task.insert(CachedDataItem::Error {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../turbo-tasks-testing/tests/transient_vc.rs
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../turbo-tasks-testing/tests/transient_vc.rs
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#![feature(arbitrary_self_types)]
2+
#![feature(arbitrary_self_types_pointers)]
3+
4+
use anyhow::Result;
5+
use turbo_tasks::{TaskInput, TransientValue, Vc};
6+
use turbo_tasks_testing::{register, run_without_cache_check, Registration};
7+
8+
static REGISTRATION: Registration = register!();
9+
10+
#[tokio::test]
11+
async fn test_transient_vc() -> Result<()> {
12+
run_without_cache_check(&REGISTRATION, async {
13+
test_transient_operation(TransientValue::new(123))
14+
.read_strongly_consistent()
15+
.await?;
16+
Ok(())
17+
})
18+
.await
19+
}
20+
21+
#[turbo_tasks::function(operation)]
22+
async fn test_transient_operation(transient_arg: TransientValue<i32>) -> Result<()> {
23+
let called_with_transient = has_transient_arg(transient_arg);
24+
let called_with_persistent = has_persistent_arg(123);
25+
26+
assert!(called_with_transient.is_transient());
27+
assert!(!called_with_persistent.is_transient());
28+
assert!(has_vc_arg(called_with_transient).is_transient());
29+
assert!(!has_vc_arg(called_with_persistent).is_transient());
30+
31+
let called_with_transient_resolved = called_with_transient.to_resolved().await?;
32+
let called_with_persistent_resolved = called_with_persistent.to_resolved().await?;
33+
34+
assert!(called_with_transient_resolved.is_transient());
35+
assert!(!called_with_persistent_resolved.is_transient());
36+
assert!(has_vc_arg(*called_with_transient_resolved).is_transient());
37+
assert!(!has_vc_arg(*called_with_persistent_resolved).is_transient());
38+
39+
Ok(())
40+
}
41+
42+
#[turbo_tasks::function]
43+
fn has_transient_arg(arg: TransientValue<i32>) -> Vc<i32> {
44+
Vc::cell(*arg)
45+
}
46+
47+
#[turbo_tasks::function]
48+
fn has_persistent_arg(arg: i32) -> Vc<i32> {
49+
Vc::cell(arg)
50+
}
51+
52+
#[turbo_tasks::function]
53+
async fn has_vc_arg(arg: Vc<i32>) -> Result<Vc<i32>> {
54+
Ok(Vc::cell(*arg.await?))
55+
}

turbopack/crates/turbo-tasks/src/manager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ pub struct UpdateInfo {
306306
placeholder_for_future_fields: (),
307307
}
308308

309-
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
309+
#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Serialize, Deserialize)]
310310
pub enum TaskPersistence {
311311
/// Tasks that may be persisted across sessions using serialization.
312312
Persistent,
@@ -796,7 +796,7 @@ impl<B: Backend + 'static> TurboTasks<B> {
796796
#[cfg(not(feature = "tokio_tracing"))]
797797
tokio::task::spawn(future);
798798

799-
RawVc::LocalOutput(parent_task_id, local_task_id)
799+
RawVc::LocalOutput(parent_task_id, persistence, local_task_id)
800800
}
801801

802802
fn begin_primary_job(&self) {

turbopack/crates/turbo-tasks/src/raw_vc.rs

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use crate::{
1111
id::LocalTaskId,
1212
manager::{read_local_output, read_task_cell, read_task_output, with_turbo_tasks},
1313
registry::{self, get_value_type},
14-
turbo_tasks, CollectiblesSource, ReadCellOptions, ReadConsistency, TaskId, TraitTypeId,
15-
ValueType, ValueTypeId, Vc, VcValueTrait,
14+
turbo_tasks, CollectiblesSource, ReadCellOptions, ReadConsistency, TaskId, TaskPersistence,
15+
TraitTypeId, ValueType, ValueTypeId, Vc, VcValueTrait,
1616
};
1717

1818
#[derive(Error, Debug)]
@@ -57,31 +57,34 @@ impl Display for CellId {
5757
pub enum RawVc {
5858
TaskOutput(TaskId),
5959
TaskCell(TaskId, CellId),
60-
LocalOutput(TaskId, LocalTaskId),
60+
LocalOutput(TaskId, TaskPersistence, LocalTaskId),
6161
}
6262

6363
impl RawVc {
6464
pub(crate) fn is_resolved(&self) -> bool {
6565
match self {
66-
RawVc::TaskOutput(_) => false,
67-
RawVc::TaskCell(_, _) => true,
68-
RawVc::LocalOutput(_, _) => false,
66+
RawVc::TaskOutput(..) => false,
67+
RawVc::TaskCell(..) => true,
68+
RawVc::LocalOutput(..) => false,
6969
}
7070
}
7171

7272
pub(crate) fn is_local(&self) -> bool {
7373
match self {
74-
RawVc::TaskOutput(_) => false,
75-
RawVc::TaskCell(_, _) => false,
76-
RawVc::LocalOutput(_, _) => true,
74+
RawVc::TaskOutput(..) => false,
75+
RawVc::TaskCell(..) => false,
76+
RawVc::LocalOutput(..) => true,
7777
}
7878
}
7979

80+
/// Returns `true` if the task this `RawVc` reads from cannot be serialized and will not be
81+
/// stored in the persistent cache.
82+
///
83+
/// See [`TaskPersistence`] for more details.
8084
pub fn is_transient(&self) -> bool {
8185
match self {
82-
RawVc::TaskOutput(task) | RawVc::TaskCell(task, _) | RawVc::LocalOutput(task, _) => {
83-
task.is_transient()
84-
}
86+
RawVc::TaskOutput(task) | RawVc::TaskCell(task, ..) => task.is_transient(),
87+
RawVc::LocalOutput(_, persistence, ..) => *persistence == TaskPersistence::Transient,
8588
}
8689
}
8790

@@ -145,7 +148,7 @@ impl RawVc {
145148
return Err(ResolveTypeError::NoContent);
146149
}
147150
}
148-
RawVc::LocalOutput(task_id, local_task_id) => {
151+
RawVc::LocalOutput(task_id, _persistence, local_task_id) => {
149152
current = read_local_output(&*tt, task_id, local_task_id)
150153
.await
151154
.map_err(|source| ResolveTypeError::TaskError { source })?;
@@ -182,7 +185,7 @@ impl RawVc {
182185
consistency = ReadConsistency::Eventual;
183186
}
184187
RawVc::TaskCell(_, _) => return Ok(current),
185-
RawVc::LocalOutput(task_id, local_task_id) => {
188+
RawVc::LocalOutput(task_id, _persistence, local_task_id) => {
186189
debug_assert_eq!(consistency, ReadConsistency::Eventual);
187190
current = read_local_output(&*tt, task_id, local_task_id).await?;
188191
}
@@ -197,7 +200,7 @@ impl RawVc {
197200
let mut current = self;
198201
loop {
199202
match current {
200-
RawVc::LocalOutput(task_id, local_task_id) => {
203+
RawVc::LocalOutput(task_id, _persistence, local_task_id) => {
201204
current = read_local_output(&*tt, task_id, local_task_id).await?;
202205
}
203206
non_local => return Ok(non_local),
@@ -212,7 +215,7 @@ impl RawVc {
212215

213216
pub fn get_task_id(&self) -> TaskId {
214217
match self {
215-
RawVc::TaskOutput(t) | RawVc::TaskCell(t, _) | RawVc::LocalOutput(t, _) => *t,
218+
RawVc::TaskOutput(t) | RawVc::TaskCell(t, _) | RawVc::LocalOutput(t, ..) => *t,
216219
}
217220
}
218221

@@ -347,7 +350,7 @@ impl Future for ReadRawVcFuture {
347350
Err(err) => return Poll::Ready(Err(err)),
348351
}
349352
}
350-
RawVc::LocalOutput(task_id, local_output_id) => {
353+
RawVc::LocalOutput(task_id, _persistence, local_output_id) => {
351354
debug_assert_eq!(this.consistency, ReadConsistency::Eventual);
352355
let read_result = tt.try_read_local_output(task_id, local_output_id);
353356
match read_result {

turbopack/crates/turbo-tasks/src/task/task_input.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ where
141141
}
142142

143143
fn is_transient(&self) -> bool {
144-
self.node.get_task_id().is_transient()
144+
self.node.is_transient()
145145
}
146146

147147
async fn resolve_input(&self) -> Result<Self> {

0 commit comments

Comments
 (0)