Skip to content

Commit 03a6b18

Browse files
authored
[turbopack] Implement remainder of local Vcs: casting and get_task_id (#68474)
*This is a migrated PR. This was in the turbo repository before the next.js merge.* **Migrated From:** vercel/turborepo#8871 ## Description With these changes, local Vcs (introduced in #68469) should have a fully functional implementation. These remaining methods were pretty straightforward. After this, my focus will shift back towards the changes needed to tasks to allow us to make use of local Vcs. ## New `resolve_type_inner` helper method I merged the implementations of `resolve_trait` and `resolve_value`, since their implementations were 95% identical. I think this is an overall win, though the logic to prevent duplicate `ValueType` lookups (they're not expensive, but this code is also potentially very hot) is a bit messy. ## Testing Instructions ``` cargo nextest r -p turbo-tasks -p turbo-tasks-memory ```
1 parent fe99a93 commit 03a6b18

File tree

3 files changed

+133
-45
lines changed

3 files changed

+133
-45
lines changed

turbopack/crates/turbo-tasks-memory/tests/local_cell.rs

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![feature(arbitrary_self_types)]
22

3-
use turbo_tasks::Vc;
3+
use turbo_tasks::{debug::ValueDebug, test_helpers::current_task_for_testing, ValueDefault, Vc};
44
use turbo_tasks_testing::{register, run, Registration};
55

66
static REGISTRATION: Registration = register!();
@@ -57,6 +57,57 @@ async fn test_return_resolved() {
5757
.await
5858
}
5959

60+
#[turbo_tasks::value_trait]
61+
trait UnimplementedTrait {}
62+
63+
#[tokio::test]
64+
async fn test_try_resolve_sidecast() {
65+
run(&REGISTRATION, async {
66+
let trait_vc: Vc<Box<dyn ValueDebug>> = Vc::upcast(Vc::<u32>::local_cell(42));
67+
68+
// `u32` is both a `ValueDebug` and a `ValueDefault`, so this sidecast is valid
69+
let sidecast_vc = Vc::try_resolve_sidecast::<Box<dyn ValueDefault>>(trait_vc)
70+
.await
71+
.unwrap();
72+
assert!(sidecast_vc.is_some());
73+
74+
// `u32` is not an `UnimplementedTrait` though, so this should return None
75+
let wrongly_sidecast_vc = Vc::try_resolve_sidecast::<Box<dyn UnimplementedTrait>>(trait_vc)
76+
.await
77+
.unwrap();
78+
assert!(wrongly_sidecast_vc.is_none());
79+
})
80+
.await
81+
}
82+
83+
#[tokio::test]
84+
async fn test_try_resolve_downcast_type() {
85+
run(&REGISTRATION, async {
86+
let trait_vc: Vc<Box<dyn ValueDebug>> = Vc::upcast(Vc::<u32>::local_cell(42));
87+
88+
let downcast_vc: Vc<u32> = Vc::try_resolve_downcast_type(trait_vc)
89+
.await
90+
.unwrap()
91+
.unwrap();
92+
assert_eq!(*downcast_vc.await.unwrap(), 42);
93+
94+
let wrongly_downcast_vc: Option<Vc<i64>> =
95+
Vc::try_resolve_downcast_type(trait_vc).await.unwrap();
96+
assert!(wrongly_downcast_vc.is_none());
97+
})
98+
.await
99+
}
100+
101+
#[tokio::test]
102+
async fn test_get_task_id() {
103+
run(&REGISTRATION, async {
104+
// the task id as reported by the RawVc
105+
let vc_task_id = Vc::into_raw(Vc::<()>::local_cell(())).get_task_id();
106+
assert_eq!(vc_task_id, current_task_for_testing());
107+
})
108+
.await
109+
}
110+
60111
#[turbo_tasks::value(eq = "manual")]
61112
#[derive(Default)]
62113
struct Untracked {
@@ -83,7 +134,7 @@ async fn get_untracked_local_cell() -> Vc<Untracked> {
83134

84135
#[tokio::test]
85136
#[should_panic(expected = "Local Vcs must only be accessed within their own task")]
86-
async fn test_panics_on_local_cell_escape() {
137+
async fn test_panics_on_local_cell_escape_read() {
87138
run(&REGISTRATION, async {
88139
get_untracked_local_cell()
89140
.await
@@ -94,3 +145,12 @@ async fn test_panics_on_local_cell_escape() {
94145
})
95146
.await
96147
}
148+
149+
#[tokio::test]
150+
#[should_panic(expected = "Local Vcs must only be accessed within their own task")]
151+
async fn test_panics_on_local_cell_escape_get_task_id() {
152+
run(&REGISTRATION, async {
153+
Vc::into_raw(get_untracked_local_cell().await.unwrap().cell).get_task_id();
154+
})
155+
.await
156+
}

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,7 +1755,8 @@ pub(crate) fn create_local_cell(value: TypedSharedReference) -> (ExecutionId, Lo
17551755
/// local cells are always filled. The returned value can be cheaply converted
17561756
/// with `.into()`.
17571757
///
1758-
/// Panics if the ExecutionId does not match the expected value.
1758+
/// Panics if the [`ExecutionId`] does not match the current task's
1759+
/// `execution_id`.
17591760
pub(crate) fn read_local_cell(
17601761
execution_id: ExecutionId,
17611762
local_cell_id: LocalCellId,
@@ -1766,12 +1767,28 @@ pub(crate) fn read_local_cell(
17661767
local_cells,
17671768
..
17681769
} = &*cell.borrow();
1769-
assert_eq!(
1770-
execution_id, *expected_execution_id,
1771-
"This Vc is local. Local Vcs must only be accessed within their own task. Resolve the \
1772-
Vc to convert it into a non-local version."
1773-
);
1770+
assert_eq_local_cell(execution_id, *expected_execution_id);
17741771
// local cell ids are one-indexed (they use NonZeroU32)
17751772
local_cells[(*local_cell_id as usize) - 1].clone()
17761773
})
17771774
}
1775+
1776+
/// Panics if the [`ExecutionId`] does not match the current task's
1777+
/// `execution_id`.
1778+
pub(crate) fn assert_execution_id(execution_id: ExecutionId) {
1779+
CURRENT_TASK_STATE.with(|cell| {
1780+
let CurrentTaskState {
1781+
execution_id: expected_execution_id,
1782+
..
1783+
} = &*cell.borrow();
1784+
assert_eq_local_cell(execution_id, *expected_execution_id);
1785+
})
1786+
}
1787+
1788+
fn assert_eq_local_cell(actual: ExecutionId, expected: ExecutionId) {
1789+
assert_eq!(
1790+
actual, expected,
1791+
"This Vc is local. Local Vcs must only be accessed within their own task. Resolve the Vc \
1792+
to convert it into a non-local version."
1793+
);
1794+
}

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

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@ use crate::{
1616
backend::{CellContent, TypedCellContent},
1717
event::EventListener,
1818
id::{ExecutionId, LocalCellId},
19-
manager::{read_local_cell, read_task_cell, read_task_output, TurboTasksApi},
19+
manager::{
20+
assert_execution_id, current_task, read_local_cell, read_task_cell, read_task_output,
21+
TurboTasksApi,
22+
},
2023
registry::{self, get_value_type},
21-
turbo_tasks, CollectiblesSource, TaskId, TraitTypeId, ValueTypeId, Vc, VcValueTrait,
24+
turbo_tasks, CollectiblesSource, TaskId, TraitTypeId, ValueType, ValueTypeId, Vc, VcValueTrait,
2225
};
2326

2427
#[derive(Error, Debug)]
@@ -100,38 +103,31 @@ impl RawVc {
100103
self,
101104
trait_type: TraitTypeId,
102105
) -> Result<Option<RawVc>, ResolveTypeError> {
103-
let tt = turbo_tasks();
104-
tt.notify_scheduled_tasks();
105-
let mut current = self;
106-
loop {
107-
match current {
108-
RawVc::TaskOutput(task) => {
109-
current = read_task_output(&*tt, task, false)
110-
.await
111-
.map_err(|source| ResolveTypeError::TaskError { source })?;
112-
}
113-
RawVc::TaskCell(task, index) => {
114-
let content = read_task_cell(&*tt, task, index)
115-
.await
116-
.map_err(|source| ResolveTypeError::ReadError { source })?;
117-
if let TypedCellContent(value_type, CellContent(Some(_))) = content {
118-
if get_value_type(value_type).has_trait(&trait_type) {
119-
return Ok(Some(RawVc::TaskCell(task, index)));
120-
} else {
121-
return Ok(None);
122-
}
123-
} else {
124-
return Err(ResolveTypeError::NoContent);
125-
}
126-
}
127-
RawVc::LocalCell(_, _) => todo!(),
128-
}
129-
}
106+
self.resolve_type_inner(|value_type_id| {
107+
let value_type = get_value_type(value_type_id);
108+
(value_type.has_trait(&trait_type), Some(value_type))
109+
})
110+
.await
130111
}
131112

132113
pub(crate) async fn resolve_value(
133114
self,
134115
value_type: ValueTypeId,
116+
) -> Result<Option<RawVc>, ResolveTypeError> {
117+
self.resolve_type_inner(|cell_value_type| (cell_value_type == value_type, None))
118+
.await
119+
}
120+
121+
/// Helper for `resolve_trait` and `resolve_value`.
122+
///
123+
/// After finding a cell, returns `Ok(Some(...))` when `conditional` returns
124+
/// `true`, and `Ok(None)` when `conditional` returns `false`.
125+
///
126+
/// As an optimization, `conditional` may return the `&'static ValueType` to
127+
/// avoid a potential extra lookup later.
128+
async fn resolve_type_inner(
129+
self,
130+
conditional: impl FnOnce(ValueTypeId) -> (bool, Option<&'static ValueType>),
135131
) -> Result<Option<RawVc>, ResolveTypeError> {
136132
let tt = turbo_tasks();
137133
tt.notify_scheduled_tasks();
@@ -147,17 +143,29 @@ impl RawVc {
147143
let content = read_task_cell(&*tt, task, index)
148144
.await
149145
.map_err(|source| ResolveTypeError::ReadError { source })?;
150-
if let TypedCellContent(cell_value_type, CellContent(Some(_))) = content {
151-
if cell_value_type == value_type {
152-
return Ok(Some(RawVc::TaskCell(task, index)));
146+
if let TypedCellContent(value_type, CellContent(Some(_))) = content {
147+
return Ok(if conditional(value_type).0 {
148+
Some(RawVc::TaskCell(task, index))
153149
} else {
154-
return Ok(None);
155-
}
150+
None
151+
});
156152
} else {
157153
return Err(ResolveTypeError::NoContent);
158154
}
159155
}
160-
RawVc::LocalCell(_, _) => todo!(),
156+
RawVc::LocalCell(execution_id, local_cell_id) => {
157+
let shared_reference = read_local_cell(execution_id, local_cell_id);
158+
return Ok(
159+
if let (true, value_type) = conditional(shared_reference.0) {
160+
// re-use the `ValueType` lookup from `conditional`, if it exists
161+
let value_type =
162+
value_type.unwrap_or_else(|| get_value_type(shared_reference.0));
163+
Some((value_type.raw_cell)(shared_reference))
164+
} else {
165+
None
166+
},
167+
);
168+
}
161169
}
162170
}
163171
}
@@ -172,7 +180,7 @@ impl RawVc {
172180
self.resolve_inner(/* strongly_consistent */ true).await
173181
}
174182

175-
pub(crate) async fn resolve_inner(self, strongly_consistent: bool) -> Result<RawVc> {
183+
async fn resolve_inner(self, strongly_consistent: bool) -> Result<RawVc> {
176184
let tt = turbo_tasks();
177185
let mut current = self;
178186
let mut notified = false;
@@ -203,7 +211,10 @@ impl RawVc {
203211
pub fn get_task_id(&self) -> TaskId {
204212
match self {
205213
RawVc::TaskOutput(t) | RawVc::TaskCell(t, _) => *t,
206-
RawVc::LocalCell(_, _) => todo!(),
214+
RawVc::LocalCell(execution_id, _) => {
215+
assert_execution_id(*execution_id);
216+
current_task("RawVc::get_task_id")
217+
}
207218
}
208219
}
209220
}

0 commit comments

Comments
 (0)