Skip to content

Commit a29bfa2

Browse files
authored
chore(turbo-tasks): Remove support for local_cells (a specific mode of local tasks we ended up not using) (#75672)
Local cells would allow us to avoid allocating real cells for data passed to local task functions. Unfortunately, in practice, we manually resolve most arguments passed to local task functions, which would defeat this optimization. I was holding onto this in the hopes that we might end up using it, but that seems fleeting: - We could get similar benefits by scanning for unreferenced cells at the end of function execution using `TraceRawVcs` at the cost of a bit of execution time. This would work even on `Vc`s that were resolved. - For ergonomics reasons, we want to eventually rename `ResolvedVc` to `Vc` (and `Vc` to something like `UnresolvedVc`), which will probably lead to us resolving even more stuff, breaking this potential optimization even more. - Cache eviction may do a good enough job of removing cells, the bigger enemy are the harder-to-evict tasks, which our current version of local tasks does do a good job of reducing. Meanwhile, it makes the code harder to follow, and could cause issues if this codepath somehow got activated.
1 parent e8b41f6 commit a29bfa2

File tree

20 files changed

+73
-495
lines changed

20 files changed

+73
-495
lines changed

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

-3
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,6 @@ impl UpdateOutputOperation {
9191
cell,
9292
})
9393
}
94-
Ok(Ok(RawVc::LocalCell(_, _))) => {
95-
panic!("LocalCell must not be output of a task");
96-
}
9794
Ok(Ok(RawVc::LocalOutput(_, _))) => {
9895
panic!("LocalOutput must not be output of a task");
9996
}

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

-1
This file was deleted.

turbopack/crates/turbo-tasks-macros-tests/tests/function/fail_attribute_invalid_args.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: unexpected token, expected one of: "fs", "network", "operation", "local", or "local_cells"
1+
error: unexpected token, expected one of: "fs", "network", "operation", "local"
22
--> tests/function/fail_attribute_invalid_args.rs:9:25
33
|
44
9 | #[turbo_tasks::function(invalid_argument)]

turbopack/crates/turbo-tasks-macros-tests/tests/function/fail_attribute_invalid_args_inherent_impl.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: unexpected token, expected one of: "fs", "network", "operation", "local", or "local_cells"
1+
error: unexpected token, expected one of: "fs", "network", "operation", "local"
22
--> tests/function/fail_attribute_invalid_args_inherent_impl.rs:14:29
33
|
44
14 | #[turbo_tasks::function(invalid_argument)]

turbopack/crates/turbo-tasks-macros/src/func.rs

+4-20
Original file line numberDiff line numberDiff line change
@@ -756,19 +756,14 @@ pub struct FunctionArguments {
756756
/// perform IO should not be manually annotated.
757757
io_markers: FxHashSet<IoMarker>,
758758
/// Should the function return an `OperationVc` instead of a `Vc`? Also ensures that all
759-
/// arguments are `OperationValue`s. Mutually exclusive with the `local_cells` flag.
759+
/// arguments are `OperationValue`s. Mutually exclusive with the `local` flag.
760760
///
761761
/// If there is an error due to this option being set, it should be reported to this span.
762762
operation: Option<Span>,
763763
/// Does not run the function as a real task, and instead runs it inside the parent task using
764764
/// task-local state. The function call itself will not be cached, but cells will be created on
765765
/// the parent task.
766766
pub local: Option<Span>,
767-
/// Changes the behavior of `Vc::cell` to create local cells that are not cached across task
768-
/// executions. Cells can be converted to their non-local versions by calling `Vc::resolve`.
769-
///
770-
/// If there is an error due to this option being set, it should be reported to this span.
771-
pub local_cells: Option<Span>,
772767
}
773768

774769
impl Parse for FunctionArguments {
@@ -796,26 +791,19 @@ impl Parse for FunctionArguments {
796791
("local", Meta::Path(_)) => {
797792
parsed_args.local = Some(meta.span());
798793
}
799-
("local_cells", Meta::Path(_)) => {
800-
parsed_args.local_cells = Some(meta.span());
801-
}
802794
(_, meta) => {
803795
return Err(syn::Error::new_spanned(
804796
meta,
805797
"unexpected token, expected one of: \"fs\", \"network\", \"operation\", \
806-
\"local\", or \"local_cells\"",
798+
\"local\"",
807799
))
808800
}
809801
}
810802
}
811-
if let (Some(_), Some(span)) = (
812-
parsed_args.local.or(parsed_args.local_cells),
813-
parsed_args.operation,
814-
) {
803+
if let (Some(_), Some(span)) = (parsed_args.local, parsed_args.operation) {
815804
return Err(syn::Error::new(
816805
span,
817-
"\"operation\" is mutually exclusive with the \"local\" and \"local_cells\" \
818-
options",
806+
"\"operation\" is mutually exclusive with the \"local\" option",
819807
));
820808
}
821809
Ok(parsed_args)
@@ -1101,7 +1089,6 @@ pub struct NativeFn {
11011089
pub is_method: bool,
11021090
pub filter_trait_call_args: Option<FilterTraitCallArgsTokens>,
11031091
pub local: bool,
1104-
pub local_cells: bool,
11051092
}
11061093

11071094
impl NativeFn {
@@ -1116,7 +1103,6 @@ impl NativeFn {
11161103
is_method,
11171104
filter_trait_call_args,
11181105
local,
1119-
local_cells,
11201106
} = self;
11211107

11221108
if *is_method {
@@ -1141,7 +1127,6 @@ impl NativeFn {
11411127
#function_path_string.to_owned(),
11421128
turbo_tasks::macro_helpers::FunctionMeta {
11431129
local: #local,
1144-
local_cells: #local_cells,
11451130
},
11461131
#arg_filter,
11471132
#function_path,
@@ -1156,7 +1141,6 @@ impl NativeFn {
11561141
#function_path_string.to_owned(),
11571142
turbo_tasks::macro_helpers::FunctionMeta {
11581143
local: #local,
1159-
local_cells: #local_cells,
11601144
},
11611145
#function_path,
11621146
)

turbopack/crates/turbo-tasks-macros/src/function_macro.rs

-2
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
4040
.inspect_err(|err| errors.push(err.to_compile_error()))
4141
.unwrap_or_default();
4242
let local = args.local.is_some();
43-
let local_cells = args.local_cells.is_some();
4443

4544
let Some(turbo_fn) = TurboFn::new(&sig, DefinitionContext::NakedFn, args) else {
4645
return quote! {
@@ -61,7 +60,6 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
6160
is_method: turbo_fn.is_method(),
6261
filter_trait_call_args: None, // not a trait method
6362
local,
64-
local_cells,
6563
};
6664
let native_function_ident = get_native_function_ident(ident);
6765
let native_function_ty = native_fn.ty();

turbopack/crates/turbo-tasks-macros/src/value_impl_macro.rs

-4
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
123123
.inspect_err(|err| errors.push(err.to_compile_error()))
124124
.unwrap_or_default();
125125
let local = func_args.local.is_some();
126-
let local_cells = func_args.local_cells.is_some();
127126

128127
let Some(turbo_fn) =
129128
TurboFn::new(sig, DefinitionContext::ValueInherentImpl, func_args)
@@ -142,7 +141,6 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
142141
is_method: turbo_fn.is_method(),
143142
filter_trait_call_args: None, // not a trait method
144143
local,
145-
local_cells,
146144
};
147145

148146
let native_function_ident = get_inherent_impl_function_ident(ty_ident, ident);
@@ -225,7 +223,6 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
225223
.inspect_err(|err| errors.push(err.to_compile_error()))
226224
.unwrap_or_default();
227225
let local = func_args.local.is_some();
228-
let local_cells = func_args.local_cells.is_some();
229226

230227
let Some(turbo_fn) =
231228
TurboFn::new(sig, DefinitionContext::ValueTraitImpl, func_args)
@@ -255,7 +252,6 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
255252
is_method: turbo_fn.is_method(),
256253
filter_trait_call_args: turbo_fn.filter_trait_call_args(),
257254
local,
258-
local_cells,
259255
};
260256

261257
let native_function_ident =

turbopack/crates/turbo-tasks-macros/src/value_macro.rs

-10
Original file line numberDiff line numberDiff line change
@@ -323,16 +323,6 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream {
323323
let content = self;
324324
turbo_tasks::ResolvedVc::cell_private(#cell_access_content)
325325
}
326-
327-
/// Places a value in a task-local cell stored in the current task.
328-
///
329-
/// Task-local cells are stored in a task-local arena, and do not persist outside the
330-
/// lifetime of the current task (including child tasks). Task-local cells can be resolved
331-
/// to be converted into normal cells.
332-
#cell_prefix fn local_cell(self) -> turbo_tasks::Vc<Self> {
333-
let content = self;
334-
turbo_tasks::Vc::local_cell_private(#cell_access_content)
335-
}
336326
};
337327

338328
let into = if let IntoMode::New | IntoMode::Shared = into_mode {

turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,11 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
121121
},
122122
is_method: turbo_fn.is_method(),
123123
filter_trait_call_args: turbo_fn.filter_trait_call_args(),
124-
// `local` and `local_cells` are currently unsupported here because:
124+
// `local` is currently unsupported here because:
125125
// - The `#[turbo_tasks::function]` macro needs to be present for us to read this
126126
// argument. (This could be fixed)
127127
// - This only makes sense when a default implementation is present.
128128
local: false,
129-
local_cells: false,
130129
};
131130

132131
let native_function_ident = get_trait_default_impl_function_ident(trait_ident, ident);

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

-1
This file was deleted.

turbopack/crates/turbo-tasks-testing/src/lib.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use turbo_tasks::{
2020
registry,
2121
test_helpers::with_turbo_tasks_for_testing,
2222
util::{SharedError, StaticOrArc},
23-
CellId, ExecutionId, InvalidationReason, LocalTaskId, MagicAny, RawVc, ReadConsistency, TaskId,
23+
CellId, InvalidationReason, LocalTaskId, MagicAny, RawVc, ReadConsistency, TaskId,
2424
TaskPersistence, TraitTypeId, TurboTasksApi, TurboTasksCallApi,
2525
};
2626

@@ -57,11 +57,9 @@ impl VcStorage {
5757
i
5858
};
5959
let task_id = TaskId::from(i as u32 + 1);
60-
let execution_id = ExecutionId::from(i as u64 + 1);
6160
handle.spawn(with_turbo_tasks_for_testing(
6261
this.clone(),
6362
task_id,
64-
execution_id,
6563
async move {
6664
let result = AssertUnwindSafe(future).catch_unwind().await;
6765

@@ -316,7 +314,6 @@ impl VcStorage {
316314
..Default::default()
317315
}),
318316
TaskId::from(u32::MAX),
319-
ExecutionId::from(u64::MAX),
320317
f,
321318
)
322319
}

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

-163
This file was deleted.

turbopack/crates/turbo-tasks-testing/tests/shrink_to_fit.rs

-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ async fn test_shrink_to_fit() -> Result<()> {
1818
let a: Vc<Wrapper> = Vc::cell(Vec::with_capacity(100));
1919
assert_eq!(a.await?.capacity(), 0);
2020

21-
let b: Vc<Wrapper> = Vc::local_cell(Vec::with_capacity(100));
22-
assert_eq!(b.await?.capacity(), 0);
23-
2421
Ok(())
2522
})
2623
.await

0 commit comments

Comments
 (0)